linux-kernel - Re: [RFC PATCH] phylib: mdio: handle register/unregister/register sequence

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]

Date:	Thu, 22 Aug 2013 14:15:52 +0100
From:	Florian Fainelli <f.fainelli@...il.com>
To:	boris brezillon <b.brezillon@...rkiz.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Mark Brown <broonie@...nel.org>,
	Nick Bowler <nbowler@...iptictech.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	netdev <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH] phylib: mdio: handle register/unregister/register sequence

2013/8/22 boris brezillon <b.brezillon@...rkiz.com>:
> Hello Florian,
>
> Thanks for your answer.
>
>
> On 22/08/2013 14:43, Florian Fainelli wrote:
>>
>> Hello Boris,
>>
>> 2013/8/22 Boris BREZILLON <b.brezillon@...rkiz.com>:
>>>
>>> Hello,
>>>
>>> This patch is a proposal to support the register/unregister/register
>>> sequence on a given mdio bus.
>>>
>>> I use the register/unregister/register sequence to add a fallback when
>>> the
>>> of_mdiobus_register (this function calls mdiobus_register with phy_mask
>>> set to ~0) does not register any phy device (because the device tree does
>>> not define any phy).
>>> In this case I call mdiobus_unregister and then call mdiobus_register
>>> with
>>> a phy_mask set to 0 to trigger a full mdio bus scan.
>>>
>>> I'm not sure this is the right way to do it (this is why I added RFC in
>>> the
>>> subject).
>>>
>>> Could someone help me figuring out what I should use to implement my
>>> fallback ?
>>>
>>> 1) use the register/unregister/register sequence
>>> 2) reimplement the "for (i = 0; i < PHY_MAX_ADDR; i++)" mdiobus_scan loop
>>
>> I think solution 2 is nicer, in that case, would it be enough in your
>> case to export a function called mdiobus_scan()? You could call at a
>> time you know PHY devices have a chance of having been probed?
>
> mdiobus_scan is already exported:
> struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);
>
> This function scans the presence of a phy device at a given address.
>
> What I need is a loop which scan all the possible address on the given
> mdio bus:
>
> struct phy_device *mdiobus_full_scan(struct mii_bus *bus)
> {
>     int i;
>     for (i = 0; i < PHY_MAX_ADDR; i++) {
>         if ((bus->phy_mask & (1 << i)) == 0) {
>             struct phy_device *phydev;
>
>             phydev = mdiobus_scan(bus, i);
>             if (IS_ERR(phydev)) {
>                 err = PTR_ERR(phydev);
>                 goto error;
>             }
>         }
>     }
>     return 0;
>
> error:
>     while (--i >= 0) {
>
>         if (bus->phy_map[i])
>             device_unregister(&bus->phy_map[i]->dev);
>     }
> }
> EXPORT_SYMBOL(mdiobus_full_scan);
>
> Since I am the only one who need this kind of functionnality right now, I'm
> not sure
> this is a good idea to export a new function.

A possible other use case for this full-scan is when you do not detect
a PHY connected to your MDIO bus, and that you did not register a
fixed PHY early enough for it to have been scanned by the fixed MDIO
bus emulation. In that case the driver may:

- scan hardware MDIO bus
- do not find any PHY, register a fixed PHY
- trigger a fixed MDIO bus full-rescan
- attach to the discovered fixed PHY

this is something currently done by the TI CPMAC driver in
drivers/net/ethernet/ti/cpmac.c (altough fixed_phy_add() is called
from platform code).

>
> This behaviour may be implemented in the of_mdiobus_register function:
> when no dt phy node are found in the mdio bus dt node, we could launch a
> full
> scan.
>
> What do you think ?

There is an existing kind of "autoscan" feature in
drivers/of/of_mdio.c, starting with the second foreach_child_node()
loop, so maybe that specific part could be exported and would achieve
what you are looking for? It relies on the Ethernet PHY nodes to be
attached to the MDIO bus node, but I assume this is what ultimately
happens in your case as well?
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/