linux-kernel - Re: [PATCH] net: mdio: force deassert MDIO reset signal

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: <54eb0ee4-7d9e-7025-f984-b1e026c18c3d@metafoo.de>
Date:   Tue, 17 Jan 2023 05:48:34 -0800
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Pierluigi Passaro <pierluigi.passaro@...il.com>,
        hkallweit1@...il.com, linux@...linux.org.uk, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        eran.m@...iscite.com, nate.d@...iscite.com,
        francesco.f@...iscite.com, pierluigi.p@...iscite.com
Subject: Re: [PATCH] net: mdio: force deassert MDIO reset signal

On 1/17/23 05:40, Andrew Lunn wrote:
>>> So compatible "ethernet-phy-ieee802.3-c45" results in is_c45 being set
>>> true. The if (is_c45 || is then true, so it does not need to call
>>> fwnode_get_phy_id(child, &phy_id) so ignores whatever ID is in DT and
>>> asks the PHY.
>>>
>>> Try this, totally untested:
>>>
>>> diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
>>> index b782c35c4ac1..13be23f8ac97 100644
>>> --- a/drivers/net/mdio/fwnode_mdio.c
>>> +++ b/drivers/net/mdio/fwnode_mdio.c
>>> @@ -134,10 +134,10 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
>>>           if (rc >= 0)
>>>                   is_c45 = true;
>>> -       if (is_c45 || fwnode_get_phy_id(child, &phy_id))
>>> +       if (fwnode_get_phy_id (child, &phy_id))
>>>                   phy = get_phy_device(bus, addr, is_c45);
>>>           else
>>> -               phy = phy_device_create(bus, addr, phy_id, 0, NULL);
>>> +               phy = phy_device_create(bus, addr, phy_id, is_c45, NULL);
>>>           if (IS_ERR(phy)) {
>>>                   rc = PTR_ERR(phy);
>>>                   goto clean_mii_ts;
>>>
>> I think part of the problem is that for C45 there are a few other fields
>> that get populated by the ID detection, such as devices_in_package and
>> mmds_present. Is this something we can do after running the PHY drivers
>> probe function? Or is it too late at that point?
> As i hinted, it needs somebody to actually debug this and figure out
> why it does not work.
>
> I think what i did above is part of the solution. You need to actually
> read the ID from the DT, which if you never call fwnode_get_phy_id()
> you never do.
>
> You then need to look at phy_bus_match() and probably remove the
>
> 		return 0;
> 	} else {
>
> so that C22 style ID matching is performed if matching via
> c45_ids.device_ids fails.

Sorry, I've should have been more clear. I did try your proposed change 
a while ago. The problem why it does not work is that there are other 
fields in the phy data structure that get initialized when reading the 
IDs for C45. Such as which MMD addresses are valid.