netdev - Re: [PATCH] [RFC] net: phy: broadcom: Add DT LED configuration support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1c57c364-dbe8-42f8-836c-52fad76a3f48@broadcom.com>
Date: Mon, 22 Jan 2024 14:34:24 -0800
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Marek Vasut <marex@...x.de>, netdev@...r.kernel.org,
 Christian Marangi <ansuelsmth@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>, Andrew Lunn <andrew@...n.ch>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>, Conor Dooley <conor+dt@...nel.org>,
 Eric Dumazet <edumazet@...gle.com>, Heiner Kallweit <hkallweit1@...il.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Lee Jones <lee@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Pavel Machek <pavel@....cz>, Rafał Miłecki
 <rafal@...ecki.pl>, Rob Herring <robh+dt@...nel.org>,
 Russell King <linux@...linux.org.uk>, devicetree@...r.kernel.org,
 linux-leds@...r.kernel.org
Subject: Re: [PATCH] [RFC] net: phy: broadcom: Add DT LED configuration
 support

On 1/22/24 12:45, Marek Vasut wrote:
> The BCM54213E and similar PHYs have extensive LED configuration
> capabilities -- the PHY has two LEDs, either of the two LEDs can
> be configured to 1 of 16 functions (speed, TX, RX, activity, on,
> off, quality, ... multi-color) used to drive single-color LED.
> The multi-color mode is special, it provides 16 more sub-modes
> used to drive multi-color LED.
> 
> The current configuration -- both LEDs configured as multi-color,
> with both LEDs multi-color sub-mode set to link activity indicator,
> is not suitable for all systems in which this PHY is used.
> 
> Attempt to implement a way to describe the LED configuration in DT.
> 
> Use Documentation/devicetree/bindings/net/ethernet-phy.yaml leds {}
> subnode of the PHY DT node, describe both LEDs present on this PHY
> as single LEDs within the leds {} subnode. Each described LED is a
> subnode of its own, the description uses standard LED subsystem
> bindings from Documentation/devicetree/bindings/leds/common.yaml .
> 
> The DT description of the LED configuration can look for example
> like this:
> 
> "
> ethernet-phy@1 {
> ...
> 	leds {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		led@0 {
> 			reg = <0>;
> 			function = LED_FUNCTION_ACTIVITY;
> 		};
> 
> 		led@1 {
> 			reg = <1>;
> 			function = LED_FUNCTION_SPEED_2;
> 		};
> 	};
> };
> "
> 
> Implement parsing code in the broadcom PHY driver to detemine desired
> LED configuration from DT. In case the leds {} subnode is present, the
> parser code iterates over its subnodes and for each led@N subnode it
> parses the following properties:
> 
> - reg - LED ID, either 0 or 1, used to identify the LED on the PHY
> - function - LED single-color function (speed, TX, RX, multi-color...),
>               uses LED subsystem LED_FUNCTION_* string. The parser in
> 	     the driver maps this to register setting.
> - function-enumerator - In case function is set to "multi-color",
>                          the multi-color function number. The parser
> 			in the driver uses this value directly for
> 			the multi-color configuration register.
> 
> Once the properties are parsed, the LED configuration registers of the
> PHY are programmed.
> 
> The current list of LED subsystem LED_FUNCTION_* does not cover the
> entire list of possible single-color LED functions of this PHY, add
> example extension for "link speed 1" and "link speed 2" setting into
> the leds/common.h header file.
> 
> The function-enumerator should probably not be a number, but maybe
> some sort of macro specific to this PHY ? I would like to avoid new
> broadcom PHY specific DT properties.

The parsing should definitively not be in the driver code, the driver 
should only be providing a mapping between the function and enumerator 
and a method to set those. Christian has been working on Ethernet PHY 
LEDs for a while now, so he would be in a better position to comment 
about how to about that.

The LED functions and register interface is actually quite stable across 
Ethernet PHYs from Broadcom so this code, however it looks like in the 
future should be moved to bcm-phy-lib.[ch]. If and where they are 
differences we can account for them in the library or by having each PHY 
driver entry provide a bcm54xx_* wrapper function that provides a table 
with the appropriate mapping.

Thanks!

-- 
Florian

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)