netdev - Re: [PATCH v2 1/4] net: phy: at803x: Document RGMII RX and TX clock delay issues

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:   Fri, 21 Jul 2017 16:36:41 +0200
From:   Marc Gonzalez <marc_gonzalez@...madesigns.com>
To:     Timur Tabi <timur@...eaurora.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>, Mans Rullgard <mans@...sr.com>,
        Martin Blumenstingl <martin.blumenstingl@...il.com>,
        Grygorii Strashko <grygorii.strashko@...com>,
        Fabio Estevam <fabio.estevam@....com>,
        Zefir Kurtisi <zefir.kurtisi@...atec.com>,
        "Daniel Mack" <zonque@...il.com>
CC:     netdev <netdev@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "David S. Miller" <davem@...emloft.net>,
        Thibaud Cornic <thibaud_cornic@...madesigns.com>,
        Mason <slash.tmp@...e.fr>
Subject: Re: [PATCH v2 1/4] net: phy: at803x: Document RGMII RX and TX clock
 delay issues

On 21/07/2017 16:06, Timur Tabi wrote:

> On 7/21/17 8:29 AM, Marc Gonzalez wrote:
>
>> I don't understand what you're saying.
>>
>> It is a correct observation that the code enabling
>> RGMII RX clock delay is a NOP, since that bit will
>> always be set at that point.
>>
>> The spec for the 8035 (I haven't checked for 8030 and 8031,
>> is that what you meant by "other systems"?) states that
>> Sel_clk125m_dsp, which is described as:
>> "Control bit for rgmii interface rx clock delay"
>> is 1 after HW reset, 1 after SW reset.
>>
>> So my statement "RX clock delay is enabled at reset"
>> is universally true. It's not just on some systems.
> 
> Ok, taken out of context, the comment doesn't really explain why the 
> code is the way it is.  I'm not really happy about the word "assumes". 

If a HW setting defaults to 0 at reset, and some init is called
right after reset, then you know the setting's value is 0.
If you need that value to be 1, all you need is a function
setting it to 1. This is the current situation.

Commit 2e5f9f281ee8 assumes 0 at reset, and provides a function
setting the value to 1.

Reality is different. The HW setting defaults to 1 at reset.
So it turns out that the function setting the value to 1
is pointless, because the value is already 1. There is
however no way to set the value to 0.

Does that explain why I wrote "assume"?

Also the commit message:
"The current code supports enabling RGMII RX and TX clock delays.
The unstated assumption is that these settings are disabled by
default at reset, which is not the case."

> Maybe you should add a sentence explaining when the code is NOT a no-op.

The code is *NEVER* NOT a no-op.
I.e. the code enabling RX clock delay is ALWAYS a no-op.
I don't understand what you think is unclear in my comment.

Regards.