netdev - Re: [PATCH] net: macb: do not disable MDIO bus when closing interface

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: <425ed5af-eac2-a58b-b6ca-f022a80367e4@microchip.com>
Date:   Thu, 9 Aug 2018 11:26:17 +0300
From:   Claudiu Beznea <Claudiu.Beznea@...rochip.com>
To:     Anssi Hannula <anssi.hannula@...wise.fi>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        "David S. Miller" <davem@...emloft.net>
CC:     <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: macb: do not disable MDIO bus when closing interface



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@...wise.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);
>