On 11/09/2016 05:02 AM, Sebastian Frias wrote: > On 11/04/2016 05:49 PM, Måns Rullgård wrote: >>>> 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):
I really don't see a point in doing this when we can just clarify what phydev->interface does and already have the knowledge that we need without introducing additional flags in the phy driver. > > 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 */ > -- Florian
