From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH net-next] net: phy: add GBit master / slave error detection Date: Wed, 18 Jul 2018 02:22:19 -0700 Message-ID: References: <60685308-a848-e4d0-e170-f2738f046679@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: "netdev@vger.kernel.org" To: Heiner Kallweit , Andrew Lunn , David Miller Return-path: Received: from mail-qt0-f193.google.com ([209.85.216.193]:38984 "EHLO mail-qt0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731108AbeGRJ7W (ORCPT ); Wed, 18 Jul 2018 05:59:22 -0400 Received: by mail-qt0-f193.google.com with SMTP id q12-v6so3375940qtp.6 for ; Wed, 18 Jul 2018 02:22:23 -0700 (PDT) In-Reply-To: <60685308-a848-e4d0-e170-f2738f046679@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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 > --- > 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