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: <edd7aaf4-53dc-6d10-5c4b-2928a4612d99@bitwise.fi>
Date:   Fri, 10 Aug 2018 09:22:05 +0300
From:   Anssi Hannula <anssi.hannula@...wise.fi>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Claudiu Beznea <Claudiu.Beznea@...rochip.com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] net: macb: do not disable MDIO bus when closing interface

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