Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level

On 11/04/2016 05:49 PM, Måns Rullgård wrote:

quoted

quoted

But when doing so, both the Atheros 8035 and the Aurora NB8800 drivers
will apply the delay.

I think a better way of dealing with this is that both, PHY and MAC
drivers exchange information so that the delay is applied only once.

Exchange what information? The PHY device interface (phydev->interface)
conveys the needed information for both entities.

There doesn't seem to be any consensus among the drivers regarding where
the delay should be applied.  Since only a few drivers, MAC or PHY, act
on this property, most combinations still work by chance.  It is common
for boards to set the delay at the PHY using external config pins so no
software setup is required (although I have one Sigma based board that
gets this wrong).  I suspect if drivers/net/ethernet/broadcom/genet were
used with one of the four PHY drivers that also set the delay based on
this DT property, things would go wrong.

Exactly, what about a patch like (I can make a formal submission, even
merge it with the patch discussed in this thread, consider this a RFC):

diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index fba2699..4217ff4 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c

@@ -1283,6 +1283,10 @@ static int nb8800_tangox_init(struct net_device *dev)
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		pad_mode = PAD_MODE_RGMII;
+
+		if ((dev->phydev->flags & PHY_SUPPORTS_TXID) == 0)
+			pad_mode |= PAD_MODE_GTX_CLK_DELAY;
+
 		break;
 
 	default:

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 2e0c759..5eddb04 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c

@@ -426,7 +426,9 @@ static int at803x_aneg_done(struct phy_device *phydev)
 	.suspend		= at803x_suspend,
 	.resume			= at803x_resume,
 	.features		= PHY_GBIT_FEATURES,
-	.flags			= PHY_HAS_INTERRUPT,
+	.flags			= PHY_HAS_INTERRUPT |
+				  PHY_SUPPORTS_RXID |
+				  PHY_SUPPORTS_TXID,
 	.config_aneg		= genphy_config_aneg,
 	.read_status		= genphy_read_status,
 	.ack_interrupt		= at803x_ack_interrupt,

diff --git a/include/linux/phy.h b/include/linux/phy.h
index e7e1fd3..0f0b17e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h

@@ -61,6 +61,8 @@
 #define PHY_HAS_INTERRUPT	0x00000001
 #define PHY_HAS_MAGICANEG	0x00000002
 #define PHY_IS_INTERNAL		0x00000004
+#define PHY_SUPPORTS_RXID       0x00000008
+#define PHY_SUPPORTS_TXID       0x00000010
 #define MDIO_DEVICE_IS_PHY	0x80000000
 
 /* Interface Mode definitions */