RE: [net-next][PATCH v4] dt-bindings: dsa: Add lan9303 yaml

[Jerry.Ray]

>> ---
>> v3->v4:
>>  - Addressed v3 community feedback
>
>More specifically?
>

- Old lan9303.txt file is totally removed rather than containing text that
  redirects the user to the microchip,lan9303.yaml source file.
- Drop "Tree Bindings" from title
- Drop quotes from dsa.yaml reference line.
- Modified the compatible second enum to include a second string.
(( I now realize this is not what was being asked for and have made
  it a single enum with 4 items, removing the oneOf. ))
- Drop "gpio specifier for a" in reset-gpois description
- added a default: property to the reset-duration item and set it to 200.
- Drop "0" from the ethernet name.  Split the MDIO and I2C examples into
  two so that the number is no longer needed.
- Placed the reg property to be directly following the compatible string
  in the mdio node.

>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    // Ethernet switch connected via mdio to the host
>> +    ethernet {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        phy-handle = <&lan9303switch>;
>> +        phy-mode = "rmii";
>> +        fixed-link {
>> +            speed = <100>;
>> +            full-duplex;
>> +        };
>
>I see the phy-handle to the switch is inherited from the .txt dt-binding,
>but I don't understand it. The switch is an mdio_device, not a phy_device,
>so what will this do?
>
>Also, any reasonable host driver will error out if it finds a phy-handle
>and a fixed-link in its OF node. So one of phy-handle or fixed-link must
>be dropped, they are bogus.
>
>Even better, just stick to the mdio node as root and drop the DSA master
>OF node, like other DSA dt-binding examples do. You can have dangling
>phandles, so "ethernet = <&ethernet>" below is not an issue.
>

I can remove the phy-handle, but I'm trying to establish the link between
this ethernet port and port0 (the CPU port) of the lan9303.  The lan9303
acts as the phy for this ethernet port and I want to force the speed and
duplex of the link to be 100 / full-duplex.

>> +        mdio {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +            lan9303switch: switch@0 {
>> +                compatible = "smsc,lan9303-mdio";
>> +                reg = <0>;
>> +                dsa,member = <0 0>;
>
>Redundant, please remove.
>

Okay.  I can remove the "dsa,member = <0,0>;" line.

>> +                ethernet-ports {
>> +                    #address-cells = <1>;
>> +                    #size-cells = <0>;
>> +                        port@0 {
>> +                            reg = <0>;
>> +                            phy-mode = "rmii";
>
>FWIW, RMII has a MAC mode and a PHY mode. Two RMII interfaces connected
>in MAC mode to one another don't work. You'll have problems if you also
>have an RMII PHY connected to one of the xMII ports, and you describe
>phy-mode = "rmii" for both. There exists a "rev-rmii" phy-mode to denote
>an RMII interface working in PHY mode. Wonder if you should be using
>that here.
>

"rev-rmii" does make more sense.  And yes, in this configuration the rmii
port of the lan9303 is acting as the PHY end.

>> +                            ethernet = <&ethernet>;
>> +                            fixed-link {
>> +                                speed = <100>;
>> +                                full-duplex;
>> +                            };
>> +                        };
>> +                        port@1 {
>> +                            reg = <1>;
>> +                            max-speed = <100>;
>> +                            label = "lan1";
>> +                        };
>> +                        port@2 {
>> +                            reg = <2>;
>> +                            max-speed = <100>;
>> +                            label = "lan2";
>> +                        };
>> +                    };
>> +                };
>> +            };
>> +        };
>> +
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +
>> +    // Ethernet switch connected via i2c to the host
>> +    ethernet {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        phy-mode = "rmii";
>> +            speed = <100>;
>> +        fixed-link {
>> +            full-duplex;
>> +        };
>> +    };
>
>No need for this node.
>

Without this, what does the port0 entry below have to point to?
How do you establish the device tree linkage between the ethenet
MAC and the rev-rmii PHY it connects to?

>> +
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        lan9303: switch@1a {
>> +            compatible = "smsc,lan9303-i2c";
>> +            reg = <0x1a>;
>> +            ethernet-ports {
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +                port@0 {
>> +                    reg = <0>;
>> +                    phy-mode = "rmii";
>> +                    ethernet = <&ethernet>;
>> +                    fixed-link {
>> +                        speed = <100>;
>> +                        full-duplex;
>> +                    };
>> +                };
>> +                port@1 {
>> +                    reg = <1>;
>> +                    max-speed = <100>;
>> +                    label = "lan1";
>> +                };
>> +                port@2 {
>> +                    reg = <2>;
>> +                    max-speed = <100>;
>> +                    label = "lan2";
>> +                };
>> +            };
>> +        };
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5d58b55c5ae5..89055ff2838a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13386,6 +13386,14 @@ L:   netdev@xxxxxxxxxxxxxxx
>>  S:   Maintained
>>  F:   drivers/net/ethernet/microchip/lan743x_*
>>
>> +MICROCHIP LAN9303/LAN9354 ETHERNET SWITCH DRIVER
>> +M:   Jerry Ray <jerry.ray@xxxxxxxxxxxxx>
>> +M:   UNGLinuxDriver@xxxxxxxxxxxxx
>> +L:   netdev@xxxxxxxxxxxxxxx
>> +S:   Maintained
>> +F:   Documentation/devicetree/bindings/net/dsa/microchip,lan9303.yaml
>> +F:   drivers/net/dsa/lan9303*
>> +
>
>Separate patch please? Changes to the MAINTAINERS file get applied to
>the "net" tree.
>
>>  MICROCHIP LAN966X ETHERNET DRIVER
>>  M:   Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx>
>>  M:   UNGLinuxDriver@xxxxxxxxxxxxx
>> --
>> 2.25.1
>>
>

Regards,
Jerry.