netdev - Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x

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]

Date:   Tue, 3 Aug 2021 22:24:27 +0530
From:   Prasanna Vengateshan <prasanna.vengateshan@...rochip.com>
To:     Vladimir Oltean <olteanv@...il.com>, Andrew Lunn <andrew@...n.ch>
CC:     <netdev@...r.kernel.org>, <robh+dt@...nel.org>,
        <UNGLinuxDriver@...rochip.com>, <Woojung.Huh@...rochip.com>,
        <hkallweit1@...il.com>, <linux@...linux.org.uk>,
        <davem@...emloft.net>, <kuba@...nel.org>,
        <linux-kernel@...r.kernel.org>, <vivien.didelot@...il.com>,
        <f.fainelli@...il.com>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support
 for microchip lan937x

On Mon, 2021-08-02 at 16:59 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Mon, Aug 02, 2021 at 03:13:01PM +0200, Andrew Lunn wrote:
> > In general, the MAC does nothing, and passes the value to the PHY. The
> > PHY inserts delays as requested. To address Vladimir point,
> > PHY_INTERFACE_MODE_RGMII_TXID would mean the PHY adds delay in the TX
> > direction, and assumes the RX delay comes from somewhere else,
> > probably the PCB.
> 
> For the PHY, that is the only portion where things are clear.
> 
> > I only recommend the MAC adds delays when the PHY cannot, or there is
> > no PHY, e.g. SoC to switch, or switch to switch link. There are a few
> > MAC drivers that do add delays, mostly because that is how the vendor
> > crap tree does it.
> > 
> > So as i said, what you propose is O.K, it follows this general rule of
> > thumb.
> 
> The "rule of thumb" for a MAC driver is actually applied in reverse by
> most MAC drivers compared to what Russell described should be happening.
> For example, mv88e6xxx_port_set_rgmii_delay():
> 
>         switch (mode) {
>         case PHY_INTERFACE_MODE_RGMII_RXID:
>                 reg |= MV88E6XXX_PORT_MAC_CTL_RGMII_DELAY_RXCLK;
> 
> The mv88e6xxx is a MAC, so when it has a phy-mode = "rgmii-rxid", it
> should assume it is connected to a link partner (PHY or otherwise) that
> has applied the RXCLK delay already. So it should only be concerned with
> the TXCLK delay. That is my point. I am just trying to lay out the
> points to Prasanna that would make a sane system going forward. I am not
> sure that we actually have an in-tree driver that is sane in that
> regard.
> 
> That discussion, and Russell's point, was here, btw:
> https://patchwork.ozlabs.org/project/netdev/patch/20200616074955.GA9092@laureti-dev/#2461123

Thanks Vladimir & Andrew for the right pointers and info. The thread talks about
"rgmii-*" are going to be applied by the PHY only as per the doc. For fixed-
link, MAC needs to add the delay. This fixed-link can be No-PHY or MAC-MAC or
MAC to in-accessible PHY. In such case, i am not convinced in using rgmii-tx-
delay-ps & rgmii-rx-delay-ps on the MAC side and apply delay. I still think
proposed code in earlier mail thread should still be okay.