On Thu, Dec 13, 2018 at 02:27:51PM +0100, Marek Vasut wrote:
> On 12/13/2018 09:38 AM, Andrew Lunn wrote:
> >> +static int tja11xx_config_aneg(struct phy_device *phydev)
> >> +{
> >> + phydev->autoneg = 0;
> >> + phydev->speed = SPEED_100;
> >> + phydev->duplex = DUPLEX_FULL;
> >> + phydev->pause = 0;
> >> + phydev->asym_pause = 0;
> >> +
> >> + return 0;
> >> +}
> >
> > Hi Marek
>
> Hi,
>
> > That is, err, interesting. Are you saying the PHY cannot do auto-neg?
>
> There isn't much to auto-negotiate and some of the bits in the standard
> registers behave weirdly.
Hi Marek
Hopefully the config_aneg callback is not called if you don't list
autoneg to the .features. The microchip_t1 driver just uses
genphy_config_aneg, but if a NULL works, i would prefer that.
> > Does it happen to be a 100T1 device for automotive?
>
> Yes, that's what the commit message says, why ?
Sorry, i missed it in the commit message. We now have enough bits in
the link_mode that we should explicitly say this is 100BaseT2_Full,
not 100BaseT_Full. So please add a new member to
ethtool_link_mode_bit_indices ...
> >> +
> >> +static int tja11xx_config_init(struct phy_device *phydev)
> >> +{
> >> + u32 features = SUPPORTED_TP | SUPPORTED_MII | SUPPORTED_100baseT_Full;
And make it part of the .features. Well, you need to add a new
PHY_BASIC_T2_FEATURES macro, which uses it, and use that in the
phy_driver structure. There should not be any need to modify it in the
config_init call.
We also need to think about what we do with the PHY_BASIC_T1_FEATURES
macro. Ideally we want to swap that to also make use of a new
ethtool_link_mode_bit_indices, but i've no idea at the moment if that
will break something.
Andrew
