On 04.08.2019 17:59, Vladimir Oltean wrote: > On Sun, 4 Aug 2019 at 17:52, Andrew Lunn <and...@lunn.ch> wrote: >> >>>> The patchset looks better now. But is it ok, I wonder, to keep >>>> PHY_BCM_FLAGS_MODE_1000BX in phydev->dev_flags, considering that >>>> phy_attach_direct is overwriting it? >>> >> >>> I checked ftgmac100 driver (used on my machine) and it calls >>> phy_connect_direct which passes phydev->dev_flags when calling >>> phy_attach_direct: that explains why the flag is not cleared in my >>> case. >> >> Yes, that is the way it is intended to be used. The MAC driver can >> pass flags to the PHY. It is a fragile API, since the MAC needs to >> know what PHY is being used, since the flags are driver specific. >> >> One option would be to modify the assignment in phy_attach_direct() to >> OR in the flags passed to it with flags which are already in >> phydev->dev_flags. >> >> Andrew > > Even if that were the case (patching phy_attach_direct to apply a > logical-or to dev_flags), it sounds fishy to me that the genphy code > is unable to determine that this PHY is running in 1000Base-X mode. > > In my opinion it all boils down to this warning: > > "PHY advertising (0,00000200,000062c0) more modes than genphy > supports, some modes not advertised". > The genphy code deals with Clause 22 + Gigabit BaseT only. Question is whether you want aneg at all in 1000Base-X mode and what you want the config_aneg callback to do. There may be some inspiration in the Marvel PHY drivers.
> You see, the 0x200 in the above advertising mask corresponds exactly > to this definition from ethtool.h: > ETHTOOL_LINK_MODE_1000baseX_Full_BIT = 41, > > But it gets truncated and hence lost. > > Regards, > -Vladimir > Heiner
