Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <54eb0ee4-7d9e-7025-f984-b1e026c18c3d@metafoo.de>
Date:   Tue, 17 Jan 2023 05:48:34 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.0
Subject: Re: [PATCH] net: mdio: force deassert MDIO reset signal
Content-Language: en-US
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Pierluigi Passaro <pierluigi.passaro@gmail.com>,
        hkallweit1@gmail.com, linux@armlinux.org.uk, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        eran.m@variscite.com, nate.d@variscite.com,
        francesco.f@variscite.com, pierluigi.p@variscite.com
References: <20230115161006.16431-1-pierluigi.p@variscite.com>
 <Y8QzI2VUY6//uBa/@lunn.ch> <f691f339-9e50-b968-01e1-1821a2f696e7@metafoo.de>
 <Y8SSb+tJsfJ3/DvH@lunn.ch> <cc338014-8a2b-87e9-7684-20b57aae4ac3@metafoo.de>
 <Y8alV6FUKsN2x2XZ@lunn.ch>
From:   Lars-Peter Clausen <lars@metafoo.de>
In-Reply-To: <Y8alV6FUKsN2x2XZ@lunn.ch>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.