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

[Jens Emil Schulz Ostergaard <jensemil.schulzostergaard@microchip.com>]

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