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