| 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
| ||
|
Message-ID: <20230818134419.mcnq7d4aj74yedum@skbuf>
Date: Fri, 18 Aug 2023 16:44:19 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Linus Walleij <linus.walleij@...aro.org>
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 03:08:52PM +0200, Linus Walleij wrote:
> 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.
Yikes. I totally missed that.
> > 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.
Ohhhh, I saw the pinctrl-gmii explanation before, but I didn't understand it.
I get it now. Sorry.
>
> > 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*.
I think that as long as the delays aren't added by the other end
("PHY"), then phy-mode = "rgmii" would be the adequate mode to describe
this (even if it only has an informative purpose).
The delays, even if added by the local system, are added externally to
the MAC block (given its current bindings). Similar in that regard to
PCB delays. Which makes the device tree basically correct/consistent in
that particular aspect.
> 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 above help or did I make it even more confusing :D
It helped a lot, thank you for clarifying.
> 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'm not sure that making changes there would become necessary.
> I suppose it fully confirms the view that the phy-mode in
> the ethernet controller is 100% placebo though!
Which is what it should be, IMO.
Powered by blists - more mailing lists