From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B04C71B653C for ; Thu, 16 Jan 2025 09:58:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737021532; cv=none; b=KufIavJTjdVqJCndl2wVIhPb78jd/4fMk9ZfwKu1tuvjcAb2WbZqSVTlMufr+CBH8WIS2qU663RACmQzCX4mUgmvjbLxp5ShmBKGGpTChYL3A0nrJxmYKwsKTyMMu39heO2eoE4n0KtsU2niBp8gczuDfvLbVGo5sUGdiTi6sY4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737021532; c=relaxed/simple; bh=Cyqv1j8F6DMV6z1elEYYVjcaLdtyX62sDpAusHZ4VIs=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=QCNyJZZhEQXP1tpuUfJywRKO1V3DJYEYfA8JMIVW12yo5ot9AuJjAvOIxdiiXK1Jg5Y4VlhwHugReuNfFMBfQE2vuzat/kwadcdcoo5Mj/r5nYYhNjGeDUmDNF9BDnDRUVKWaCrMgd+Xj1RcrKDQ0KNuZ+B31ke7xqArWx5EE10= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=T8+W1kNS; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="T8+W1kNS" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1737021528; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=L1XJosZW4NShn5dioApxiP1subq7KqxK4uXe93H7Ho8=; b=T8+W1kNSb2wed1yU9dChNhKSjhpNdf7zJ5JhBkqHVlN86MdxhdOSEL45EbADGdsUVeAxS+ Oxfa9g/MyVc5qZA+qkZCyeJ/Xp6k+Uyuqz5Y+Kkb+I1Pe5425fWO5fPpPTVdO8EE2MurRN J5IC602ocOZDDaWxxJ2A2+uqjNubkuA= Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-163-wccAnnjDPvuL0txUd-KUSQ-1; Thu, 16 Jan 2025 04:58:43 -0500 X-MC-Unique: wccAnnjDPvuL0txUd-KUSQ-1 X-Mimecast-MFC-AGG-ID: wccAnnjDPvuL0txUd-KUSQ Received: by mail-qk1-f197.google.com with SMTP id af79cd13be357-7bb849aa5fbso152143085a.0 for ; Thu, 16 Jan 2025 01:58:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737021523; x=1737626323; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=L1XJosZW4NShn5dioApxiP1subq7KqxK4uXe93H7Ho8=; b=YYtNGt0tBdcBNBsD4AnUc7AXhs2HX4EB3RdP6NZKt+Ve3nCrjAc8JLFT3LIZ0YhiLa HcfA+J5QaT7W05vSl3L0XmmCzoE7qnyYOA3OUN0GH0ZObaOhAYJuObdOuE6cxL6wdCYW IReOP+Ekgp+KPHkm540K551NsuhljDaTA8R3FYbl+RPY7B8ATySklxcorFuAZyBVMzI6 VE7HZAeP3Jnzkp8aq2zQ28yKiQ2b3VPm3cuXu2vUS2q5FQxUMVOA5Aqx2xOtO/sT+mcw DD3GUUd5rFr0C01ppgVzbq6SYM0qivSdUHYeEyGduUwlu6RdJTo2zIxgA7Rcg+t7qGLJ d32A== X-Forwarded-Encrypted: i=1; AJvYcCWjBRIkL3Dou+zh3HcvxvfvzVNdadIiB0ZqIGy8wmVS4kqSR2X5gvBQ9wPT6sDAzwtnsTO2XjyiCG3IG/I=@vger.kernel.org X-Gm-Message-State: AOJu0YwW1ftnhb+xcd1N+bk1KO+pZF+A1v74VtV07CTMPPKvn7g1S5wU JE8F3U7ELK1TxJD9DhFpDAj2C16kFp9SbocV+Zn9NNoV9TySkhd4SdF9i0/inEDkjRKHFNN8LuX BwkmNgD+7zzz1Ru7FkQ7Smv8DwH1cIiCooDBna2nKO4gD8K9Rd/WFbnM+RLIKcw== X-Gm-Gg: ASbGncslk0GSSAN+NujhdI2WN/70iTCkNEL5XmSymSNxzR6S1Cu42duj6QwZHqKy+rP Ucm4PJ8c21EaKctsEVtUORio9eO+PwQMtQlN+VW//n5Ms5LQeYhwflJmxAPjiA6sBifofmGm+xt NbtLW26Dy7dR9ekIA/F3GvQTNu0jt8wKgytSR7lfiILE2OqBNp1U97XINE7+Cq2bRss1hZOKysQ OpmFWiyMpn1gcUZC3Oj4QTkZGzu1ISk2KYElmBOlMj3FmvU+UnbhEKYo++ficSE/CaBWe0ZULvY Ovfgv76w71I= X-Received: by 2002:a05:620a:17a2:b0:7b8:6331:a55e with SMTP id af79cd13be357-7bcd975a181mr5034591685a.44.1737021522816; Thu, 16 Jan 2025 01:58:42 -0800 (PST) X-Google-Smtp-Source: AGHT+IHSzqYsRXFRHXmdcXRECfUeMtwN//Cgn8zDTTdXOVwpXVGGF5zjiCATJuBBM+fQrOcpaTKCnQ== X-Received: by 2002:a05:620a:17a2:b0:7b8:6331:a55e with SMTP id af79cd13be357-7bcd975a181mr5034589585a.44.1737021522466; Thu, 16 Jan 2025 01:58:42 -0800 (PST) Received: from [192.168.88.253] (146-241-15-169.dyn.eolo.it. [146.241.15.169]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7bce324828esm803620985a.45.2025.01.16.01.58.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Jan 2025 01:58:42 -0800 (PST) Message-ID: <4d02f786-e87e-4588-87ed-b5fa414a4b5a@redhat.com> Date: Thu, 16 Jan 2025 10:58:38 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [net-next,PATCH 2/2] net: phy: micrel: Add KSZ87XX Switch LED control To: Marek Vasut , netdev@vger.kernel.org Cc: "David S. Miller" , Andrew Lunn , Eric Dumazet , Heiner Kallweit , Jakub Kicinski , Russell King , Tristram Ha , UNGLinuxDriver@microchip.com, Vladimir Oltean , Woojung Huh , linux-kernel@vger.kernel.org References: <20250113001543.296510-1-marex@denx.de> <20250113001543.296510-2-marex@denx.de> Content-Language: en-US From: Paolo Abeni In-Reply-To: <20250113001543.296510-2-marex@denx.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 1/13/25 1:15 AM, Marek Vasut wrote: > The KSZ87xx switch contains LED control registers. There is one shared > global control register bitfield which affects behavior of all LEDs on > all ports, the Register 11 (0x0B): Global Control 9 bitfield [5:4]. > There is also one per-port Register 29/45/61 (0x1D/0x2D/0x3D): Port 1/2/3 > Control 10 bit 7 which controls enablement of both LEDs on each port > separately. > > Expose LED brightness control and HW offload support for both of the two > programmable LEDs on this KSZ87XX Switch. Note that on KSZ87xx there are > three or more instances of simple KSZ87XX Switch PHY, one for each port, > however, the registers which control the LED behavior are mostly shared. > > Introduce LED brightness control using Register 29/45/61 (0x1D/0x2D/0x3D): > Port 1/2/3 Control 10 bit 7. This bit selects between LEDs disabled and > LEDs set to Function mode. In case LED brightness is set to 0, both LEDs > are turned off, otherwise both LEDs are configured to Function mode which > follows the global Register 11 (0x0B): Global Control 9 bitfield [5:4] > setting. @Andrew, @Russel: can the above problem be address with the current phy API? or does phy device need to expose a new brightness_get op? [...] > @@ -891,6 +892,112 @@ static int ksz8795_match_phy_device(struct phy_device *phydev) > return ksz8051_ksz8795_match_phy_device(phydev, false); > } > > +#define KSZ8795_LED_COUNT 2 > + > +static const unsigned long ksz8795_led_rules_map[4][2] = { > + { > + /* Control Bits = 2'b00 => LEDx_0=Link/ACT LEDx_1=Speed */ > + BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_RX) | > + BIT(TRIGGER_NETDEV_TX), > + BIT(TRIGGER_NETDEV_LINK_100) > + }, { > + /* Control Bits = 2'b01 => LEDx_0=Link LEDx_1=ACT */ > + BIT(TRIGGER_NETDEV_LINK), > + BIT(TRIGGER_NETDEV_RX) | BIT(TRIGGER_NETDEV_TX) > + }, { > + /* Control Bits = 2'b10 => LEDx_0=Link/ACT LEDx_1=Duplex */ > + BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_RX) | > + BIT(TRIGGER_NETDEV_TX), > + BIT(TRIGGER_NETDEV_FULL_DUPLEX) > + }, { > + /* Control Bits = 2'b11 => LEDx_0=Link LEDx_1=Duplex */ > + BIT(TRIGGER_NETDEV_LINK), > + BIT(TRIGGER_NETDEV_FULL_DUPLEX) > + } > +}; > + > +static int ksz8795_led_brightness_set(struct phy_device *phydev, u8 index, > + enum led_brightness value) > +{ > + /* Turn all LEDs on this port on or off */ > + /* Emulated rmw of Register 29/45/61 (0x1D/0x2D/0x3D): Port 1/2/3 Control 10 */ > + return phy_modify(phydev, 0x0d00, BIT(7), (value == LED_OFF) ? BIT(7) : 0); Please defines macros for all the above 'magic numbers' > +} > + > +static int ksz8795_led_hw_is_supported(struct phy_device *phydev, u8 index, > + unsigned long rules) > +{ > + const unsigned long mask[2] = { > + BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_RX) | > + BIT(TRIGGER_NETDEV_TX), > + BIT(TRIGGER_NETDEV_LINK_100) | BIT(TRIGGER_NETDEV_RX) | > + BIT(TRIGGER_NETDEV_TX) | BIT(TRIGGER_NETDEV_FULL_DUPLEX) > + }; > + > + if (index >= KSZ8795_LED_COUNT) > + return -EINVAL; > + > + /* Filter out any other unsupported triggers. */ > + if (rules & ~mask[index]) > + return -EOPNOTSUPP; > + > + /* RX and TX are not differentiated, either both are set or not set. */ > + if (!(rules & BIT(TRIGGER_NETDEV_RX)) ^ !(rules & BIT(TRIGGER_NETDEV_TX))) > + return -EOPNOTSUPP; > + > + return 0; > +} > + > +static int ksz8795_led_hw_control_get(struct phy_device *phydev, u8 index, > + unsigned long *rules) > +{ > + int val; > + > + if (index >= KSZ8795_LED_COUNT) > + return -EINVAL; > + > + /* Emulated read of Register 11 (0x0B): Global Control 9 */ > + val = phy_read(phydev, 0x0b00); > + if (val < 0) > + return val; > + > + /* Extract bits [5:4] and look up matching LED configuration */ > + *rules = ksz8795_led_rules_map[(val >> 4) & 0x3][index]; This calls for FIELD_GET() usage. [...] > @@ -5666,10 +5773,15 @@ static struct phy_driver ksphy_driver[] = { > }, { > .name = "Micrel KSZ87XX Switch", > /* PHY_BASIC_FEATURES */ > + .probe = kszphy_probe, > .config_init = kszphy_config_init, > .match_phy_device = ksz8795_match_phy_device, > .suspend = genphy_suspend, > .resume = genphy_resume, > + .led_brightness_set = ksz8795_led_brightness_set, > + .led_hw_is_supported = ksz8795_led_hw_is_supported, > + .led_hw_control_get = ksz8795_led_hw_control_get, > + .led_hw_control_set = ksz8795_led_hw_control_set, The preferred style is to use an additional tab to align all the '=' in this new block. /P