On Sat, 2021-01-30 at 04:02 +0200, Vladimir Oltean wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > Thanks for your time on reviewing the patch series. > On Thu, Jan 28, 2021 at 12:11:05PM +0530, Prasanna Vengateshan wrote: > > + spi-max-frequency: > > + maximum: 50000000 > > And it actually works at 50 MHz? Cool. Yes. > > > + > > + reset-gpios: > > + description: Optional gpio specifier for a reset line > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/gpio/gpio.h> > > + > > + //Ethernet switch connected via spi to the host, CPU port > > wired to eth1 > > + eth1 { > > So if you do bother to add the DSA master in the example, can this be > ð1 so that we could associate with the phandle below? Sure. > > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + fixed-link { > > + speed = <1000>; > > + full-duplex; > > + }; > > + }; > > + > > + spi1 { > > Is this a label or a node name? spi1 or spi@1? This is a label. > > > + #address-cells = <1>; > > + #size-cells = <0>; > > + pinctrl-0 = <&pinctrl_spi_ksz>; > > + cs-gpios = <0>, <0>, <0>, <&pioC 28 0>; > > + id = <1>; > > I know this is the SPI controller and thus mostly irrelevant, but > what > is "id = <1>"? id is not needed, i will remove it. > > > + > > + lan9374: switch@0 { > > + compatible = "microchip,lan9374"; > > + reg = <0>; > > + > > + spi-max-frequency = <44000000>; > > + > > + ethernet-ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + port@0 { > > + reg = <0>; > > + label = "lan1"; > > + }; > > + port@1 { > > + reg = <1>; > > + label = "lan2"; > > + }; > > + port@2 { > > + reg = <7>; > > reg should match node index (port@2), here and everywhere below. As > for > the net device labels, I'm not sure if the mismatch is deliberate > there. reg & port node indexes are different here because to match with the physical to logical port mapping done in the LAN9374. I realized that the description is missing and that is to be added. However, should reg & port node index match for the net dev? If it should be the same, then the same can be acheived by renaming a label (lanx) as well. > > > + label = "lan3"; > > + }; > > + port@3 { > > + reg = <2>; > > + label = "lan4"; > > + }; > > + port@4 { > > + reg = <6>; > > + label = "lan5"; > > + }; > > + port@5 { > > + reg = <3>; > > + label = "lan6"; > > + }; > > + port@6 { > > + reg = <4>; > > + label = "cpu"; > > label for CPU port is not needed/used. Sure, will remove it. > > > + ethernet = <ð1>; > > + fixed-link { > > + speed = <1000>; > > + full-duplex; > > + }; > > + }; > > + port@7 { > > + reg = <5>; > > + label = "lan7"; > > + fixed-link { > > + speed = <1000>; > > + full-duplex; > > + }; > > + }; > > + }; > > + }; > > + };