Re: [PATCH net-next 2/8] dt-bindings: net: lan9645x: add LAN9645X switch bindings

[Christian Marangi <ansuelsmth@gmail.com>]

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