| Message ID | 20190331174232.22060-18-olteanv@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | NXP SJA1105 DSA driver | expand |
> +Optional properties: > + > +- sja1105,mac-mode, sja1105,phy-mode: Boolean properties that can be assigned > + under each port node that is MII or RMII (has no effect for RGMII). By > + default (unless otherwise specified) a port is configured as MAC if it is > + driving a PHY (phy-handle is present) or as PHY if it is PHY-less (fixed-link > + specified, presumably because it is connected to a MAC). These properties > + are required in the case where SJA1105 ports are at both ends of an MII/RMII > + PHY-less setup. One end would need to have sja1105,mac-mode, while the other > + sja1105,phy-mode. Hi Vladimir phy-mode has a well known meaning, indicating mii, gmii, sqmii, rmii, rxaud, etc. The meaning here is quite different. To maybe avoid confusion, could we flip the name around? sja1105,mode-mac and sja1105,mode-phy? > + port@4 { > + /* Internal port connected to eth2 */ > + ethernet = <&enet2>; > + phy-mode = "rgmii"; > + reg = <4>; > + /* Implicit "sja1105,phy-mode;" */ > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > + }; > + }; > + }; > +}; > +&enet2 { > + phy-connection-type = "rgmii"; You don't see this used much, phy-mode is preferred. Do you have a reason for this? Also, you have the MAC using RGMII and the port using RGMII. Neither is inserting delays. That implies the delays are added by the track layout of the PCB. It would be good to add a comment about this. Anybody copying this using a design without the delays via PCB are probably going to get it wrong to start with and not realise they need to change one end to RGMII-ID. Thanks Andrew
On 4/3/19 12:38 AM, Andrew Lunn wrote: >> +Optional properties: >> + >> +- sja1105,mac-mode, sja1105,phy-mode: Boolean properties that can be assigned >> + under each port node that is MII or RMII (has no effect for RGMII). By >> + default (unless otherwise specified) a port is configured as MAC if it is >> + driving a PHY (phy-handle is present) or as PHY if it is PHY-less (fixed-link >> + specified, presumably because it is connected to a MAC). These properties >> + are required in the case where SJA1105 ports are at both ends of an MII/RMII >> + PHY-less setup. One end would need to have sja1105,mac-mode, while the other >> + sja1105,phy-mode. > > Hi Vladimir > > phy-mode has a well known meaning, indicating mii, gmii, sqmii, rmii, > rxaud, etc. > > The meaning here is quite different. To maybe avoid confusion, could > we flip the name around? > > sja1105,mode-mac and sja1105,mode-phy? > Hi Andrew, I agree the clash of words is confusing. Which one of "sja1105,mode-phy", "sja1105,type-phy", "sja1105,role-phy" sounds easier to digest? >> + port@4 { >> + /* Internal port connected to eth2 */ >> + ethernet = <&enet2>; >> + phy-mode = "rgmii"; >> + reg = <4>; >> + /* Implicit "sja1105,phy-mode;" */ >> + fixed-link { >> + speed = <1000>; >> + full-duplex; >> + }; >> + }; >> + }; >> + }; >> +}; > > >> +&enet2 { >> + phy-connection-type = "rgmii"; > > You don't see this used much, phy-mode is preferred. Do you have a > reason for this? No particular reason for using phy-connection-type. It is just a copy-pasta from the (not yet submitted) ls1021a-tsn.dts that I'm using to test the driver on. The other LS1021A DTS files were already using this notation, so that's how it got there. But the gianfar driver works just as well with phy-mode, so maybe I will send another patch for converting the existing notations when I submit the new board DTS too. > > Also, you have the MAC using RGMII and the port using RGMII. Neither > is inserting delays. That implies the delays are added by the track > layout of the PCB. It would be good to add a comment about > this. Anybody copying this using a design without the delays via PCB > are probably going to get it wrong to start with and not realise they > need to change one end to RGMII-ID. > You've hit the nail on the head with this excellent comment. So the second-generation switches (P/Q/R/S) have addressed this hardware limitation and do provide some tunable delay lines for RGMII. But on the LS1021A-TSN, which is using a first-generation T chip, the RGMII delays for correct sample/hold times are obtained through ~50cm copper wires, since the LS1021 cannot apply them internally either. > https://www.nxp.com/docs/en/data-sheet/SJA1105.pdf > > Section 6.2.3 RGMII signaling and encoding > > Note that RGMII requires an external delay of between 1.5 ns and 2 ns > on TXC and RXC. > > So it sounds like the switch only supports PHY_INTERFACE_MODE_RGMII. > > If the port is in MAC mode, you should pass this phy-mode to the PHY > when you connect to it. The PHY can then add the delay if needed. > > If however, the port is in PHY mode, and it is asked to do RGMII other > than PHY_INTERFACE_MODE_RGMII, you should report an error. It cannot > do it. I expect that next week I will be able to get a reworked LS1021A-TSN board with a second-generation switch, and the delay wires removed. Using that, I can at least add support for the tunable delay lines in a fashion similar to what you're suggesting. But since you brought this up, let me point out some complications I see: * I think the way this works is that phy_attach_direct populates phydev->interface based on the MAC's DT bindings. Then the PHY driver is supposed to apply internal delays based on that. But this shadows the MAC's own ability to add internal delays based on the same DT bindings. You told me to pass the internal delay settings to the PHY if "sja1105,mode-mac" is (implicitly or explicitly) specified, or otherwise ("sja1105,mode-phy"), take the internal delay settings and apply them myself (if the switch revision supports this). But in the latter case, should I mask off the RGMII_*ID modes and convert everything to RGMII when doing the of_phy_connect (to ensure that at the other end nobody will add the internal delays twice)? I know the theory but I have never actually seen a MAC driver apply the internal delay settings. And even in my case I'm a bit surprised that there isn't a more generic mechanism to specify this and that I have to rely on custom DT bindings. * The 2 groups of devices (E, T, P, Q) and (R, S) are pin-compatible with one another, and in principle hot-swappable. Initially I had explicit "compatible" properties for each device model, but with the code for device_id detection, this became a bit redundant so I just made everything "nxp,sja1105". But with the different RGMII internal delay capabilities, and later on with SGMII support (R, S), some differences at the DT level will become apparent. For example my T -> Q swap on the LS1021A-TSN board will require a DT change for the internal delay settings. How should I handle the "compatible" property for this driver? > Thanks > Andrew > Thank you, -Vladimir
diff --git a/Documentation/devicetree/bindings/net/dsa/sja1105.txt b/Documentation/devicetree/bindings/net/dsa/sja1105.txt new file mode 100644 index 000000000000..2c82b6fc37e3 --- /dev/null +++ b/Documentation/devicetree/bindings/net/dsa/sja1105.txt @@ -0,0 +1,123 @@ +NXP SJA1105 switch driver +========================= + +Required properties: + +- compatible: Must be "nxp,sja1105". Device ID identification (one of + E/T/P/Q/R/S) is performed by driver at probe time. Swapping pin-compatible + parts is possible with no DTS change. + +Optional properties: + +- sja1105,mac-mode, sja1105,phy-mode: Boolean properties that can be assigned + under each port node that is MII or RMII (has no effect for RGMII). By + default (unless otherwise specified) a port is configured as MAC if it is + driving a PHY (phy-handle is present) or as PHY if it is PHY-less (fixed-link + specified, presumably because it is connected to a MAC). These properties + are required in the case where SJA1105 ports are at both ends of an MII/RMII + PHY-less setup. One end would need to have sja1105,mac-mode, while the other + sja1105,phy-mode. + +See Documentation/devicetree/bindings/net/dsa/dsa.txt for the list of standard +DSA required and optional properties. + +Other observations: + +The SJA1105 SPI interface requires a CS-to-CLK time (t2 in UM10944) of at least +one half of t_CLK. At an SPI frequency of 1MHz, this means a minimum +cs_sck_delay of 500ns. Ensuring that this SPI timing requirement is observed +depends on the SPI bus master driver. + +Example: + +Ethernet switch connected via SPI to the host, CPU port wired to eth0: + +arch/arm/boot/dts/ls1021a-tsn.dts: + +/* SPI controller of the LS1021 */ +&dspi0 { + sja1105@1 { + reg = <0x1>; + #address-cells = <1>; + #size-cells = <0>; + compatible = "nxp,sja1105"; + spi-max-frequency = <4000000>; + fsl,spi-cs-sck-delay = <1000>; + fsl,spi-sck-cs-delay = <1000>; + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + /* ETH5 written on chassis */ + label = "swp5"; + phy-handle = <&rgmii_phy6>; + phy-mode = "rgmii"; + reg = <0>; + /* Implicit "sja1105,mac-mode;" */ + }; + port@1 { + /* ETH2 written on chassis */ + label = "swp2"; + phy-handle = <&rgmii_phy3>; + phy-mode = "rgmii"; + reg = <1>; + /* Implicit "sja1105,mac-mode;" */ + }; + port@2 { + /* ETH3 written on chassis */ + label = "swp3"; + phy-handle = <&rgmii_phy4>; + phy-mode = "rgmii"; + reg = <2>; + /* Implicit "sja1105,mac-mode;" */ + }; + port@3 { + /* ETH4 written on chassis */ + phy-handle = <&rgmii_phy5>; + label = "swp4"; + phy-mode = "rgmii"; + reg = <3>; + /* Implicit "sja1105,mac-mode;" */ + }; + port@4 { + /* Internal port connected to eth2 */ + ethernet = <&enet2>; + phy-mode = "rgmii"; + reg = <4>; + /* Implicit "sja1105,phy-mode;" */ + fixed-link { + speed = <1000>; + full-duplex; + }; + }; + }; + }; +}; + +/* MDIO controller of the LS1021 */ +&mdio0 { + /* BCM5464 */ + rgmii_phy3: ethernet-phy@3 { + reg = <0x3>; + }; + rgmii_phy4: ethernet-phy@4 { + reg = <0x4>; + }; + rgmii_phy5: ethernet-phy@5 { + reg = <0x5>; + }; + rgmii_phy6: ethernet-phy@6 { + reg = <0x6>; + }; +}; + +/* Ethernet master port of the LS1021 */ +&enet2 { + phy-connection-type = "rgmii"; + status = "ok"; + fixed-link { + speed = <1000>; + full-duplex; + }; +}; +