From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH net 1/3] lan78xx: PHY DSP registers initialization to address EEE link drop issues with long cables Date: Fri, 06 Apr 2018 11:21:58 -0400 (EDT) Message-ID: <20180406.112158.2189892992551033169.davem@davemloft.net> References: <20180406061204.18257-1-raghuramchary.jallipalli@microchip.com> <20180406061204.18257-2-raghuramchary.jallipalli@microchip.com> <20180406144342.GN17495@lunn.ch> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: raghuramchary.jallipalli@microchip.com, netdev@vger.kernel.org, unglinuxdriver@microchip.com, woojung.huh@microchip.com To: andrew@lunn.ch Return-path: Received: from shards.monkeyblade.net ([184.105.139.130]:43500 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752977AbeDFPWA (ORCPT ); Fri, 6 Apr 2018 11:22:00 -0400 In-Reply-To: <20180406144342.GN17495@lunn.ch> Sender: netdev-owner@vger.kernel.org List-ID: From: Andrew Lunn Date: Fri, 6 Apr 2018 16:43:42 +0200 > On Fri, Apr 06, 2018 at 11:42:02AM +0530, Raghuram Chary J wrote: >> The patch is to configure DSP registers of PHY device >> to handle Gbe-EEE failures with >40m cable length. >> >> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver") >> Signed-off-by: Raghuram Chary J >> --- >> drivers/net/phy/microchip.c | 123 ++++++++++++++++++++++++++++++++++++++++++- >> include/linux/microchipphy.h | 8 +++ >> 2 files changed, 130 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c >> index 0f293ef28935..174ae9808722 100644 >> --- a/drivers/net/phy/microchip.c >> +++ b/drivers/net/phy/microchip.c >> @@ -20,6 +20,7 @@ >> #include >> #include >> #include >> +#include >> >> #define DRIVER_AUTHOR "WOOJUNG HUH " >> #define DRIVER_DESC "Microchip LAN88XX PHY driver" >> @@ -66,6 +67,107 @@ static int lan88xx_suspend(struct phy_device *phydev) >> return 0; >> } >> >> +static void lan88xx_TR_reg_set(struct phy_device *phydev, u16 regaddr, >> + u32 data) >> +{ >> + int val; >> + u16 buf; >> + >> + /* Get access to token ring page */ >> + phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS, >> + LAN88XX_EXT_PAGE_ACCESS_TR); > > Hi Raghuram > > You might want to look at phy_read_paged(), phy_write_paged(), etc. > > There can be race conditions with paged access. Yep, so something like: static void lan88xx_TR_reg_set(struct phy_device *phydev, u16 regaddr, u32 data) { int save_page, val; u16 buf; save_page = phy_save_page(phydev); phy_write_paged(phydev, LAN88XX_EXT_PAGE_ACCESS_TR, LAN88XX_EXT_PAGE_TR_LOW_DATA, (data & 0xFFFF)); phy_write_paged(phydev, LAN88XX_EXT_PAGE_ACCESS_TR, LAN88XX_EXT_PAGE_TR_HIGH_DATA, (data & 0x00FF0000) >> 16); /* Config control bits [15:13] of register */ buf = (regaddr & ~(0x3 << 13));/* Clr [14:13] to write data in reg */ buf |= 0x8000; /* Set [15] to Packet transmit */ phy_write_paged(phydev, LAN88XX_EXT_PAGE_ACCESS_TR, LAN88XX_EXT_PAGE_TR_CR, buf); usleep_range(1000, 2000);/* Wait for Data to be written */ val = phy_read_paged(phydev, LAN88XX_EXT_PAGE_ACCESS_TR, LAN88XX_EXT_PAGE_TR_CR); if (!(val & 0x8000)) pr_warn("TR Register[0x%X] configuration failed\n", regaddr); phy_restore_page(phydev, save_page, 0); } Since PHY accesses and thus things like phy_save_page() can fail, the return type of this function should be changed to 'int' and some error checking should be added.