| Message ID | 20230327141031.11904-13-ansuelsmth@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | net: Add basic LED support for switch/phy | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Mon, Mar 27, 2023 at 04:10:27PM +0200, Christian Marangi wrote: > Add LEDs definition example for qca8k Switch Family to describe how they > should be defined for a correct usage. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > .../devicetree/bindings/net/dsa/qca8k.yaml | 24 +++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml > index 389892592aac..ad354864187a 100644 > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml > @@ -18,6 +18,8 @@ description: > PHY it is connected to. In this config, an internal mdio-bus is registered and > the MDIO master is used for communication. Mixed external and internal > mdio-bus configurations are not supported by the hardware. > + Each phy has at most 3 LEDs connected and can be declared > + using the standard LEDs structure. > > properties: > compatible: > @@ -117,6 +119,7 @@ unevaluatedProperties: false > examples: > - | > #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/leds/common.h> > > mdio { > #address-cells = <1>; > @@ -226,6 +229,27 @@ examples: > label = "lan1"; > phy-mode = "internal"; > phy-handle = <&internal_phy_port1>; > + > + leds { > + #address-cells = <1>; > + #size-cells = <0>; > + > + led@0 { > + reg = <0>; > + color = <LED_COLOR_ID_WHITE>; > + function = LED_FUNCTION_LAN; > + function-enumerator = <1>; > + default-state = "keep"; > + }; > + > + led@1 { > + reg = <1>; > + color = <LED_COLOR_ID_AMBER>; > + function = LED_FUNCTION_LAN; > + function-enumerator = <1>; Isn't function-enumerator supposed to be unique within a given 'function'? > + default-state = "keep"; > + }; > + }; > }; > > port@2 { > -- > 2.39.2 >
On Thu, Apr 06, 2023 at 09:10:18AM -0500, Rob Herring wrote: > On Mon, Mar 27, 2023 at 04:10:27PM +0200, Christian Marangi wrote: > > Add LEDs definition example for qca8k Switch Family to describe how they > > should be defined for a correct usage. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > .../devicetree/bindings/net/dsa/qca8k.yaml | 24 +++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml > > index 389892592aac..ad354864187a 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml > > @@ -18,6 +18,8 @@ description: > > PHY it is connected to. In this config, an internal mdio-bus is registered and > > the MDIO master is used for communication. Mixed external and internal > > mdio-bus configurations are not supported by the hardware. > > + Each phy has at most 3 LEDs connected and can be declared > > + using the standard LEDs structure. > > > > properties: > > compatible: > > @@ -117,6 +119,7 @@ unevaluatedProperties: false > > examples: > > - | > > #include <dt-bindings/gpio/gpio.h> > > + #include <dt-bindings/leds/common.h> > > > > mdio { > > #address-cells = <1>; > > @@ -226,6 +229,27 @@ examples: > > label = "lan1"; > > phy-mode = "internal"; > > phy-handle = <&internal_phy_port1>; > > + > > + leds { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + led@0 { > > + reg = <0>; > > + color = <LED_COLOR_ID_WHITE>; > > + function = LED_FUNCTION_LAN; > > + function-enumerator = <1>; > > + default-state = "keep"; > > + }; > > + > > + led@1 { > > + reg = <1>; > > + color = <LED_COLOR_ID_AMBER>; > > + function = LED_FUNCTION_LAN; > > + function-enumerator = <1>; > > Isn't function-enumerator supposed to be unique within a given > 'function'? > In the following example the output would be: - amber:lan-1 - white:lan-1 So in theory it's unique for the same color and function. Is it acceptable? Seems sane that there may be multiple color for the same function (and enum) > > + default-state = "keep"; > > + }; > > + }; > > }; > > > > port@2 { > > -- > > 2.39.2 > >
> > > examples: > > > - | > > > #include <dt-bindings/gpio/gpio.h> > > > + #include <dt-bindings/leds/common.h> > > > > > > mdio { > > > #address-cells = <1>; > > > @@ -226,6 +229,27 @@ examples: > > > label = "lan1"; > > > phy-mode = "internal"; > > > phy-handle = <&internal_phy_port1>; > > > + > > > + leds { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + led@0 { > > > + reg = <0>; > > > + color = <LED_COLOR_ID_WHITE>; > > > + function = LED_FUNCTION_LAN; > > > + function-enumerator = <1>; > > > + default-state = "keep"; > > > + }; > > > + > > > + led@1 { > > > + reg = <1>; > > > + color = <LED_COLOR_ID_AMBER>; > > > + function = LED_FUNCTION_LAN; > > > + function-enumerator = <1>; > > > > Isn't function-enumerator supposed to be unique within a given > > 'function'? > > > > In the following example the output would be: > - amber:lan-1 > - white:lan-1 > > So in theory it's unique for the same color and function. Is it > acceptable? Seems sane that there may be multiple color for the same > function (and enum) But what does the -1 actually mean? At Pavel's request, i documented 'good' names for these LEDs. I suggested that if there are multiple LEDs for one MAC/PHY, you use something like 'left' or 'right' to indicate their position on the RJ45 socket. That has a clear meaning. Andrew
diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml index 389892592aac..ad354864187a 100644 --- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml @@ -18,6 +18,8 @@ description: PHY it is connected to. In this config, an internal mdio-bus is registered and the MDIO master is used for communication. Mixed external and internal mdio-bus configurations are not supported by the hardware. + Each phy has at most 3 LEDs connected and can be declared + using the standard LEDs structure. properties: compatible: @@ -117,6 +119,7 @@ unevaluatedProperties: false examples: - | #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/leds/common.h> mdio { #address-cells = <1>; @@ -226,6 +229,27 @@ examples: label = "lan1"; phy-mode = "internal"; phy-handle = <&internal_phy_port1>; + + leds { + #address-cells = <1>; + #size-cells = <0>; + + led@0 { + reg = <0>; + color = <LED_COLOR_ID_WHITE>; + function = LED_FUNCTION_LAN; + function-enumerator = <1>; + default-state = "keep"; + }; + + led@1 { + reg = <1>; + color = <LED_COLOR_ID_AMBER>; + function = LED_FUNCTION_LAN; + function-enumerator = <1>; + default-state = "keep"; + }; + }; }; port@2 {
Add LEDs definition example for qca8k Switch Family to describe how they should be defined for a correct usage. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- .../devicetree/bindings/net/dsa/qca8k.yaml | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+)