lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACRpkdYf-VwCUFigjb1ZHJfieXkxqLSwVmG_S-SKJQY1bciCHA@mail.gmail.com>
Date: Fri, 18 Aug 2023 15:08:52 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: Vladimir Oltean <olteanv@...il.com>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>, Andrew Lunn <andrew@...n.ch>, 
	Heiner Kallweit <hkallweit1@...il.com>, Florian Fainelli <f.fainelli@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: mark parsed interface mode for legacy
 switch drivers

On Fri, Aug 18, 2023 at 1:40 PM Vladimir Oltean <olteanv@...il.com> wrote:

> Though, to have more confidence in the validity of the change, I'd need
> the phy-mode of the &gmac0 node from arch/arm/boot/dts/gemini/gemini-dlink-dir-685.dts,
> and I'm not seeing it.

The assignment of the gmac0 label is in the top-level DTSI:

(...)
                ethernet: ethernet@...00000 {
                        compatible = "cortina,gemini-ethernet";
(...)
                        gmac0: ethernet-port@0 {
                                compatible = "cortina,gemini-ethernet-port";
(...)

Then in the DIR-685 overlay DTS It looks like this:

                ethernet@...00000 {
                        status = "okay";

                        ethernet-port@0 {
                                phy-mode = "rgmii";
                                fixed-link {
                                        speed = <1000>;
                                        full-duplex;
                                        pause;
                                };
                        };
                        ethernet-port@1 {
                                /* Not used in this platform */
                        };
                };

Russell pointed out that this style with overlays of nodes are
confusing. I agree. If I did it today I would use &gmac0 to
configure it. There is a bit of development history here.

> Looking at its driver (drivers/net/ethernet/cortina/gemini.c), I don't
> see any handling of RGMII delays, and it accepts any RGMII phy-mode.

The handling of the delays exist and is done orthogonally, from
the pin controller, which can assign skewing to pins on the chip.

There are also SoCs that will do clock skewing in the external
bus controller, from a driver in drivers/bus such as what
drivers/bus/intel-ixp4xx-eb.c is doing for IXP4xx. So there
are even several different places where clock skewing/transmit
delays can be handled.

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.

> So, if neither the Ethernet controller nor the RTL8366RB switch are
> adding RGMII delays,

Actually the SoC is. When the ethernet is probed, it asks for its
default pin configuration, and it will applied from the device tree
from the above node. It's just that the registers to control them
are not in the ethernet hardware block, but in the pin controller.

> it becomes plausible that there are skews added
> through PCB traces.

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.

> 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

If guess I could imagine the delays being configured directly
from the ethernet driver if that is highly desired. For the IXP4xx
ATA controller (that has similar needs to a network phy) I just
shortcut the whole "this goes into the external memory
controller" and look up the address space for the delay
settings and control these settings directly from the driver.
It's not very encapsulated but sometimes the world out there
is ugly, and the ATA drivers really want to control this
see drivers/ata/pata_ixp4xx_cf.c,
ixp4xx_set_8bit_timing() etc.

To do delay set-up fully from the ethernet controller, for Gemini,
I would just pull this register range out from the pin controller,
remove the pin control stuff in the device tree, and poke it
directly from the ethernet driver. It can be done for sure.
It's just two different ways to skin the cat.

I suppose it fully confirms the view that the phy-mode in
the ethernet controller is 100% placebo though!

Just my €0.01

Yours,
Linus Walleij

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ