linux-kernel - Re: [PATCH v2 2/2] arm64: dts: qcom: qcs615-ride: Enable ethernet node

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: <581776bc-f3bc-44c1-b7c0-4c2e637fcd67@quicinc.com>
Date: Mon, 16 Dec 2024 14:56:13 +0800
From: Yijie Yang <quic_yijiyang@...cinc.com>
To: Andrew Lunn <andrew@...n.ch>
CC: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Bjorn Andersson
	<andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, Rob Herring
	<robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
	<conor+dt@...nel.org>,
        Richard Cochran <richardcochran@...il.com>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] arm64: dts: qcom: qcs615-ride: Enable ethernet
 node



On 2024-12-10 12:09, Andrew Lunn wrote:
>> As previously mentioned, using 'rgmii' will enable EMAC to provide the delay
>> while disabling the delay for EPHY. So there's won't be double delay.
>>
>> Additionally, the current implementation of the QCOM driver code exclusively
>> supports this mode, with the entire initialization sequence of EMAC designed
>> and fixed for this specific mode.
> 
> OK. If it is impossible to disable these delays, you need to validate
> phy-mode. Only rgmii-id is allowed. Anybody trying to build a board
> using extra long clock lines is out of luck. It does not happen very
> often, but there are a small number of boards which do this, and the
> definitions of phy-mode are designed to support them.
> 
>> I'm not sure if there's a disagreement about the definition or a
>> misunderstanding with other vendors. From my understanding, 'rgmii' should
>> not imply that the delay must be provided by the board, based on both the
>> definition in the dt-binding file and the implementations by other EMAC
>> vendors. Most EMAC drivers provide the delay in this mode.
> 
> Nope. You are wrong. I've been enforcing this meaning for maybe the
> last 10 years. You can go search the email archive for netdev. Before
> that, we had a bit of a mess, developers were getting it wrong, and
> reviewing was not as good. And i don't review everything, so some bad
> code does get passed me every so often, e.g. if found out today that
> TI AM62 got this wrong, they hard code TX delays in the MAC, and DT
> developers have been using rgmii-rxid, not rgmii-id, and the MAC
> driver is missing the mask operation before calling phy_connect.
> 
>> I confirmed that there is no delay on the qcs615-ride board., and the QCOM
>> EMAC driver will adds the delay by shifting the clock after receiving
>> PHY_INTERFACE_MODE_RGMII.
> 
> Which is wrong. Because you cannot disable the delay,
> PHY_INTERFACE_MODE_RGMII should return in EINVAL, or maybe
> EOPNOTSUPP. Your hardware only supports PHY_INTERFACE_MODE_RGMII_ID,
> and you need to mask what you pass to phylib/phylink to make it clear
> the MAC has added the delays.
> 
> 	Andrew
I intend to follow these steps. Could you please check if they are correct?
1. Add a new flag in DTS to inform the MAC driver to include the delay 
when configured with 'rgmii-id'. Without this flag, the MAC driver will 
not be aware of the need for the delay.
2. In the driver, if this flag is set to true, change the phy_mode to 
PHY_INTERFACE_MODE_RGMII to instruct the PHY not to add the delay.

-- 
Best Regards,
Yijie