On 11.08.2019 18:08, Marek Behun wrote: > On Sun, 11 Aug 2019 17:40:01 +0200 > Andrew Lunn <and...@lunn.ch> wrote: > >> On Sun, Aug 11, 2019 at 05:08:12PM +0200, Marek Behún wrote: >>> The fixed_phy driver does not set the phydev->is_gigabit_capable member >>> when the fixed_phy is gigabit capable. >> >> Neither does any other PHY driver. It should be possible to tell if a >> PHY supports 1G by looking at register values. If this does not work >> for fixed_link, it means we are missing something in the emulation. >> That is what we should be fixing. >> >> Also, this change has nothing to do the lp_advertise, what you >> previously said the problem was. At the moment, i don't get the >> feeling you have really dug all the way down and really understand the >> root causes. >> >> Andrew > > Andrew, > is_gigabit_capable is otherwise set only in the phy_probe function. > This function is not called at all for the DSA cpu port fixed_link phy. > Why is that? But I guess it is not important anymore, if CPU and DSA > were converted to phylink in net-next. I shall test it and let you know. > In any case, sorry for the spam. > Marek >
A fixed phy should be bound to the genphy driver, but that happens later in phy_attach_direct(). Maybe the fixed phy device of a CPU port gets never attached to a net device? This would explain why phy_probe() doesn't get called. Following idea: We could swphy let return PHY ID 0xffffffff (instead of current value 0x00000000). Then the fixed phy device would be bound to the genphy driver immediately at device registration time. As a consequence phy_probe() would call genphy_read_abilities() that sets supported modes properly. This should allow to remove the manual adding of supported modes in dsa_port_fixed_link_register_of(). One more thing regarding genphy_read_abilities() and fixed phys in general: swphy sets bit BMSR_ESTATEN, then genphy_read_abilities() reads MII_ESTATUS to check if and which 1Gbps modes are supported. MII_ESTATUS however isn't defined in swphy. We're just fortunate that therefore the default value 0xffff is returned and both 1Gbps modes seem to be supported. Last but not least I think the calls to genphy_config_init() and genphy_read_status() in dsa_port_fixed_link_register_of() are both not needed and could be removed. Heiner
