From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claudiu Beznea Subject: Re: [PATCH v2] net: macb: do not disable MDIO bus at open/close time Date: Fri, 24 Aug 2018 17:47:52 +0300 Message-ID: <4800e8c0-5282-a01b-ba15-a0c01ad08038@microchip.com> References: <6b9e6b74-374b-8ad1-cae6-7d00548a88a5@microchip.com> <20180823074522.5663-1-anssi.hannula@bitwise.fi> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: "David S. Miller" , Andrew Lunn To: Anssi Hannula , , Nicolas Ferre Return-path: Received: from esa2.microchip.iphmx.com ([68.232.149.84]:57253 "EHLO esa2.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726264AbeHXSWy (ORCPT ); Fri, 24 Aug 2018 14:22:54 -0400 In-Reply-To: <20180823074522.5663-1-anssi.hannula@bitwise.fi> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 23.08.2018 10:45, Anssi Hannula wrote: > macb_reset_hw() is called from macb_close() and indirectly from > macb_open(). macb_reset_hw() zeroes the NCR register, including the MPE > (Management Port Enable) bit. > > This will prevent accessing any other PHYs for other Ethernet MACs on > the MDIO bus, which remains registered at macb_reset_hw() time, until > macb_init_hw() is called from macb_open() which sets the MPE bit again. > > I.e. currently the MDIO bus has a short disruption at open time and is > disabled at close time until the interface is opened again. > > Fix that by only touching the RE and TE bits when enabling and disabling > RX/TX. > > v2: Make macb_init_hw() NCR write a single statement. This usually goes after the tree '-' below. > > Fixes: 6c36a7074436 ("macb: Use generic PHY layer") > Signed-off-by: Anssi Hannula > --- Here: v2: Make macb_init_hw() NCR write a single statement. Other than this: Reviewed-by: Claudiu Beznea Checked that link goes up and ping is working on a SAMA5D2 Xplained: Tested-by: Claudiu Beznea > drivers/net/ethernet/cadence/macb_main.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index dc09f9a8a49b..225a7c8bad2d 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -2028,14 +2028,17 @@ static void macb_reset_hw(struct macb *bp) > { > struct macb_queue *queue; > unsigned int q; > + u32 ctrl = macb_readl(bp, NCR); > > /* Disable RX and TX (XXX: Should we halt the transmission > * more gracefully?) > */ > - macb_writel(bp, NCR, 0); > + ctrl &= ~(MACB_BIT(RE) | MACB_BIT(TE)); > > /* Clear the stats registers (XXX: Update stats first?) */ > - macb_writel(bp, NCR, MACB_BIT(CLRSTAT)); > + ctrl |= MACB_BIT(CLRSTAT); > + > + macb_writel(bp, NCR, ctrl); > > /* Clear all status flags */ > macb_writel(bp, TSR, -1); > @@ -2223,7 +2226,7 @@ static void macb_init_hw(struct macb *bp) > } > > /* Enable TX and RX */ > - macb_writel(bp, NCR, MACB_BIT(RE) | MACB_BIT(TE) | MACB_BIT(MPE)); > + macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE)); > } > > /* The hash address register is 64 bits long and takes up two >