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: <CAJ+vNU3C2KWUBMU_-kMABwb8wfox_xV_3+9bivi=Adpd_vXCDg@mail.gmail.com>
Date:   Wed, 13 Dec 2017 15:28:33 -0800
From:   Tim Harvey <tharvey@...eworks.com>
To:     Andrew Lunn <andrew@...n.ch>
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 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.

The current code assumes the opposite setting CLKRX_BYP and clearing
CLKTX_BYP such that the RGX interface introduces a TX delay but not RX
which would be appropriate for
PHY_INTERFACE_MODE_RGMII_RXID/rgmii-txid (right, or is my logic there
backwards?). I may have my commit msg wrong in this case.

At any rate, I've got a board where the phy provides both TX/RX delay
and thus I don't want the RGX to insert any delays which means I need
to set both CLKRX_BYP and CLKTX_BYP which the driver currently doesn't
support.

Tim