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

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:

quoted

On Wed, Mar 18, 2026 at 05:18:42PM +0000, Conor Dooley wrote:

quoted

Christian,

On Wed, Mar 18, 2026 at 03:19:20PM +0100, Jens Emil Schulz Ostergaard wrote:

quoted

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: 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?

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?

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