In the initial device tree submission, the definitions of rgmii and
rgmii-id in the phy-mode property were inverted compared to the
conventions used by the upstream Linux kernel community.

Only QCS-EVB-400 and SA8155-ADP platforms are affected due to the
incorrect PHY mode configuration: 'rgmii' was used instead of the
correct 'rgmii-id'. This change results in an ABI compatibility break,
but it is acceptable as these platforms are not actively used by any
customers, based on current observations.

Qualcomm expects the MAC, not the PHY, to introduce the timing delay,
and the driver is designed accordingly. This is due to specific SoC
hardware that handles delay and sampling internally.

Signed-off-by: Yijie Yang <yijie.yang@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 14 +++++++++-----
 1 file changed, 9 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 a4ea72f86ca8..a3e595e3b1e4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -390,14 +390,11 @@ static int ethqos_dll_configure(struct qcom_ethqos *ethqos)
 static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
 {
 	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 */
@@ -803,7 +800,14 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 	if (!ethqos)
 		return -ENOMEM;
 
+	/* Qualcomm configures the MAC to introduce delay; instruct the
+	 * PHY not to add additional delay.
+	 */
+	if (plat_dat->phy_interface == PHY_INTERFACE_MODE_RGMII_ID)
+		plat_dat->phy_interface = PHY_INTERFACE_MODE_RGMII;
+
 	ethqos->phy_mode = plat_dat->phy_interface;
+
 	switch (ethqos->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:

-- 
2.34.1

>  static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
>  {
>  	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;

Does this one setting control both RX and TX delays? The hardware
cannot support 2ns delay on TX, but 0ns on RX? Or 2ns on RX but 0ns on
TX?

	Andrew

On 2025-08-20 00:20, Andrew Lunn wrote:
>>   static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
>>   {
>>   	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;
> 
> Does this one setting control both RX and TX delays? The hardware
> cannot support 2ns delay on TX, but 0ns on RX? Or 2ns on RX but 0ns on
> TX?
> 

This setting is only for Tx delay. Rx delays are taken care separately 
with DLL delays.

> 	Andrew

-- 
Best Regards,
Yijie

On Thu, Aug 21, 2025 at 10:22:05AM +0800, Yijie Yang wrote:
> 
> 
> On 2025-08-20 00:20, Andrew Lunn wrote:
> > >   static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
> > >   {
> > >   	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;
> > 
> > Does this one setting control both RX and TX delays? The hardware
> > cannot support 2ns delay on TX, but 0ns on RX? Or 2ns on RX but 0ns on
> > TX?
> > 
> 
> This setting is only for Tx delay. Rx delays are taken care separately with
> DLL delays.

If this is only for Tx delays, why is it also not used for
PHY_INTERFACE_MODE_RGMII_TXID?

It is simpler to just let the PHY add the delays, the PHY drivers get
this right, are well tested, and just work. MAC drivers often get
delays wrong.

	Andrew