linux-kernel - Re: [PATCH] Revert "of: property: fw_devlink: Add support for "phy-handle" property"

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:   Fri, 17 Sep 2021 09:58:57 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Saravana Kannan <saravanak@...gle.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Russell King <linux@...linux.org.uk>, kernel-team@...roid.com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Revert "of: property: fw_devlink: Add support for
 "phy-handle" property"

On 9/16/21 7:27 PM, Saravana Kannan wrote:
> On Thu, Sep 16, 2021 at 7:21 PM Florian Fainelli <f.fainelli@...il.com> wrote:
>>
>>
>>
>> On 9/15/2021 1:19 AM, Saravana Kannan wrote:
>>> This reverts commit cf4b94c8530d14017fbddae26aad064ddc42edd4.
>>>
>>> Some PHYs pointed to by "phy-handle" will never bind to a driver until a
>>> consumer attaches to it. And when the consumer attaches to it, they get
>>> forcefully bound to a generic PHY driver. In such cases, parsing the
>>> phy-handle property and creating a device link will prevent the consumer
>>> from ever probing. We don't want that. So revert support for
>>> "phy-handle" property until we come up with a better mechanism for
>>> binding PHYs to generic drivers before a consumer tries to attach to it.
>>>
>>> Signed-off-by: Saravana Kannan <saravanak@...gle.com>
>>
>> Thanks for getting this revert submitted, I just ran a bisection this
>> afternoon that pointed to this offending commit. It would cause the dead
>> lock
> 
> Dead lock in the kernel? Or do you mean just a hang waiting forever for network?

It locks up since we try to acquire __device_driver_lock() while we are
in the main driver's probe function.

> 
>> on boot with drivers/net/dsa/bcm_sf2.c pasted below.
> 
> The log is too jumbled up to be readable (word wrap I suppose) and
> maybe even multiple thread printing at the same time.

Re-attached (thunderbird is not really helping me).

> 
>> Saravana, can
>> you CC on me on your fix or what you would want me to be testing?
> 
> By fix, I assume you mean when I bring back phy-handle with a proper
> fix to handle the case in the commit text? Yeah, that's going to take
> a while. It's brewing in my head and there are some issues that's not
> fully resolved. But I haven't had time to code it up or dig into the
> net code to make sure it'll work. But yes, I'll CC you when I do so
> you can test it with this case.

Well by fix, I meant something that does not lock up on my system, it is
a different problem from supporting 'phy-handle', but it should not
regress an existing system, no matter how quirky that system behaves in
its probe function. For history and reference, the "offending" change
and background can be found here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=771089c2a485958e423f305e974303760167b45c

Thanks for your patience working on the quirky MDIO/PHY subsystem :)
-- 
Florian

View attachment "7445d0-lockup.log" of type "text/x-log" (22518 bytes)