Add bindings for LAN9645X switch. We use a fallback compatible for the
smallest SKU microchip,lan96455s-switch.
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
---
.../net/dsa/microchip,lan9645x-switch.yaml | 137 +++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 138 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml
new file mode 100644
index 000000000000..4a19dfa7e9d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml
@@ -0,0 +1,137 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/microchip,lan9645x-switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip LAN9645x Ethernet switch
+
+maintainers:
+ - Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
+
+description: |
+ The LAN9645x switch is a multi-port Gigabit AVB/TSN Ethernet switch with
+ five integrated 10/100/1000Base-T PHYs. In addition to the integrated PHYs,
+ it supports up to 2 RGMII/RMII, up to 2 BASE-X/SERDES/2.5GBASE-X and one
+ Quad-SGMII interfaces.
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - microchip,lan96455s-switch
+ - items:
+ - enum:
+ - microchip,lan96455f-switch
+ - microchip,lan96457f-switch
+ - microchip,lan96459f-switch
+ - microchip,lan96457s-switch
+ - microchip,lan96459s-switch
+ - const: microchip,lan96455s-switch
+
+ reg:
+ maxItems: 1
+
+$ref: dsa.yaml#
+
+patternProperties:
+ "^(ethernet-)?ports$":
+ type: object
+ additionalProperties: true
+ patternProperties:
+ "^(ethernet-)?port@[0-8]$":
+ type: object
+ description: Ethernet switch ports
+
+ $ref: dsa-port.yaml#
+
+ properties:
+ microchip,led-drive-mode:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Set the LED drive mode for the copper PHY associated with
+ this port.
+
+ 0 - LED1 and LED2 in open-drain mode
+ 1 - LED1 in active drive mode (can be used for single-LED
+ configurations requiring active drive)
+ 2 - Reserved
+ 3 - LED1 and LED2 in active drive mode
+ minimum: 0
+ maximum: 3
+
+ unevaluatedProperties: false
+
+oneOf:
+ - required:
+ - ports
+ - required:
+ - ethernet-ports
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ soc {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-switch@0 {
+ reg = <0>;
+ compatible = "microchip,lan96459f-switch", "microchip,lan96455s-switch";
+ pinctrl-0 = <&lan9645x_leds>;
+ pinctrl-names = "default";
+
+ ethernet-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ label = "lan0";
+ phy-mode = "gmii";
+ phy-handle = <&cuphy0>;
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "lan1";
+ phy-mode = "gmii";
+ phy-handle = <&cuphy1>;
+ microchip,led-drive-mode = <3>;
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "lan2";
+ phy-mode = "gmii";
+ phy-handle = <&cuphy2>;
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "lan3";
+ phy-mode = "gmii";
+ phy-handle = <&cuphy3>;
+ };
+
+ port@7 {
+ reg = <7>;
+ label = "lan7";
+ phy-mode = "rgmii-id";
+ ethernet = <&cpu_host_port>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ pause;
+ };
+ };
+ };
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 2712aaf7cedd..ab92b342877b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17291,6 +17291,7 @@ M: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
M: UNGLinuxDriver@microchip.com
L: netdev@vger.kernel.org
S: Maintained
+F: Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml
F: include/linux/dsa/lan9645x.h
F: net/dsa/tag_lan9645x.c
--
2.52.0
On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote:
> +examples:
> + - |
> + soc {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-switch@0 {
> + reg = <0>;
Also, this is an odd example, why are you at address 0 on a "soc" bus,
which usually means that this device on an AXI/AHB bus, and 0 is very
unusual for that. Obviously the example doesn't have to match the real
user, but this stands out.
I may have some follow up questions I think depending on your answer.
On Tue, 2026-03-03 at 18:56 +0000, Conor Dooley wrote:
> On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote:
> > +examples:
> > + - |
> > + soc {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ethernet-switch@0 {
> > + reg = <0>;
>
> Also, this is an odd example, why are you at address 0 on a "soc" bus,
> which usually means that this device on an AXI/AHB bus, and 0 is very
> unusual for that. Obviously the example doesn't have to match the real
> user, but this stands out.
> I may have some follow up questions I think depending on your answer.
The intended way to bind this driver is via a parent MFD driver which sets
up the SPI register protocol, initiates regmaps and distributes them to child
devices (like this DSA driver).
Similar to mscc,vsc7512 in drivers/mfd/ocelot-spi.c.
This MFD would be the soc node. All child nodes perform register IO over
spi, using the regmaps requested from this parent so I think the addresses
on the bus are purely ornamental. Should I write the smallest register
address in all regions used by the DSA driver instead?
On Wed, Mar 04, 2026 at 05:10:11PM +0100, Jens Emil Schulz Ostergaard wrote:
> On Tue, 2026-03-03 at 18:56 +0000, Conor Dooley wrote:
> > On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote:
> > > +examples:
> > > + - |
> > > + soc {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + ethernet-switch@0 {
> > > + reg = <0>;
> >
> > Also, this is an odd example, why are you at address 0 on a "soc" bus,
> > which usually means that this device on an AXI/AHB bus, and 0 is very
> > unusual for that. Obviously the example doesn't have to match the real
> > user, but this stands out.
> > I may have some follow up questions I think depending on your answer.
>
> The intended way to bind this driver is via a parent MFD driver which sets
> up the SPI register protocol, initiates regmaps and distributes them to child
> devices (like this DSA driver).
>
> Similar to mscc,vsc7512 in drivers/mfd/ocelot-spi.c.
>
> This MFD would be the soc node. All child nodes perform register IO over
> spi, using the regmaps requested from this parent so I think the addresses
> on the bus are purely ornamental. Should I write the smallest register
> address in all regions used by the DSA driver instead?
They are not ornamental, they should be the same addresses you'd put if
Linux had direct access to the SoC interconnect for MMIO, rather than to
an SPI bridge to the SoC interconnect. Or at least I don't see why it
wouldn't be that way.
On Wed, Mar 04, 2026 at 06:14:57PM +0200, Vladimir Oltean wrote:
> On Wed, Mar 04, 2026 at 05:10:11PM +0100, Jens Emil Schulz Ostergaard wrote:
> > On Tue, 2026-03-03 at 18:56 +0000, Conor Dooley wrote:
> > > On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote:
> > > > +examples:
> > > > + - |
> > > > + soc {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + ethernet-switch@0 {
> > > > + reg = <0>;
> > >
> > > Also, this is an odd example, why are you at address 0 on a "soc" bus,
> > > which usually means that this device on an AXI/AHB bus, and 0 is very
> > > unusual for that. Obviously the example doesn't have to match the real
> > > user, but this stands out.
> > > I may have some follow up questions I think depending on your answer.
> >
> > The intended way to bind this driver is via a parent MFD driver which sets
> > up the SPI register protocol, initiates regmaps and distributes them to child
> > devices (like this DSA driver).
> >
> > Similar to mscc,vsc7512 in drivers/mfd/ocelot-spi.c.
> >
> > This MFD would be the soc node. All child nodes perform register IO over
> > spi, using the regmaps requested from this parent so I think the addresses
> > on the bus are purely ornamental. Should I write the smallest register
> > address in all regions used by the DSA driver instead?
>
> They are not ornamental, they should be the same addresses you'd put if
> Linux had direct access to the SoC interconnect for MMIO, rather than to
> an SPI bridge to the SoC interconnect. Or at least I don't see why it
> wouldn't be that way.
I don't mind if they're made up addresses, but the example should be
realistic. 0 is likely not realistic, but if it was 0x4000_0000 when the
real thing is 0x8123_1234 then I don't really care. Since the parent is
not actually a soc bus, can't you just remove this fake parent entirely,
like the ocelot switch example does? Or insert something more genuine,
like:
spi {
#address-cells = <1>;
#size-cells = <0>;
soc@0 {
compatible = "microchip,lan96455s";
reg = <0>;
#address-cells = <1>;
#size-cells = <1>;
ethernet-switch@4000000 {
reg = <4000000 44>;
On Wed, 2026-03-04 at 19:06 +0000, Conor Dooley wrote:
> On Wed, Mar 04, 2026 at 06:14:57PM +0200, Vladimir Oltean wrote:
> > On Wed, Mar 04, 2026 at 05:10:11PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > On Tue, 2026-03-03 at 18:56 +0000, Conor Dooley wrote:
> > > > On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote:
> > > > > +examples:
> > > > > + - |
> > > > > + soc {
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + ethernet-switch@0 {
> > > > > + reg = <0>;
> > > >
> > > > Also, this is an odd example, why are you at address 0 on a "soc" bus,
> > > > which usually means that this device on an AXI/AHB bus, and 0 is very
> > > > unusual for that. Obviously the example doesn't have to match the real
> > > > user, but this stands out.
> > > > I may have some follow up questions I think depending on your answer.
> > >
> > > The intended way to bind this driver is via a parent MFD driver which sets
> > > up the SPI register protocol, initiates regmaps and distributes them to child
> > > devices (like this DSA driver).
> > >
> > > Similar to mscc,vsc7512 in drivers/mfd/ocelot-spi.c.
> > >
> > > This MFD would be the soc node. All child nodes perform register IO over
> > > spi, using the regmaps requested from this parent so I think the addresses
> > > on the bus are purely ornamental. Should I write the smallest register
> > > address in all regions used by the DSA driver instead?
> >
> > They are not ornamental, they should be the same addresses you'd put if
> > Linux had direct access to the SoC interconnect for MMIO, rather than to
> > an SPI bridge to the SoC interconnect. Or at least I don't see why it
> > wouldn't be that way.
>
> I don't mind if they're made up addresses, but the example should be
> realistic. 0 is likely not realistic, but if it was 0x4000_0000 when the
> real thing is 0x8123_1234 then I don't really care. Since the parent is
> not actually a soc bus, can't you just remove this fake parent entirely,
> like the ocelot switch example does? Or insert something more genuine,
> like:
> spi {
> #address-cells = <1>;
> #size-cells = <0>;
>
> soc@0 {
> compatible = "microchip,lan96455s";
> reg = <0>;
> #address-cells = <1>;
> #size-cells = <1>;
>
> ethernet-switch@4000000 {
> reg = <4000000 44>;
>
I will go with the genuine example. I was not sure if it was appropriate
to mention the spi node from here. But this is what it would look like with
the mfd driver. I will also fix the addresses.
Thanks,
Emil
On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote: > Add bindings for LAN9645X switch. We use a fallback compatible for the > smallest SKU microchip,lan96455s-switch. > > Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com> > Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> > --- > .../net/dsa/microchip,lan9645x-switch.yaml | 137 +++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 138 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml > new file mode 100644 > index 000000000000..4a19dfa7e9d5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml > @@ -0,0 +1,137 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/dsa/microchip,lan9645x-switch.yaml# Just name the binding microchip,lan96455s-switch please, to match the standalone compatible.
On Tue, 2026-03-03 at 18:49 +0000, Conor Dooley wrote: > On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote: > > Add bindings for LAN9645X switch. We use a fallback compatible for the > > smallest SKU microchip,lan96455s-switch. > > > > Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com> > > Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> > > --- > > .../net/dsa/microchip,lan9645x-switch.yaml | 137 +++++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 138 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml > > new file mode 100644 > > index 000000000000..4a19dfa7e9d5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan9645x-switch.yaml > > @@ -0,0 +1,137 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/dsa/microchip,lan9645x-switch.yaml# > > Just name the binding microchip,lan96455s-switch please, to match the > standalone compatible. I will rename the binding. Thanks, Emil
> + properties:
> + microchip,led-drive-mode:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Set the LED drive mode for the copper PHY associated with
> + this port.
> +
> + 0 - LED1 and LED2 in open-drain mode
> + 1 - LED1 in active drive mode (can be used for single-LED
> + configurations requiring active drive)
> + 2 - Reserved
> + 3 - LED1 and LED2 in active drive mode
> + minimum: 0
> + maximum: 3
I doubt the DT Maintainers will accept that. This looks a lot like a
value you write into a register. How are active drive and open-drain
described in other DT bindings? Is there something you can reuse?
For 1, what happens to LED2? Not used at all?
Andrew
On Tue, 2026-03-03 at 15:18 +0100, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > + properties: > > + microchip,led-drive-mode: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + Set the LED drive mode for the copper PHY associated with > > + this port. > > + > > + 0 - LED1 and LED2 in open-drain mode > > + 1 - LED1 in active drive mode (can be used for single-LED > > + configurations requiring active drive) > > + 2 - Reserved > > + 3 - LED1 and LED2 in active drive mode > > + minimum: 0 > > + maximum: 3 > > I doubt the DT Maintainers will accept that. This looks a lot like a > value you write into a register. How are active drive and open-drain > described in other DT bindings? Is there something you can reuse? > > For 1, what happens to LED2? Not used at all? > > Andrew Yes, this is written to a register. I did not know that was a no-no. I believe LED2 is not used, but I will ask design about 1 and LED2 to be sure. The register model does not mention LED2 here. Thanks, Emil
On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote: > > + properties: > > + microchip,led-drive-mode: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: | > > + Set the LED drive mode for the copper PHY associated with > > + this port. > > + > > + 0 - LED1 and LED2 in open-drain mode > > + 1 - LED1 in active drive mode (can be used for single-LED > > + configurations requiring active drive) > > + 2 - Reserved > > + 3 - LED1 and LED2 in active drive mode > > + minimum: 0 > > + maximum: 3 > > I doubt the DT Maintainers will accept that. This looks a lot like a > value you write into a register. How are active drive and open-drain > described in other DT bindings? Is there something you can reuse? I had a quick look and I didn't see anything really that stood out to me that would be a drop-in replacement. I also tried looking in the datasheet for more information on these modes, but I couldn't see anything obvious. For example, there were zero hits for "drain" in either LAN9645xS or LAN9645xF datasheets. That said, yea you're right about DT maintainer feelings about it. There's a couple things I could suggest, but I'd like to know about what mode 1 means for LED2 first. If there's actually nothing similar, what about representing each led with a child node and having open-drain be the default with a property in the child for active-drive? > > For 1, what happens to LED2? Not used at all? > > Andrew
On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote: > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote: > > > + properties: > > > + microchip,led-drive-mode: > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + description: | > > > + Set the LED drive mode for the copper PHY associated with > > > + this port. > > > + > > > + 0 - LED1 and LED2 in open-drain mode > > > + 1 - LED1 in active drive mode (can be used for single-LED > > > + configurations requiring active drive) > > > + 2 - Reserved > > > + 3 - LED1 and LED2 in active drive mode > > > + minimum: 0 > > > + maximum: 3 > > > > I doubt the DT Maintainers will accept that. This looks a lot like a > > value you write into a register. How are active drive and open-drain > > described in other DT bindings? Is there something you can reuse? > > I had a quick look and I didn't see anything really that stood out to me > that would be a drop-in replacement. > I also tried looking in the datasheet for more information on these > modes, but I couldn't see anything obvious. For example, there were zero > hits for "drain" in either LAN9645xS or LAN9645xF datasheets. > > That said, yea you're right about DT maintainer feelings about it. > There's a couple things I could suggest, but I'd like to know about what > mode 1 means for LED2 first. If there's actually nothing similar, what > about representing each led with a child node and having open-drain be > the default with a property in the child for active-drive? > > > > > For 1, what happens to LED2? Not used at all? > > > > Andrew In mode 1 LED2 will be open-drain. This mode only makes sense if you have just 1 LED. With two LEDs mode 0 or mode 3 should be used. Thanks, Emil
On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote: > On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote: > > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote: > > > > + properties: > > > > + microchip,led-drive-mode: > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > + description: | > > > > + Set the LED drive mode for the copper PHY associated with > > > > + this port. > > > > + > > > > + 0 - LED1 and LED2 in open-drain mode > > > > + 1 - LED1 in active drive mode (can be used for single-LED > > > > + configurations requiring active drive) > > > > + 2 - Reserved > > > > + 3 - LED1 and LED2 in active drive mode > > > > + minimum: 0 > > > > + maximum: 3 > > > > > > I doubt the DT Maintainers will accept that. This looks a lot like a > > > value you write into a register. How are active drive and open-drain > > > described in other DT bindings? Is there something you can reuse? > > > > I had a quick look and I didn't see anything really that stood out to me > > that would be a drop-in replacement. > > I also tried looking in the datasheet for more information on these > > modes, but I couldn't see anything obvious. For example, there were zero > > hits for "drain" in either LAN9645xS or LAN9645xF datasheets. > > > > That said, yea you're right about DT maintainer feelings about it. > > There's a couple things I could suggest, but I'd like to know about what > > mode 1 means for LED2 first. If there's actually nothing similar, what > > about representing each led with a child node and having open-drain be > > the default with a property in the child for active-drive? > > > > > > > > For 1, what happens to LED2? Not used at all? > > In mode 1 LED2 will be open-drain. This mode only makes sense if you have > just 1 LED. With two LEDs mode 0 or mode 3 should be used. Could we then have child nodes for each led, and have a property in each that sets the mode to either open-drain or active-drive? Or am I just inserting complexity by asking for that?
On Thu, 2026-03-05 at 18:31 +0000, Conor Dooley wrote: > On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote: > > On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote: > > > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote: > > > > > + properties: > > > > > + microchip,led-drive-mode: > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > + description: | > > > > > + Set the LED drive mode for the copper PHY associated with > > > > > + this port. > > > > > + > > > > > + 0 - LED1 and LED2 in open-drain mode > > > > > + 1 - LED1 in active drive mode (can be used for single-LED > > > > > + configurations requiring active drive) > > > > > + 2 - Reserved > > > > > + 3 - LED1 and LED2 in active drive mode > > > > > + minimum: 0 > > > > > + maximum: 3 > > > > > > > > I doubt the DT Maintainers will accept that. This looks a lot like a > > > > value you write into a register. How are active drive and open-drain > > > > described in other DT bindings? Is there something you can reuse? > > > > > > I had a quick look and I didn't see anything really that stood out to me > > > that would be a drop-in replacement. > > > I also tried looking in the datasheet for more information on these > > > modes, but I couldn't see anything obvious. For example, there were zero > > > hits for "drain" in either LAN9645xS or LAN9645xF datasheets. > > > > > > That said, yea you're right about DT maintainer feelings about it. > > > There's a couple things I could suggest, but I'd like to know about what > > > mode 1 means for LED2 first. If there's actually nothing similar, what > > > about representing each led with a child node and having open-drain be > > > the default with a property in the child for active-drive? > > > > > > > > > > > For 1, what happens to LED2? Not used at all? > > > > In mode 1 LED2 will be open-drain. This mode only makes sense if you have > > just 1 LED. With two LEDs mode 0 or mode 3 should be used. > > Could we then have child nodes for each led, and have a property in each > that sets the mode to either open-drain or active-drive? Or am I just > inserting complexity by asking for that? I think it sounds sensible, I will add this. Thanks, Emil
On Fri, Mar 06, 2026 at 04:08:01PM +0100, Jens Emil Schulz Ostergaard wrote: > On Thu, 2026-03-05 at 18:31 +0000, Conor Dooley wrote: > > On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote: > > > On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote: > > > > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote: > > > > > > + properties: > > > > > > + microchip,led-drive-mode: > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > + description: | > > > > > > + Set the LED drive mode for the copper PHY associated with > > > > > > + this port. > > > > > > + > > > > > > + 0 - LED1 and LED2 in open-drain mode > > > > > > + 1 - LED1 in active drive mode (can be used for single-LED > > > > > > + configurations requiring active drive) > > > > > > + 2 - Reserved > > > > > > + 3 - LED1 and LED2 in active drive mode > > > > > > + minimum: 0 > > > > > > + maximum: 3 > > > > > > > > > > I doubt the DT Maintainers will accept that. This looks a lot like a > > > > > value you write into a register. How are active drive and open-drain > > > > > described in other DT bindings? Is there something you can reuse? > > > > > > > > I had a quick look and I didn't see anything really that stood out to me > > > > that would be a drop-in replacement. > > > > I also tried looking in the datasheet for more information on these > > > > modes, but I couldn't see anything obvious. For example, there were zero > > > > hits for "drain" in either LAN9645xS or LAN9645xF datasheets. > > > > > > > > That said, yea you're right about DT maintainer feelings about it. > > > > There's a couple things I could suggest, but I'd like to know about what > > > > mode 1 means for LED2 first. If there's actually nothing similar, what > > > > about representing each led with a child node and having open-drain be > > > > the default with a property in the child for active-drive? > > > > > > > > > > > > > > For 1, what happens to LED2? Not used at all? > > > > > > In mode 1 LED2 will be open-drain. This mode only makes sense if you have > > > just 1 LED. With two LEDs mode 0 or mode 3 should be used. > > > > Could we then have child nodes for each led, and have a property in each > > that sets the mode to either open-drain or active-drive? Or am I just > > inserting complexity by asking for that? > > I think it sounds sensible, I will add this. You don't need a property for each, just make one mode the default (prob open-drain given it's the 0 setting, but whatever is default out of reset for the part) and have the property for the other mode. Just some bool property like "microchip,active-drive" or whatever.
Hi Conor,
On Fri, 2026-03-06 at 15:20 +0000, Conor Dooley wrote:
> On Fri, Mar 06, 2026 at 04:08:01PM +0100, Jens Emil Schulz Ostergaard wrote:
> > On Thu, 2026-03-05 at 18:31 +0000, Conor Dooley wrote:
> > > On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote:
> > > > > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote:
> > > > > > > + properties:
> > > > > > > + microchip,led-drive-mode:
> > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > + description: |
> > > > > > > + Set the LED drive mode for the copper PHY associated with
> > > > > > > + this port.
> > > > > > > +
> > > > > > > + 0 - LED1 and LED2 in open-drain mode
> > > > > > > + 1 - LED1 in active drive mode (can be used for single-LED
> > > > > > > + configurations requiring active drive)
> > > > > > > + 2 - Reserved
> > > > > > > + 3 - LED1 and LED2 in active drive mode
> > > > > > > + minimum: 0
> > > > > > > + maximum: 3
> > > > > >
> > > > > > I doubt the DT Maintainers will accept that. This looks a lot like a
> > > > > > value you write into a register. How are active drive and open-drain
> > > > > > described in other DT bindings? Is there something you can reuse?
> > > > >
> > > > > I had a quick look and I didn't see anything really that stood out to me
> > > > > that would be a drop-in replacement.
> > > > > I also tried looking in the datasheet for more information on these
> > > > > modes, but I couldn't see anything obvious. For example, there were zero
> > > > > hits for "drain" in either LAN9645xS or LAN9645xF datasheets.
> > > > >
> > > > > That said, yea you're right about DT maintainer feelings about it.
> > > > > There's a couple things I could suggest, but I'd like to know about what
> > > > > mode 1 means for LED2 first. If there's actually nothing similar, what
> > > > > about representing each led with a child node and having open-drain be
> > > > > the default with a property in the child for active-drive?
> > > > >
> > > > > >
> > > > > > For 1, what happens to LED2? Not used at all?
> > > >
> > > > In mode 1 LED2 will be open-drain. This mode only makes sense if you have
> > > > just 1 LED. With two LEDs mode 0 or mode 3 should be used.
> > >
> > > Could we then have child nodes for each led, and have a property in each
> > > that sets the mode to either open-drain or active-drive? Or am I just
> > > inserting complexity by asking for that?
> >
> > I think it sounds sensible, I will add this.
>
>
> You don't need a property for each, just make one mode the default (prob
> open-drain given it's the 0 setting, but whatever is default out of
> reset for the part) and have the property for the other mode. Just
> some bool property like "microchip,active-drive" or whatever.
Based on your feedback I went with this under port properties:
leds:
patternProperties:
'^led@[a-f0-9]+$':
$ref: /schemas/leds/common.yaml#
properties:
reg:
maxItems: 1
microchip,active-drive:
type: boolean
description:
Set the LED output to active drive mode. The default
is open-drain.
required:
- reg
unevaluatedProperties: false
and then the example has
port@1 {
reg = <1>;
phy-mode = "gmii";
phy-handle = <&cuphy1>;
leds {
#address-cells = <1>;
#size-cells = <0>;
led@0 {
reg = <0>;
microchip,active-drive;
};
led@1 {
reg = <1>;
microchip,active-drive;
};
};
}
However, this does not pass dt_binding_check because we pull in $ref: dsa-port.yaml,
which pulls in ethernet-controller.yaml.
I believe the 'unevaluatedProperties: false' on LED nodes in ethernet-controller.yaml
prevents downstream bindings from adding vendor-specific LED properties.
Is the right move removing unevaluatedProperties: false from the LED node in
ethernet-controller.yaml, or is there a preferred way to extend per-port LEDs?
Thanks,
Emil
Christian,
On Wed, Mar 18, 2026 at 03:19:20PM +0100, Jens Emil Schulz Ostergaard wrote:
> Hi Conor,
>
> On Fri, 2026-03-06 at 15:20 +0000, Conor Dooley wrote:
> > On Fri, Mar 06, 2026 at 04:08:01PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > On Thu, 2026-03-05 at 18:31 +0000, Conor Dooley wrote:
> > > > On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > > On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote:
> > > > > > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote:
> > > > > > > > + properties:
> > > > > > > > + microchip,led-drive-mode:
> > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > + description: |
> > > > > > > > + Set the LED drive mode for the copper PHY associated with
> > > > > > > > + this port.
> > > > > > > > +
> > > > > > > > + 0 - LED1 and LED2 in open-drain mode
> > > > > > > > + 1 - LED1 in active drive mode (can be used for single-LED
> > > > > > > > + configurations requiring active drive)
> > > > > > > > + 2 - Reserved
> > > > > > > > + 3 - LED1 and LED2 in active drive mode
> > > > > > > > + minimum: 0
> > > > > > > > + maximum: 3
> > > > > > >
> > > > > > > I doubt the DT Maintainers will accept that. This looks a lot like a
> > > > > > > value you write into a register. How are active drive and open-drain
> > > > > > > described in other DT bindings? Is there something you can reuse?
> > > > > >
> > > > > > I had a quick look and I didn't see anything really that stood out to me
> > > > > > that would be a drop-in replacement.
> > > > > > I also tried looking in the datasheet for more information on these
> > > > > > modes, but I couldn't see anything obvious. For example, there were zero
> > > > > > hits for "drain" in either LAN9645xS or LAN9645xF datasheets.
> > > > > >
> > > > > > That said, yea you're right about DT maintainer feelings about it.
> > > > > > There's a couple things I could suggest, but I'd like to know about what
> > > > > > mode 1 means for LED2 first. If there's actually nothing similar, what
> > > > > > about representing each led with a child node and having open-drain be
> > > > > > the default with a property in the child for active-drive?
> > > > > >
> > > > > > >
> > > > > > > For 1, what happens to LED2? Not used at all?
> > > > >
> > > > > In mode 1 LED2 will be open-drain. This mode only makes sense if you have
> > > > > just 1 LED. With two LEDs mode 0 or mode 3 should be used.
> > > >
> > > > Could we then have child nodes for each led, and have a property in each
> > > > that sets the mode to either open-drain or active-drive? Or am I just
> > > > inserting complexity by asking for that?
> > >
> > > I think it sounds sensible, I will add this.
> >
> >
> > You don't need a property for each, just make one mode the default (prob
> > open-drain given it's the 0 setting, but whatever is default out of
> > reset for the part) and have the property for the other mode. Just
> > some bool property like "microchip,active-drive" or whatever.
>
>
> Based on your feedback I went with this under port properties:
>
> leds:
> patternProperties:
> '^led@[a-f0-9]+$':
> $ref: /schemas/leds/common.yaml#
>
> properties:
> reg:
> maxItems: 1
>
> microchip,active-drive:
> type: boolean
> description:
> Set the LED output to active drive mode. The default
> is open-drain.
>
> required:
> - reg
>
> unevaluatedProperties: false
>
> and then the example has
>
> port@1 {
> reg = <1>;
> phy-mode = "gmii";
> phy-handle = <&cuphy1>;
>
> leds {
> #address-cells = <1>;
> #size-cells = <0>;
>
> led@0 {
> reg = <0>;
> microchip,active-drive;
> };
>
> led@1 {
> reg = <1>;
> microchip,active-drive;
> };
> };
> }
>
> However, this does not pass dt_binding_check because we pull in $ref: dsa-port.yaml,
> which pulls in ethernet-controller.yaml.
>
> I believe the 'unevaluatedProperties: false' on LED nodes in ethernet-controller.yaml
> prevents downstream bindings from adding vendor-specific LED properties.
>
> Is the right move removing unevaluatedProperties: false from the LED node in
> ethernet-controller.yaml, or is there a preferred way to extend per-port LEDs?
The addition looks recent enough, should probably ask Christian why it
was done this way and if removing it makes sense. Christian?
Cheers,
Conor.
On Wed, Mar 18, 2026 at 05:18:42PM +0000, Conor Dooley wrote:
> Christian,
>
> On Wed, Mar 18, 2026 at 03:19:20PM +0100, Jens Emil Schulz Ostergaard wrote:
> > Hi Conor,
> >
> > On Fri, 2026-03-06 at 15:20 +0000, Conor Dooley wrote:
> > > On Fri, Mar 06, 2026 at 04:08:01PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > On Thu, 2026-03-05 at 18:31 +0000, Conor Dooley wrote:
> > > > > On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > > > On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote:
> > > > > > > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote:
> > > > > > > > > + properties:
> > > > > > > > > + microchip,led-drive-mode:
> > > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > > + description: |
> > > > > > > > > + Set the LED drive mode for the copper PHY associated with
> > > > > > > > > + this port.
> > > > > > > > > +
> > > > > > > > > + 0 - LED1 and LED2 in open-drain mode
> > > > > > > > > + 1 - LED1 in active drive mode (can be used for single-LED
> > > > > > > > > + configurations requiring active drive)
> > > > > > > > > + 2 - Reserved
> > > > > > > > > + 3 - LED1 and LED2 in active drive mode
> > > > > > > > > + minimum: 0
> > > > > > > > > + maximum: 3
> > > > > > > >
> > > > > > > > I doubt the DT Maintainers will accept that. This looks a lot like a
> > > > > > > > value you write into a register. How are active drive and open-drain
> > > > > > > > described in other DT bindings? Is there something you can reuse?
> > > > > > >
> > > > > > > I had a quick look and I didn't see anything really that stood out to me
> > > > > > > that would be a drop-in replacement.
> > > > > > > I also tried looking in the datasheet for more information on these
> > > > > > > modes, but I couldn't see anything obvious. For example, there were zero
> > > > > > > hits for "drain" in either LAN9645xS or LAN9645xF datasheets.
> > > > > > >
> > > > > > > That said, yea you're right about DT maintainer feelings about it.
> > > > > > > There's a couple things I could suggest, but I'd like to know about what
> > > > > > > mode 1 means for LED2 first. If there's actually nothing similar, what
> > > > > > > about representing each led with a child node and having open-drain be
> > > > > > > the default with a property in the child for active-drive?
> > > > > > >
> > > > > > > >
> > > > > > > > For 1, what happens to LED2? Not used at all?
> > > > > >
> > > > > > In mode 1 LED2 will be open-drain. This mode only makes sense if you have
> > > > > > just 1 LED. With two LEDs mode 0 or mode 3 should be used.
> > > > >
> > > > > Could we then have child nodes for each led, and have a property in each
> > > > > that sets the mode to either open-drain or active-drive? Or am I just
> > > > > inserting complexity by asking for that?
> > > >
> > > > I think it sounds sensible, I will add this.
> > >
> > >
> > > You don't need a property for each, just make one mode the default (prob
> > > open-drain given it's the 0 setting, but whatever is default out of
> > > reset for the part) and have the property for the other mode. Just
> > > some bool property like "microchip,active-drive" or whatever.
> >
> >
> > Based on your feedback I went with this under port properties:
> >
> > leds:
> > patternProperties:
> > '^led@[a-f0-9]+$':
> > $ref: /schemas/leds/common.yaml#
> >
> > properties:
> > reg:
> > maxItems: 1
> >
> > microchip,active-drive:
> > type: boolean
> > description:
> > Set the LED output to active drive mode. The default
> > is open-drain.
> >
> > required:
> > - reg
> >
> > unevaluatedProperties: false
> >
> > and then the example has
> >
> > port@1 {
> > reg = <1>;
> > phy-mode = "gmii";
> > phy-handle = <&cuphy1>;
> >
> > leds {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > led@0 {
> > reg = <0>;
> > microchip,active-drive;
> > };
> >
> > led@1 {
> > reg = <1>;
> > microchip,active-drive;
> > };
> > };
> > }
> >
> > However, this does not pass dt_binding_check because we pull in $ref: dsa-port.yaml,
> > which pulls in ethernet-controller.yaml.
> >
> > I believe the 'unevaluatedProperties: false' on LED nodes in ethernet-controller.yaml
> > prevents downstream bindings from adding vendor-specific LED properties.
> >
> > Is the right move removing unevaluatedProperties: false from the LED node in
> > ethernet-controller.yaml, or is there a preferred way to extend per-port LEDs?
>
> The addition looks recent enough, should probably ask Christian why it
> was done this way and if removing it makes sense. Christian?
>
Ah shit, I autopiloted into sending. Actually +CC Christian this time.
On Wed, Mar 18, 2026 at 05:20:03PM +0000, Conor Dooley wrote:
> On Wed, Mar 18, 2026 at 05:18:42PM +0000, Conor Dooley wrote:
> > Christian,
> >
> > On Wed, Mar 18, 2026 at 03:19:20PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > Hi Conor,
> > >
> > > On Fri, 2026-03-06 at 15:20 +0000, Conor Dooley wrote:
> > > > On Fri, Mar 06, 2026 at 04:08:01PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > > On Thu, 2026-03-05 at 18:31 +0000, Conor Dooley wrote:
> > > > > > On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > > > > On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote:
> > > > > > > > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote:
> > > > > > > > > > + properties:
> > > > > > > > > > + microchip,led-drive-mode:
> > > > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > > > + description: |
> > > > > > > > > > + Set the LED drive mode for the copper PHY associated with
> > > > > > > > > > + this port.
> > > > > > > > > > +
> > > > > > > > > > + 0 - LED1 and LED2 in open-drain mode
> > > > > > > > > > + 1 - LED1 in active drive mode (can be used for single-LED
> > > > > > > > > > + configurations requiring active drive)
> > > > > > > > > > + 2 - Reserved
> > > > > > > > > > + 3 - LED1 and LED2 in active drive mode
> > > > > > > > > > + minimum: 0
> > > > > > > > > > + maximum: 3
> > > > > > > > >
> > > > > > > > > I doubt the DT Maintainers will accept that. This looks a lot like a
> > > > > > > > > value you write into a register. How are active drive and open-drain
> > > > > > > > > described in other DT bindings? Is there something you can reuse?
> > > > > > > >
> > > > > > > > I had a quick look and I didn't see anything really that stood out to me
> > > > > > > > that would be a drop-in replacement.
> > > > > > > > I also tried looking in the datasheet for more information on these
> > > > > > > > modes, but I couldn't see anything obvious. For example, there were zero
> > > > > > > > hits for "drain" in either LAN9645xS or LAN9645xF datasheets.
> > > > > > > >
> > > > > > > > That said, yea you're right about DT maintainer feelings about it.
> > > > > > > > There's a couple things I could suggest, but I'd like to know about what
> > > > > > > > mode 1 means for LED2 first. If there's actually nothing similar, what
> > > > > > > > about representing each led with a child node and having open-drain be
> > > > > > > > the default with a property in the child for active-drive?
> > > > > > > >
> > > > > > > > >
> > > > > > > > > For 1, what happens to LED2? Not used at all?
> > > > > > >
> > > > > > > In mode 1 LED2 will be open-drain. This mode only makes sense if you have
> > > > > > > just 1 LED. With two LEDs mode 0 or mode 3 should be used.
> > > > > >
> > > > > > Could we then have child nodes for each led, and have a property in each
> > > > > > that sets the mode to either open-drain or active-drive? Or am I just
> > > > > > inserting complexity by asking for that?
> > > > >
> > > > > I think it sounds sensible, I will add this.
> > > >
> > > >
> > > > You don't need a property for each, just make one mode the default (prob
> > > > open-drain given it's the 0 setting, but whatever is default out of
> > > > reset for the part) and have the property for the other mode. Just
> > > > some bool property like "microchip,active-drive" or whatever.
> > >
> > >
> > > Based on your feedback I went with this under port properties:
> > >
> > > leds:
> > > patternProperties:
> > > '^led@[a-f0-9]+$':
> > > $ref: /schemas/leds/common.yaml#
> > >
> > > properties:
> > > reg:
> > > maxItems: 1
> > >
> > > microchip,active-drive:
> > > type: boolean
> > > description:
> > > Set the LED output to active drive mode. The default
> > > is open-drain.
> > >
> > > required:
> > > - reg
> > >
> > > unevaluatedProperties: false
> > >
> > > and then the example has
> > >
> > > port@1 {
> > > reg = <1>;
> > > phy-mode = "gmii";
> > > phy-handle = <&cuphy1>;
> > >
> > > leds {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > led@0 {
> > > reg = <0>;
> > > microchip,active-drive;
> > > };
> > >
> > > led@1 {
> > > reg = <1>;
> > > microchip,active-drive;
> > > };
> > > };
> > > }
> > >
> > > However, this does not pass dt_binding_check because we pull in $ref: dsa-port.yaml,
> > > which pulls in ethernet-controller.yaml.
> > >
> > > I believe the 'unevaluatedProperties: false' on LED nodes in ethernet-controller.yaml
> > > prevents downstream bindings from adding vendor-specific LED properties.
> > >
> > > Is the right move removing unevaluatedProperties: false from the LED node in
> > > ethernet-controller.yaml, or is there a preferred way to extend per-port LEDs?
> >
> > The addition looks recent enough, should probably ask Christian why it
> > was done this way and if removing it makes sense. Christian?
> >
>
> Ah shit, I autopiloted into sending. Actually +CC Christian this time.
Hi, give me some time to experiment... AFAIK with my limited info on DT,
unevaluatedProperties permits to add vendor property as long as they are
well defined. If the dt_binding_check is failing, then it's probably
because unevaluatedProperties is finding that in the expected LED node
there is extra stuff in the SCHEMA example.
But give me some time to experiment with some other SCHEMA.
--
Ansuel
On Wed, 2026-03-18 at 18:26 +0100, Christian Marangi wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Wed, Mar 18, 2026 at 05:20:03PM +0000, Conor Dooley wrote:
> > On Wed, Mar 18, 2026 at 05:18:42PM +0000, Conor Dooley wrote:
> > > Christian,
> > >
> > > On Wed, Mar 18, 2026 at 03:19:20PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > Hi Conor,
> > > >
> > > > On Fri, 2026-03-06 at 15:20 +0000, Conor Dooley wrote:
> > > > > On Fri, Mar 06, 2026 at 04:08:01PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > > > On Thu, 2026-03-05 at 18:31 +0000, Conor Dooley wrote:
> > > > > > > On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote:
> > > > > > > > On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote:
> > > > > > > > > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote:
> > > > > > > > > > > + properties:
> > > > > > > > > > > + microchip,led-drive-mode:
> > > > > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > > > > + description: |
> > > > > > > > > > > + Set the LED drive mode for the copper PHY associated with
> > > > > > > > > > > + this port.
> > > > > > > > > > > +
> > > > > > > > > > > + 0 - LED1 and LED2 in open-drain mode
> > > > > > > > > > > + 1 - LED1 in active drive mode (can be used for single-LED
> > > > > > > > > > > + configurations requiring active drive)
> > > > > > > > > > > + 2 - Reserved
> > > > > > > > > > > + 3 - LED1 and LED2 in active drive mode
> > > > > > > > > > > + minimum: 0
> > > > > > > > > > > + maximum: 3
> > > > > > > > > >
> > > > > > > > > > I doubt the DT Maintainers will accept that. This looks a lot like a
> > > > > > > > > > value you write into a register. How are active drive and open-drain
> > > > > > > > > > described in other DT bindings? Is there something you can reuse?
> > > > > > > > >
> > > > > > > > > I had a quick look and I didn't see anything really that stood out to me
> > > > > > > > > that would be a drop-in replacement.
> > > > > > > > > I also tried looking in the datasheet for more information on these
> > > > > > > > > modes, but I couldn't see anything obvious. For example, there were zero
> > > > > > > > > hits for "drain" in either LAN9645xS or LAN9645xF datasheets.
> > > > > > > > >
> > > > > > > > > That said, yea you're right about DT maintainer feelings about it.
> > > > > > > > > There's a couple things I could suggest, but I'd like to know about what
> > > > > > > > > mode 1 means for LED2 first. If there's actually nothing similar, what
> > > > > > > > > about representing each led with a child node and having open-drain be
> > > > > > > > > the default with a property in the child for active-drive?
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > For 1, what happens to LED2? Not used at all?
> > > > > > > >
> > > > > > > > In mode 1 LED2 will be open-drain. This mode only makes sense if you have
> > > > > > > > just 1 LED. With two LEDs mode 0 or mode 3 should be used.
> > > > > > >
> > > > > > > Could we then have child nodes for each led, and have a property in each
> > > > > > > that sets the mode to either open-drain or active-drive? Or am I just
> > > > > > > inserting complexity by asking for that?
> > > > > >
> > > > > > I think it sounds sensible, I will add this.
> > > > >
> > > > >
> > > > > You don't need a property for each, just make one mode the default (prob
> > > > > open-drain given it's the 0 setting, but whatever is default out of
> > > > > reset for the part) and have the property for the other mode. Just
> > > > > some bool property like "microchip,active-drive" or whatever.
> > > >
> > > >
> > > > Based on your feedback I went with this under port properties:
> > > >
> > > > leds:
> > > > patternProperties:
> > > > '^led@[a-f0-9]+$':
> > > > $ref: /schemas/leds/common.yaml#
> > > >
> > > > properties:
> > > > reg:
> > > > maxItems: 1
> > > >
> > > > microchip,active-drive:
> > > > type: boolean
> > > > description:
> > > > Set the LED output to active drive mode. The default
> > > > is open-drain.
> > > >
> > > > required:
> > > > - reg
> > > >
> > > > unevaluatedProperties: false
> > > >
> > > > and then the example has
> > > >
> > > > port@1 {
> > > > reg = <1>;
> > > > phy-mode = "gmii";
> > > > phy-handle = <&cuphy1>;
> > > >
> > > > leds {
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > led@0 {
> > > > reg = <0>;
> > > > microchip,active-drive;
> > > > };
> > > >
> > > > led@1 {
> > > > reg = <1>;
> > > > microchip,active-drive;
> > > > };
> > > > };
> > > > }
> > > >
> > > > However, this does not pass dt_binding_check because we pull in $ref: dsa-port.yaml,
> > > > which pulls in ethernet-controller.yaml.
> > > >
> > > > I believe the 'unevaluatedProperties: false' on LED nodes in ethernet-controller.yaml
> > > > prevents downstream bindings from adding vendor-specific LED properties.
> > > >
> > > > Is the right move removing unevaluatedProperties: false from the LED node in
> > > > ethernet-controller.yaml, or is there a preferred way to extend per-port LEDs?
> > >
> > > The addition looks recent enough, should probably ask Christian why it
> > > was done this way and if removing it makes sense. Christian?
> > >
> >
> > Ah shit, I autopiloted into sending. Actually +CC Christian this time.
>
> Hi, give me some time to experiment... AFAIK with my limited info on DT,
>
> unevaluatedProperties permits to add vendor property as long as they are
> well defined. If the dt_binding_check is failing, then it's probably
> because unevaluatedProperties is finding that in the expected LED node
> there is extra stuff in the SCHEMA example.
>
> But give me some time to experiment with some other SCHEMA.
>
> --
> Ansuel
Hi, I will just remove the LED handling from this series, and handle it in a
different series, once this is settled.
Thanks,
Emil
On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote: > On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote: > > > + properties: > > > + microchip,led-drive-mode: > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + description: | > > > + Set the LED drive mode for the copper PHY associated with > > > + this port. > > > + > > > + 0 - LED1 and LED2 in open-drain mode > > > + 1 - LED1 in active drive mode (can be used for single-LED > > > + configurations requiring active drive) > > > + 2 - Reserved > > > + 3 - LED1 and LED2 in active drive mode > > > + minimum: 0 > > > + maximum: 3 > > > > I doubt the DT Maintainers will accept that. This looks a lot like a > > value you write into a register. How are active drive and open-drain > > described in other DT bindings? Is there something you can reuse? > > I had a quick look and I didn't see anything really that stood out to me > that would be a drop-in replacement. > I also tried looking in the datasheet for more information on these > modes, but I couldn't see anything obvious. For example, there were zero > hits for "drain" in either LAN9645xS or LAN9645xF datasheets. > > That said, yea you're right about DT maintainer feelings about it. > There's a couple things I could suggest, but I'd like to know about what > mode 1 means for LED2 first. If there's actually nothing similar, what > about representing each led with a child node and having open-drain be > the default with a property in the child for active-drive? > I will ask design about mode 1 and LED2, and get back to you. The register model I have does not mention LED2. > > > > For 1, what happens to LED2? Not used at all? > > > > Andrew
On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote:
> +examples:
> + - |
> + soc {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-switch@0 {
> + reg = <0>;
> + compatible = "microchip,lan96459f-switch", "microchip,lan96455s-switch";
> + pinctrl-0 = <&lan9645x_leds>;
> + pinctrl-names = "default";
> +
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + label = "lan0";
Please remove labels from DSA examples. They do not constitute best
practice, and it is preferable they don't leak into device trees by
promoting them through binding examples.
It is currently recommended to name DSA (like other) network interfaces
using udev rules set up by user space. Using labels is perhaps only
tolerated for cases where the udev implementation is sub-par, like
OpenWrt.
On Tue, 2026-03-03 at 15:22 +0200, Vladimir Oltean wrote:
>
> On Tue, Mar 03, 2026 at 01:22:28PM +0100, Jens Emil Schulz Østergaard wrote:
> > +examples:
> > + - |
> > + soc {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ethernet-switch@0 {
> > + reg = <0>;
> > + compatible = "microchip,lan96459f-switch", "microchip,lan96455s-switch";
> > + pinctrl-0 = <&lan9645x_leds>;
> > + pinctrl-names = "default";
> > +
> > + ethernet-ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + label = "lan0";
>
> Please remove labels from DSA examples. They do not constitute best
> practice, and it is preferable they don't leak into device trees by
> promoting them through binding examples.
>
> It is currently recommended to name DSA (like other) network interfaces
> using udev rules set up by user space. Using labels is perhaps only
> tolerated for cases where the udev implementation is sub-par, like
> OpenWrt.
I will remove the labels in the next version.
Thanks,
Emil
© 2016 - 2026 Red Hat, Inc.