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: <31a87bd9-4ffb-4d5a-a77b-7411234f1a03@lunn.ch>
Date: Tue, 10 Dec 2024 05:09:12 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Yijie Yang <quic_yijiyang@...cinc.com>
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

> 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