Message ID | 20180820145530.7657-1-anssi.hannula@bitwise.fi |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Series | net: macb: do not disable MDIO bus at open/close time | expand |
On 20.08.2018 17:55, 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. > > Fixes: 6c36a7074436 ("macb: Use generic PHY layer") > Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi> > --- > > Claudiu Beznea wrote: >> On 10.08.2018 09:22, Anssi Hannula wrote: >>> >>> macb_reset_hw() is called in init path too, >> >> I only see it in macb_close() and macb_open() called from macb_init_hw(). > > Yeah, macb_init_hw() is what I meant :) > > > drivers/net/ethernet/cadence/macb_main.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index dc09f9a8a49b..6501e9b3785a 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); > @@ -2170,6 +2173,7 @@ static void macb_init_hw(struct macb *bp) > unsigned int q; > > u32 config; > + u32 ctrl; > > macb_reset_hw(bp); > macb_set_hwaddr(bp); > @@ -2223,7 +2227,9 @@ 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)); > + ctrl = macb_readl(bp, NCR); > + ctrl |= MACB_BIT(RE) | MACB_BIT(TE); > + macb_writel(bp, NCR, ctrl); I would keep it as: 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 >
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index dc09f9a8a49b..6501e9b3785a 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); @@ -2170,6 +2173,7 @@ static void macb_init_hw(struct macb *bp) unsigned int q; u32 config; + u32 ctrl; macb_reset_hw(bp); macb_set_hwaddr(bp); @@ -2223,7 +2227,9 @@ 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)); + ctrl = macb_readl(bp, NCR); + ctrl |= MACB_BIT(RE) | MACB_BIT(TE); + macb_writel(bp, NCR, ctrl); } /* The hash address register is 64 bits long and takes up two