netdev - Re: [PATCH v2] net: macb: do not disable MDIO bus at open/close time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4800e8c0-5282-a01b-ba15-a0c01ad08038@microchip.com>
Date:   Fri, 24 Aug 2018 17:47:52 +0300
From:   Claudiu Beznea <Claudiu.Beznea@...rochip.com>
To:     Anssi Hannula <anssi.hannula@...wise.fi>, <netdev@...r.kernel.org>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH v2] net: macb: do not disable MDIO bus at open/close time



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@...wise.fi>
> ---

Here:

v2: Make macb_init_hw() NCR write a single statement.

Other than this: 

Reviewed-by: Claudiu Beznea <claudiu.beznea@...rochip.com>

Checked that link goes up and ping is working on a SAMA5D2 Xplained:

Tested-by: Claudiu Beznea <claudiu.beznea@...rochip.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
>