From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH net-next] net: phy: add GBit master / slave error detection Date: Thu, 19 Jul 2018 07:54:31 +0200 Message-ID: <8ccaeb57-3150-65ce-2310-3aac52f446c5@gmail.com> 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: Florian Fainelli , Andrew Lunn , David Miller Return-path: Received: from mail-wr1-f67.google.com ([209.85.221.67]:45383 "EHLO mail-wr1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726625AbeGSGgF (ORCPT ); Thu, 19 Jul 2018 02:36:05 -0400 Received: by mail-wr1-f67.google.com with SMTP id c4-v6so6722950wrs.12 for ; Wed, 18 Jul 2018 22:54:41 -0700 (PDT) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 18.07.2018 11:22, Florian Fainelli wrote: > > > 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? > Indeed, this could be done, based on the local automatic/manual setting. > 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. > Do you by chance have a reference to this patch? There's heavy development on the Mellanox drivers with a lot of patches. > Thanks for your improvements to PHYLIB. >