Re: [PATCH net-next] net: phy: add GBit master / slave error detection

[Florian Fainelli <f.fainelli@gmail.com>]

On 07/17/2018 11:14 PM, Heiner Kallweit wrote:
> Certain PHY's have issues when operating in GBit slave mode and can
> be forced to master mode. Examples are RTL8211C, also the Micrel PHY
> driver has a DT setting to force master mode.
> If two such chips are link partners the autonegotiation will fail.
> Standard defines a self-clearing on read, latched-high bit to
> indicate this error. Check this bit to inform the user.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/phy/phy_device.c | 5 +++++
>  include/uapi/linux/mii.h     | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index b9f5f40a..249c6f75 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1551,6 +1551,11 @@ int genphy_read_status(struct phy_device *phydev)
>  			if (lpagb < 0)
>  				return lpagb;
>  
> +			if (lpagb & LPA_1000MSFAIL) {
> +				phydev_err(phydev, "Master/Slave resolution failed, maybe conflicting manual settings?\n");
> +				return -ENOLINK;
> +			}
> +

You should also be able to read whether Master/Slave was configured as
automatic or manual, and possibly issue a different message when
automatic/forced is specified?

AFAIR there was a patch a while ago from Mellanox guys that was possibly
extending the link notification with an error cause, this sounds like
something that could be useful to report to user space somehow to help
troubleshoot link down events.

Thanks for your improvements to PHYLIB.
-- 
Florian