netdev - Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

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, 17 Feb 2023 17:25:14 +0200
From:   Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Lee Jones <lee@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Emil Renner Berthing <kernel@...il.dk>,
        Conor Dooley <conor@...nel.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Richard Cochran <richardcochran@...il.com>,
        Sagar Kadam <sagar.kadam@...ive.com>,
        Yanhong Wang <yanhong.wang@...rfivetech.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, linux-riscv@...ts.infradead.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

On 2/17/23 15:30, Andrew Lunn wrote:
>>> I would actually say it shows we don't understand what is going on
>>> with delays. "rgmii" is not every often the correct value. The fact it
>>> works suggests the MAC is adding delays.
>>>
>>> What value are you using for starfive,gtxclk-dlychain ?
>>
>> This is set to '4' in patch 12/12.
>>
>>> Try 0 and then "rgmii-id"
>>
>> I made some more tests and it seems the only stable configuration is "rgmii"
>> with "starfive,gtxclk-dlychain" set to 4:
>>
>> phy-mode | dlychain | status
>> ---------+----------+--------------------------------------------
>> rgmii    |        4 | OK (no issues observed)
>> rgmii-id |        4 | BROKEN (errors reported [1])
>> rgmii    |        0 | UNRELIABLE (no errors, but frequent stalls)
>> rgmii-id |        0 | BROKEN (errors reported)
>>
>> [1] Reported errors in case of BROKEN status:
>> $ grep '' /sys/class/net/eth0/statistics/* | grep -v ':0$'
> 
> Thanks for the testing.
> 
> So it seems like something is adding delays when it probably should
> not. Ideally we want to know what.
> 
> There is a danger here, something which has happened in the past. A
> PHY which ignored "rgmii" and actually did power on defaults which was
> "rgmii-id". As a result, lots of boards put "rmgii" in there DT blob,
> which 'worked'. Until a board came along which really did need
> "rgmii". The developer bringing that board up debugged the PHY, found
> the problem and made it respect "rgmii" so their board worked. And the
> fix broke a number of 'working' boards which had the wrong "rgmii"
> instead of "rgmii-id".

Thanks for the heads-up.

> So you have a choice. Go with 4 and "rgmii", but put in a big fat
> warning, "Works somehow but is technically wrong and will probably
> break sometime in the future". Or try to understand what is really
> going on here, were are the delays coming from, and fix the issue.
> 
>        Andrew

I will try to analyze this further.

Regards,
Cristian