Re: [PATCH net-next 2/8] dt-bindings: net: lan9645x: add LAN9645X switch bindings
flat view
From: Conor Dooley <conor@kernel.org>
Date: 2026-03-18 17:18:48
Also in:
linux-devicetree, lkml
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:quoted
On Fri, Mar 06, 2026 at 04:08:01PM +0100, Jens Emil Schulz Ostergaard wrote:quoted
On Thu, 2026-03-05 at 18:31 +0000, Conor Dooley wrote:quoted
On Thu, Mar 05, 2026 at 01:57:37PM +0100, Jens Emil Schulz Ostergaard wrote:quoted
On Tue, 2026-03-03 at 19:04 +0000, Conor Dooley wrote:quoted
On Tue, Mar 03, 2026 at 03:18:45PM +0100, Andrew Lunn wrote:quoted
quoted
+ 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: 3I 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?quoted
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.
Attachments
- signature.asc [application/pgp-signature] 228 bytes