netdev - Re: [PATCH] net: thunderx: add support for rgmii internal delay

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: <20171214084525.GA19186@lunn.ch>
Date:   Thu, 14 Dec 2017 09:45:25 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Tim Harvey <tharvey@...eworks.com>
Cc:     Sunil Goutham <sgoutham@...ium.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: thunderx: add support for rgmii internal delay

On Wed, Dec 13, 2017 at 03:28:33PM -0800, Tim Harvey wrote:
> On Wed, Dec 13, 2017 at 3:10 AM, Andrew Lunn <andrew@...n.ch> wrote:
> >> +void xcv_init_hw(int phy_mode)
> >>  {
> >>       u64  cfg;
> >>
> >> @@ -81,12 +81,31 @@ void xcv_init_hw(void)
> >>       /* Wait for DLL to lock */
> >>       msleep(1);
> >>
> >> -     /* Configure DLL - enable or bypass
> >> -      * TX no bypass, RX bypass
> >> -      */
> >> +     /* enable/bypass DLL providing MAC based internal TX/RX delays */
> >>       cfg = readq_relaxed(xcv->reg_base + XCV_DLL_CTL);
> >> -     cfg &= ~0xFF03;
> >> -     cfg |= CLKRX_BYP;
> >> +     cfg &= ~0xffff00;
> >> +     switch (phy_mode) {
> >> +     /* RX and TX delays are added by the MAC */
> >> +     case PHY_INTERFACE_MODE_RGMII:
> >> +             break;
> >> +     /* internal RX and TX delays provided by the PHY */
> >> +     case PHY_INTERFACE_MODE_RGMII_ID:
> >> +             cfg |= CLKRX_BYP;
> >> +             cfg |= CLKTX_BYP;
> >> +             break;
> >> +     /* internal RX delay provided by the PHY, the MAC
> >> +      * should not add an RX delay in this case
> >> +      */
> >> +     case PHY_INTERFACE_MODE_RGMII_RXID:
> >> +             cfg |= CLKRX_BYP;
> >> +             break;
> >> +     /* internal TX delay provided by the PHY, the MAC
> >> +      * should not add an TX delay in this case
> >> +      */
> >> +     case PHY_INTERFACE_MODE_RGMII_TXID:
> >> +             cfg |= CLKRX_BYP;
> >> +             break;
> >> +     }
> >
> > Hi Tim
> >
> > This i don't get. Normally, you leave the PHY to handle delays, if
> > needed. The MAC should not add any. Here you seem to assume a delay is
> > always needed, and if the PHY is not providing it, the MAC should.
> >
> >        Andrew
> 
> Andrew,
> 
> The thunder RGX inserts a delay via an on-board DLL. The 'bypass'
> register will bypass this DLL and not insert a delay from the MAC
> side. By default out of reset CLKTX_BYP=1 causing the RGX transmit
> interface to not introduce a delay and CLKRX_BYP=0 causing the RGX
> receive interface to introduce a delay.

Hi Tim

So the MAC by default is doing PHY_INTERFACE_MODE_RGMII_RXID.  And it
calls phy_connect_direct() passing PHY_INTERFACE_MODE_RGMII. It does
not get anything from device tree. So it looks like we have a chance
to clean this up.

So the correct thing to do is set the MAC to PHY_INTERFACE_MODE_RGMII,
i.e. no delays. By default call phy_connect_direct()
PHY_INTERFACE_MODE_RGMII_RXID. That should give you the same behaviour
as today.

Then add code to look in device tree, to find a per board setting. In
your case, you want PHY_INTERFACE_MODE_RGMII_ID. And make sure the PHY
driver respects the value passed.

       Andrew