netdev - Re: [PATCH v2 net-next 18/22] net: dsa: sja1105: Error out if RGMII delays are requested in DT

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d9826cea-1e7f-3ae7-4ff9-0e7d644bc743@gmail.com>
Date:   Tue, 9 Apr 2019 19:15:09 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>, vivien.didelot@...il.com,
        andrew@...n.ch, davem@...emloft.net
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        georg.waibel@...sor-technik.de
Subject: Re: [PATCH v2 net-next 18/22] net: dsa: sja1105: Error out if RGMII
 delays are requested in DT



On 4/9/2019 5:56 PM, Vladimir Oltean wrote:
> Documentation/devicetree/bindings/net/ethernet.txt is confusing because
> it says what the MAC should not do, but not what it *should* do:
> 
>   * "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC
>      should not add an RX delay in this case)
> 
> The gap in semantics is threefold:
> 1. Is it illegal for the MAC to apply the Rx internal delay by itself,
>    and simplify the phy_mode (mask off "rgmii-rxid" into "rgmii") before
>    passing it to of_phy_connect? The documentation would suggest yes.

I would agree with that statement.

> 1. For "rgmii-rxid", while the situation with the Rx clock skew is more
>    or less clear (needs to be added by the PHY), what should the MAC
>    driver do about the Tx delays? Is it an implicit wild card for the
>    MAC to apply delays in the Tx direction if it can? What if those were
>    already added as serpentine PCB traces, how could that be made more
>    obvious through DT bindings so that the MAC doesn't attempt to add
>    them twice and again potentially break the link?

I would say it can be either the MAC adding the TX delay, or the PCB
traces doing that, in that case you would have to change the property to
"rgmii-id" to account for that. Which is still ambiguous I agree...

> 3. If the interface is a fixed-link and therefore the PHY object is
>    fixed (a purely software entity that obviously cannot add clock
>    skew), what is the meaning of the above property?

fixed-link really should denote a MAC to MAC connection so if you have
"rgmii-id" on one side, you would expect "rgmii" on the other side
(unless PCB traces account for delays, grrr).

> 
> So an interpretation of the RGMII bindings was chosen that hopefully
> does not contradict their intention but also makes them more applied.
> The SJA1105 driver understands to act upon "rgmii-*id" phy-mode bindings
> if the port is in the PHY role (either explicitly, or if it is a
> fixed-link). Otherwise it always passes the duty of setting up delays to
> the PHY driver.
> 
> The error behavior that this patch adds is required on SJA1105E/T where
> the MAC really cannot apply internal delays. If the other end of the
> fixed-link cannot apply RGMII delays either (this would be specified
> through its own DT bindings), then the situation requires PCB delays.
> 
> For SJA1105P/Q/R/S, this is however hardware supported and the error is
> thus only temporary. I created a stub function pointer for configuring
> delays per-port on RXC and TXC, and will implement it when I have access
> to a board with this hardware setup.
> 
> Meanwhile do not allow the user to select an invalid configuration.
> 
> Signed-off-by: Vladimir Oltean <olteanv@...il.com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
-- 
Florian