Messages in this thread | | | Subject | Re: [PATCH net 2/2] net: phy: Use C45 Helpers in PHY_FORCING state | From | Jose Abreu <> | Date | Mon, 4 Mar 2019 15:07:24 +0000 |
| |
Hi Andrew,
On 3/1/2019 1:53 PM, Andrew Lunn wrote: > On Fri, Mar 01, 2019 at 11:54:24AM +0100, Jose Abreu wrote: >> +static inline int phy_update_link(struct phy_device *phydev) >> +{ >> + if (!phydev->drv) >> + return -EIO; >> + >> + if (phydev->drv->read_status) >> + return phydev->drv->read_status(phydev); >> + else if (phydev->is_c45) >> + return gen10g_read_status(phydev); >> + else >> + return genphy_update_link(phydev); >> +} > > Hi Jose > > The asymmetry here could be an issue. We might fall into the trap > that a c45 PHY has the full state in phydev updated, were as a c22 > only has the link updated. Somebody testing on C45 might miss a bug > for a C22 device.
Notice that this phy_update_link() is called from PHY_FORCING state which in my case happens when autoneg is not enabled / is not supported.
I think it makes sense, in this case, to only update link status, no ?
Thanks, Jose Miguel Abreu
> > Maybe this should be called phy_read_state(), and calls > genphy_read_status() not genphy_update_link(). > > Andrew >
| |