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.hann...@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