On 12.04.2019 01:01, Vladimir Oltean wrote: > With Heiner's recent patch "b6163f194c69 net: phy: improve > genphy_read_status", the phydev->speed is now initialized by default to > SPEED_UNKNOWN even for fixed PHYs. This is not necessarily bad, since it > is not correct to call genphy_config_init() and genphy_read_status() for > a fixed PHY. > What do you mean with "it is not correct"? Whether the calls are always needed may be a valid question, but it's not forbidden to use these calls with a fixed PHY. Actually in phylib polling mode genphy_read_status is called every second also for a fixed PHY. swphy emulates all relevant PHY registers.
> This dates back all the way to "39b0c705195e net: dsa: Allow > configuration of CPU & DSA port speeds/duplex" (discussion thread: > https://www.spinics.net/lists/netdev/msg340862.html). > > I don't seem to understand why these calls were necessary back then, but > removing these calls seemingly has no impact now apart from preventing > the phydev->speed that was set in of_phy_register_fixed_link() from > getting overwritten. > I tend to agree with the change itself, but not with the justification. For genphy_config_init I'd rather say: genphy_config_init removes invalid modes based on the abilities read from the chip. But as we emulate all registers anyway, this doesn't provide any benefit for a swphy. > Signed-off-by: Vladimir Oltean <[email protected]> > --- > net/dsa/port.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/net/dsa/port.c b/net/dsa/port.c > index 87769cb38c31..481412c892a7 100644 > --- a/net/dsa/port.c > +++ b/net/dsa/port.c > @@ -485,9 +485,6 @@ static int dsa_port_fixed_link_register_of(struct > dsa_port *dp) > mode = PHY_INTERFACE_MODE_NA; > phydev->interface = mode; > > - genphy_config_init(phydev); > - genphy_read_status(phydev); > - > if (ds->ops->adjust_link) > ds->ops->adjust_link(ds, port, phydev); > >
