Re: [PATCH net-next] net: dsa: mark parsed interface mode for legacy switch drivers

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Fri, Aug 18, 2023 at 03:08:52PM +0200, Linus Walleij wrote:
> For DIR-685 is looks like this:
> 
>                                 pinctrl-gmii {
>                                         mux {
>                                                 function = "gmii";
>                                                 groups = "gmii_gmac0_grp";
>                                         };
>                                         conf0 {
>                                                 pins = "V8 GMAC0
> RXDV", "T10 GMAC1 RXDV",
>                                                      "Y7 GMAC0 RXC",
> "Y11 GMAC1 RXC",
>                                                      "T8 GMAC0 TXEN",
> "W11 GMAC1 TXEN",
>                                                      "U8 GMAC0 TXC",
> "V11 GMAC1 TXC",
>                                                      "W8 GMAC0 RXD0",
> "V9 GMAC0 RXD1",
>                                                      "Y8 GMAC0 RXD2",
> "U9 GMAC0 RXD3",
>                                                      "T7 GMAC0 TXD0",
> "U6 GMAC0 TXD1",
>                                                      "V7 GMAC0 TXD2",
> "U7 GMAC0 TXD3",
>                                                      "Y12 GMAC1 RXD0",
> "V12 GMAC1 RXD1",
>                                                      "T11 GMAC1 RXD2",
> "W12 GMAC1 RXD3",
>                                                      "U10 GMAC1 TXD0",
> "Y10 GMAC1 TXD1",
>                                                      "W10 GMAC1 TXD2",
> "T9 GMAC1 TXD3";
>                                                 skew-delay = <7>;
>                                         };
>                                         /* Set up drive strength on
> GMAC0 to 16 mA */
>                                         conf1 {
>                                                 groups = "gmii_gmac0_grp";
>                                                 drive-strength = <16>;
>                                         };
>                                 };
> 
> So skew-delay of 7 steps, each step is ~0.2ns so all pins have a delay
> of 7*0.2 = 1.4ns.

If I read this correctly, then isn't this 1.4ns delay added not only
to the RXD and TXD signals, but also RXC and TXC - meaning that although
there is a delay, the effect is that (e.g.) the relative delay between
TXC and TXD is zero?

> In the DIR-685 example I think it is *both* (yeah...) because it
> makes no sense that all delays are 1.4ns. I think the PCB
> routing influence it *too*.
> 
> However the D-Link DNS-313 makes more elaborate use of
> these settings:
> 
>                                 pinctrl-gmii {
>                                         mux {
>                                                 function = "gmii";
>                                                 groups = "gmii_gmac0_grp";
>                                         };
>                                         /*
>                                          * In the vendor Linux tree,
> these values are set for the C3
>                                          * version of the SL3512 ASIC
> with the comment "benson suggest"
>                                          */
>                                         conf0 {
>                                                 pins = "R8 GMAC0
> RXDV", "U11 GMAC1 RXDV";
>                                                 skew-delay = <0>;
>                                         };
>                                         conf1 {
>                                                 pins = "T8 GMAC0 RXC";
>                                                 skew-delay = <10>;
>                                         };
>                                         conf2 {
>                                                 pins = "T11 GMAC1 RXC";
>                                                 skew-delay = <15>;
>                                         };
>                                         conf3 {
>                                                 pins = "P8 GMAC0
> TXEN", "V11 GMAC1 TXEN";
>                                                 skew-delay = <7>;
>                                         };
>                                         conf4 {
>                                                 pins = "V7 GMAC0 TXC",
> "P10 GMAC1 TXC";
>                                                 skew-delay = <10>;
>                                         };
>                                         conf5 {
>                                                 /* The data lines all
> have default skew */
>                                                 pins = "U8 GMAC0
> RXD0", "V8 GMAC0 RXD1",
>                                                        "P9 GMAC0
> RXD2", "R9 GMAC0 RXD3",
>                                                        "R11 GMAC1
> RXD0", "P11 GMAC1 RXD1",
>                                                        "V12 GMAC1
> RXD2", "U12 GMAC1 RXD3",
>                                                        "R10 GMAC1
> TXD0", "T10 GMAC1 TXD1",
>                                                        "U10 GMAC1
> TXD2", "V10 GMAC1 TXD3";
>                                                 skew-delay = <7>;
>                                         };
>                                         conf6 {
>                                                 pins = "U7 GMAC0
> TXD0", "T7 GMAC0 TXD1",
>                                                        "R7 GMAC0
> TXD2", "P7 GMAC0 TXD3";
>                                                 skew-delay = <5>;
>                                         };
>                                         /* Set up drive strength on
> GMAC0 to 16 mA */
>                                         conf7 {
>                                                 groups = "gmii_gmac0_grp";
>                                                 drive-strength = <16>;
>                                         };
>                                 };
> 
> So there are definitely systems doing this.

So here, the skews are:
- GMAC0 RXD skewed by 7 = 1.4ns, and GMAC0 RXC by 10, making 2ns.
	Relative skew = 0.6ns.

- GMAC0 TXD by 5 = 1.0ns, and GMAC0 TXD by 10, making 2ns.
	Relative skew = 1ns.

I think this suggests there's additional skew by the PCB traces to make
up to the required 1.5 to 2ns skew required by RGMII.

> > In that case, the current phy-mode = "rgmii" would
> > be the correct choice, and changing that would be invalid. Some more
> > documentary work would be needed.
> 
> Does the aboe help or did I make it even more confusing :D

As far as I'm concerned, I think I have an overall picture of what is
going on here - it's whether anyone else agrees with that!

Going back to the 685, the skews for the datalines and clocks are:

	gmac0                          rtl8366rb
	pinctrl ----- pcb traces ----- pinstrapped
RXC	1.4ns         unknown          unknown
RXD*	1.4ns         unknown          unknown
TXC	1.4ns         unknown          unknown
TXD*	1.4ns         unknown          unknown

In the case of 313:

	gmac0                          PHY
	pinctrl ----- pcb traces ----- phy0
RXC	2.0ns         unknown          0ns	(PHY 0ns due to using
RXD*	1.4ns         unknown          0ns	 phy-mode = "rgmii" on)
TXC	2.0ns         unknown          0ns	 gmac0's node.)
TXD*	1.0ns         unknown          0ns


-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!