* [PATCH] net: macb: do not disable MDIO bus when closing interface @ 2018-08-08 12:19 Anssi Hannula 2018-08-09 8:26 ` Claudiu Beznea 0 siblings, 1 reply; 11+ messages in thread From: Anssi Hannula @ 2018-08-08 12:19 UTC (permalink / raw) To: Nicolas Ferre, David S. Miller; +Cc: netdev macb_close() calls macb_reset_hw() which zeroes 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 is still registered. Fix that by keeping the MPE bit set. Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi> --- drivers/net/ethernet/cadence/macb_main.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index dc09f9a8a49b..3ca98fc32144 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -2030,12 +2030,13 @@ static void macb_reset_hw(struct macb *bp) unsigned int q; /* Disable RX and TX (XXX: Should we halt the transmission - * more gracefully?) + * more gracefully?) but keep management port open since there + * may be other users of the mdio bus */ - macb_writel(bp, NCR, 0); + macb_writel(bp, NCR, MACB_BIT(MPE)); /* Clear the stats registers (XXX: Update stats first?) */ - macb_writel(bp, NCR, MACB_BIT(CLRSTAT)); + macb_writel(bp, NCR, MACB_BIT(CLRSTAT) | MACB_BIT(MPE)); /* Clear all status flags */ macb_writel(bp, TSR, -1); -- 2.16.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] net: macb: do not disable MDIO bus when closing interface 2018-08-08 12:19 [PATCH] net: macb: do not disable MDIO bus when closing interface Anssi Hannula @ 2018-08-09 8:26 ` Claudiu Beznea 2018-08-09 14:54 ` Anssi Hannula 0 siblings, 1 reply; 11+ messages in thread From: Claudiu Beznea @ 2018-08-09 8:26 UTC (permalink / raw) To: Anssi Hannula, Nicolas Ferre, David S. Miller; +Cc: netdev On 08.08.2018 15:19, Anssi Hannula wrote: > macb_close() calls macb_reset_hw() which zeroes 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 is still registered. > > Fix that by keeping the MPE bit set. > > Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi> > --- > drivers/net/ethernet/cadence/macb_main.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index dc09f9a8a49b..3ca98fc32144 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -2030,12 +2030,13 @@ static void macb_reset_hw(struct macb *bp) > unsigned int q; > > /* Disable RX and TX (XXX: Should we halt the transmission > - * more gracefully?) > + * more gracefully?) but keep management port open since there > + * may be other users of the mdio bus > */ > - macb_writel(bp, NCR, 0); > + macb_writel(bp, NCR, MACB_BIT(MPE)); Would be better to read the NCR and clear only RX and TX bits, something like: val = macb_readl(bp, NCR); /* Disable TX and RX. */ val &= ~(MACB_BIT(TE) | MACB_BIT(RE)); /* Clear statistics */ val |= MACB_BIT(CLRSTAT); macb_writel(bp, NCR, val); MPE should have been enabled by previous operations. Thank you, Claudiu Beznea > > /* Clear the stats registers (XXX: Update stats first?) */ > - macb_writel(bp, NCR, MACB_BIT(CLRSTAT)); > + macb_writel(bp, NCR, MACB_BIT(CLRSTAT) | MACB_BIT(MPE)); > > /* Clear all status flags */ > macb_writel(bp, TSR, -1); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: macb: do not disable MDIO bus when closing interface 2018-08-09 8:26 ` Claudiu Beznea @ 2018-08-09 14:54 ` Anssi Hannula 2018-08-09 15:14 ` Andrew Lunn 0 siblings, 1 reply; 11+ messages in thread From: Anssi Hannula @ 2018-08-09 14:54 UTC (permalink / raw) To: Claudiu Beznea; +Cc: Nicolas Ferre, David S. Miller, netdev On 9.8.2018 11:26, Claudiu Beznea wrote: > On 08.08.2018 15:19, Anssi Hannula wrote: >> macb_close() calls macb_reset_hw() which zeroes 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 is still registered. >> >> Fix that by keeping the MPE bit set. >> >> Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi> >> --- >> drivers/net/ethernet/cadence/macb_main.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c >> index dc09f9a8a49b..3ca98fc32144 100644 >> --- a/drivers/net/ethernet/cadence/macb_main.c >> +++ b/drivers/net/ethernet/cadence/macb_main.c >> @@ -2030,12 +2030,13 @@ static void macb_reset_hw(struct macb *bp) >> unsigned int q; >> >> /* Disable RX and TX (XXX: Should we halt the transmission >> - * more gracefully?) >> + * more gracefully?) but keep management port open since there >> + * may be other users of the mdio bus >> */ >> - macb_writel(bp, NCR, 0); >> + macb_writel(bp, NCR, MACB_BIT(MPE)); > Would be better to read the NCR and clear only RX and TX bits, something like: > val = macb_readl(bp, NCR); > > /* Disable TX and RX. */ > val &= ~(MACB_BIT(TE) | MACB_BIT(RE)); > > /* Clear statistics */ > val |= MACB_BIT(CLRSTAT); > > macb_writel(bp, NCR, val); > > MPE should have been enabled by previous operations. macb_reset_hw() is called in init path too, though, so maybe clearing all bits is intentional / wanted to get the controller to a known state, even though the comment only mentions TX/RX? If so, maybe the below would be better? But if you think just clearing TE/RE is better I'll just do it like you suggested. val = macb_readl(bp, NCR); /* Keep only MDIO port active */ val &= MACB_BIT(MPE); /* Clear statistics */ val |= MACB_BIT(CLRSTAT); macb_writel(bp, NCR, val); > Thank you, > Claudiu Beznea > >> >> /* Clear the stats registers (XXX: Update stats first?) */ >> - macb_writel(bp, NCR, MACB_BIT(CLRSTAT)); >> + macb_writel(bp, NCR, MACB_BIT(CLRSTAT) | MACB_BIT(MPE)); >> >> /* Clear all status flags */ >> macb_writel(bp, TSR, -1); >> -- Anssi Hannula / Bitwise Oy +358 503803997 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: macb: do not disable MDIO bus when closing interface 2018-08-09 14:54 ` Anssi Hannula @ 2018-08-09 15:14 ` Andrew Lunn 2018-08-10 6:22 ` Anssi Hannula 0 siblings, 1 reply; 11+ messages in thread From: Andrew Lunn @ 2018-08-09 15:14 UTC (permalink / raw) To: Anssi Hannula; +Cc: Claudiu Beznea, Nicolas Ferre, David S. Miller, netdev Hi Anssi > macb_reset_hw() is called in init path too, though, so maybe clearing > all bits is intentional / wanted to get the controller to a known state, > even though the comment only mentions TX/RX? You need to be careful here. Once of_mdiobus_register() is called, the MDIO should be usable. If you happen to have an Ethernet switch on the bus, it could be probed then. The DSA driver will start using the bus. Or if you have a second PHY, connected to some other MAC, it could be used by the other MAC. This all happens in the macb_probe function. Sometime later, the interface will be up'ed. At this point macb_open() is called, which calls macb_init_hw(), which calls macb_reset_hw(). What you don't want happening is changes to the NCR at this point breaking an MDIO transaction which might be going on. Ideally, the MPE should be enabled before of_mdiobus_register(), and left alone until mdiobus_unregister() is called in macb_remove(). Andrew ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: macb: do not disable MDIO bus when closing interface 2018-08-09 15:14 ` Andrew Lunn @ 2018-08-10 6:22 ` Anssi Hannula 2018-08-13 9:08 ` Claudiu Beznea 0 siblings, 1 reply; 11+ messages in thread From: Anssi Hannula @ 2018-08-10 6:22 UTC (permalink / raw) To: Andrew Lunn; +Cc: Claudiu Beznea, Nicolas Ferre, David S. Miller, netdev On 9.8.2018 18:14, Andrew Lunn wrote: > Hi Anssi Hi! >> macb_reset_hw() is called in init path too, though, so maybe clearing >> all bits is intentional / wanted to get the controller to a known state, >> even though the comment only mentions TX/RX? > You need to be careful here. Once of_mdiobus_register() is called, the > MDIO should be usable. If you happen to have an Ethernet switch on the > bus, it could be probed then. The DSA driver will start using the bus. > Or if you have a second PHY, connected to some other MAC, it could be > used by the other MAC. This all happens in the macb_probe function. > > Sometime later, the interface will be up'ed. At this point macb_open() > is called, which calls macb_init_hw(), which calls > macb_reset_hw(). What you don't want happening is changes to the NCR > at this point breaking an MDIO transaction which might be going on. > > Ideally, the MPE should be enabled before of_mdiobus_register(), and > left alone until mdiobus_unregister() is called in macb_remove(). Yep, fixing the use case of having PHYs of other MACs is why I wrote the patch :) Currently the reset code disables MPE while other MACs are using PHYs on the bus. -- Anssi Hannula / Bitwise Oy +358 503803997 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net: macb: do not disable MDIO bus when closing interface 2018-08-10 6:22 ` Anssi Hannula @ 2018-08-13 9:08 ` Claudiu Beznea 2018-08-20 14:55 ` [PATCH] net: macb: do not disable MDIO bus at open/close time Anssi Hannula 0 siblings, 1 reply; 11+ messages in thread From: Claudiu Beznea @ 2018-08-13 9:08 UTC (permalink / raw) To: Anssi Hannula, Andrew Lunn; +Cc: Nicolas Ferre, David S. Miller, netdev On 10.08.2018 09:22, Anssi Hannula wrote: > On 9.8.2018 18:14, Andrew Lunn wrote: >> Hi Anssi > > Hi! > >>> macb_reset_hw() is called in init path too, I only see it in macb_close() and macb_open() called from macb_init_hw(). though, so maybe clearing >>> all bits is intentional / wanted to get the controller to a known state, As far as I know the NCR is used in this driver only to control transmit enable, receive enable, MPE and tx start. Not clearing NCR in the init phase should not have any impact on IP. Same for not clearing Tx start on "if down" path (if tx is in progress). >>> even though the comment only mentions TX/RX? >> You need to be careful here. Once of_mdiobus_register() is called, the >> MDIO should be usable. If you happen to have an Ethernet switch on the >> bus, it could be probed then. The DSA driver will start using the bus. >> Or if you have a second PHY, connected to some other MAC, it could be >> used by the other MAC. This all happens in the macb_probe function. >> >> Sometime later, the interface will be up'ed. At this point macb_open() >> is called, which calls macb_init_hw(), which calls >> macb_reset_hw(). What you don't want happening is changes to the NCR >> at this point breaking an MDIO transaction which might be going on. >> >> Ideally, the MPE should be enabled before of_mdiobus_register(), and >> left alone until mdiobus_unregister() is called in macb_remove(). > > Yep, fixing the use case of having PHYs of other MACs is why I wrote the > patch :) > > Currently the reset code disables MPE while other MACs are using PHYs on > the bus. MPE is set in the early phase of initialization, in macb_mii_init(), called from probe() function. Due to the fact that NCR is cleared in macb_reset_hw() it is set again at the end of macb_init_hw(). So, as per Andrew comments in this thread, setting it in macb_mii_init() and letting it unchanged until remove would be the best. So, if you clear only TE and RE bits in macb_reset_hw() please also remove MACB_BIT(MPE) from below line at the end of macb_init_hw(): macb_writel(bp, NCR, MACB_BIT(RE) | MACB_BIT(TE) | MACB_BIT(MPE)); Thank you, Claudiu Beznea > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] net: macb: do not disable MDIO bus at open/close time 2018-08-13 9:08 ` Claudiu Beznea @ 2018-08-20 14:55 ` Anssi Hannula 2018-08-22 15:33 ` Claudiu Beznea 0 siblings, 1 reply; 11+ messages in thread From: Anssi Hannula @ 2018-08-20 14:55 UTC (permalink / raw) To: netdev, Nicolas Ferre, Claudiu Beznea; +Cc: David S. Miller, Andrew Lunn 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); } /* The hash address register is 64 bits long and takes up two -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] net: macb: do not disable MDIO bus at open/close time 2018-08-20 14:55 ` [PATCH] net: macb: do not disable MDIO bus at open/close time Anssi Hannula @ 2018-08-22 15:33 ` Claudiu Beznea 2018-08-23 7:45 ` [PATCH v2] " Anssi Hannula 0 siblings, 1 reply; 11+ messages in thread From: Claudiu Beznea @ 2018-08-22 15:33 UTC (permalink / raw) To: Anssi Hannula, netdev, Nicolas Ferre; +Cc: David S. Miller, Andrew Lunn 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] net: macb: do not disable MDIO bus at open/close time 2018-08-22 15:33 ` Claudiu Beznea @ 2018-08-23 7:45 ` Anssi Hannula 2018-08-24 14:47 ` Claudiu Beznea 2018-08-26 0:35 ` David Miller 0 siblings, 2 replies; 11+ messages in thread From: Anssi Hannula @ 2018-08-23 7:45 UTC (permalink / raw) To: netdev, Nicolas Ferre, Claudiu Beznea; +Cc: David S. Miller, Andrew Lunn 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. Fixes: 6c36a7074436 ("macb: Use generic PHY layer") Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi> --- 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 -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] net: macb: do not disable MDIO bus at open/close time 2018-08-23 7:45 ` [PATCH v2] " Anssi Hannula @ 2018-08-24 14:47 ` Claudiu Beznea 2018-08-26 0:35 ` David Miller 1 sibling, 0 replies; 11+ messages in thread From: Claudiu Beznea @ 2018-08-24 14:47 UTC (permalink / raw) To: Anssi Hannula, netdev, Nicolas Ferre; +Cc: David S. Miller, Andrew Lunn 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 <anssi.hannula@bitwise.fi> > --- Here: v2: Make macb_init_hw() NCR write a single statement. Other than this: Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com> Checked that link goes up and ping is working on a SAMA5D2 Xplained: Tested-by: Claudiu Beznea <claudiu.beznea@microchip.com> > 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] net: macb: do not disable MDIO bus at open/close time 2018-08-23 7:45 ` [PATCH v2] " Anssi Hannula 2018-08-24 14:47 ` Claudiu Beznea @ 2018-08-26 0:35 ` David Miller 1 sibling, 0 replies; 11+ messages in thread From: David Miller @ 2018-08-26 0:35 UTC (permalink / raw) To: anssi.hannula; +Cc: netdev, nicolas.ferre, Claudiu.Beznea, andrew From: Anssi Hannula <anssi.hannula@bitwise.fi> Date: Thu, 23 Aug 2018 10:45:22 +0300 > 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. > > Fixes: 6c36a7074436 ("macb: Use generic PHY layer") > Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi> Applied and queued up for -stable. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-08-26 4:16 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-08 12:19 [PATCH] net: macb: do not disable MDIO bus when closing interface Anssi Hannula 2018-08-09 8:26 ` Claudiu Beznea 2018-08-09 14:54 ` Anssi Hannula 2018-08-09 15:14 ` Andrew Lunn 2018-08-10 6:22 ` Anssi Hannula 2018-08-13 9:08 ` Claudiu Beznea 2018-08-20 14:55 ` [PATCH] net: macb: do not disable MDIO bus at open/close time Anssi Hannula 2018-08-22 15:33 ` Claudiu Beznea 2018-08-23 7:45 ` [PATCH v2] " Anssi Hannula 2018-08-24 14:47 ` Claudiu Beznea 2018-08-26 0:35 ` David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.