linux-kernel - Re: [PATCH] net: phy: allow scanning busses with missing phys

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: <20180424180153.GD2360@lunn.ch>
Date:   Tue, 24 Apr 2018 20:01:53 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "David S . Miller" <davem@...emloft.net>,
        Allan Nielsen <Allan.Nielsen@...rosemi.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: phy: allow scanning busses with missing phys

On Tue, Apr 24, 2018 at 09:37:09AM -0700, Florian Fainelli wrote:
> 
> 
> On 04/24/2018 09:09 AM, Alexandre Belloni wrote:
> > Some MDIO busses will error out when trying to read a phy address with no
> > phy present at that address. In that case, probing the bus will fail
> > because __mdiobus_register() is scanning the bus for all possible phys
> > addresses.
> > 
> > In case MII_PHYSID1 returns -EIO or -ENODEV, consider there is no phy at
> > this address and set the phy ID to 0xffffffff which is then properly
> > handled in get_phy_device().
> 
> Humm, why not have your MDIO bus implementation do the scanning itself
> in a reset() callback, which happens before probing the bus, and based
> on the results, set phy_mask accordingly such that only PHYs present are
> populated?

Hi Florian

Seems a bit odd have the driver do this, when the core could.

> My only concern with your change is that we are having a special
> treatment for EIO and ENODEV, so we must make sure MDIO bus drivers are
> all conforming to that.
 
I don't see how it could be a problem. It used to be any error was a
real error, and would stop the bus from loading. It now means there is
nothing there. The only possible side effect is an mdio bus driver
might remain loaded without any devices if all reads return
ENODEV/EIO, were as before it would probably never load. 

	    Andrew