On 1/22/24 23:34, Florian Fainelli wrote:
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.
I very much agree. I also hope Rafal can chime in, I saw some openwrt
LED patches floating around recently.
