On 22.01.2025 10:48 AM, Krzysztof Kozlowski wrote:

On 22/01/2025 09:56, Yijie Yang wrote:

quoted

On 2025-01-21 20:47, Krzysztof Kozlowski wrote:

quoted

On 21/01/2025 08:54, Yijie Yang wrote:

quoted

The Qualcomm board always chooses the MAC to provide the delay instead of
the PHY, which is completely opposite to the suggestion of the Linux
kernel.

How does the Linux kernel suggest it?

quoted

The usage of phy-mode in legacy DTS was also incorrect. Change the
phy_mode passed from the DTS to the driver from PHY_INTERFACE_MODE_RGMII_ID
to PHY_INTERFACE_MODE_RGMII to ensure correct operation and adherence to
the definition.
To address the ABI compatibility issue between the kernel and DTS caused by
this change, handle the compatible string 'qcom,qcs404-evb-4000' in the
code, as it is the only legacy board that mistakenly uses the 'rgmii'
phy-mode.

Signed-off-by: Yijie Yang <redacted>
---
  .../net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    | 18 +++++++++++++-----
  1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 2a5b38723635b5ef9233ca4709e99dd5ddf06b77..e228a62723e221d58d8c4f104109e0dcf682d06d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c

@@ -401,14 +401,11 @@ static int ethqos_dll_configure(struct qcom_ethqos *ethqos)
  static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
  {
  	struct device *dev = &ethqos->pdev->dev;
-	int phase_shift;
+	int phase_shift = 0;
  	int loopback;
  
  	/* Determine if the PHY adds a 2 ns TX delay or the MAC handles it */
-	if (ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_ID ||
-	    ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
-		phase_shift = 0;
-	else
+	if (ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_ID)
  		phase_shift = RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN;
  
  	/* Disable loopback mode */

@@ -810,6 +807,17 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
  	ret = of_get_phy_mode(np, &ethqos->phy_mode);
  	if (ret)
  		return dev_err_probe(dev, ret, "Failed to get phy mode\n");
+
+	root = of_find_node_by_path("/");
+	if (root && of_device_is_compatible(root, "qcom,qcs404-evb-4000"))

First, just check if machine is compatible, don't open code it.

Second, drivers should really, really not rely on the machine. I don't
think how this resolves ABI break for other users at all.

As detailed in the commit description, some boards mistakenly use the 
'rgmii' phy-mode, and the MAC driver has also incorrectly parsed and 

That's a kind of an ABI now, assuming it worked fine.

I'm inclined to think it's better to drop compatibility given we're talking
about rather obscure boards here.

$ rg 'mode.*=.*"rgmii"' arch/arm64/boot/dts/qcom -l

arch/arm64/boot/dts/qcom/sa8155p-adp.dts
arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts

QCS404 seems to have zero interest from anyone (and has been considered
for removal upstream..).

The ADP doesn't see much traction either, last time around someone found
a boot breaking issue months after it was committed.

Konrad