netdev - Re: MDIO over I2C driver driver probe dependency issue

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] [day] [month] [year] [list]

Message-ID: <X/j8qw+g1P7WCmlH@lunn.ch>
Date:   Sat, 9 Jan 2021 01:45:31 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Brian Silverman <silvermanbri@...il.com>, netdev@...r.kernel.org
Subject: Re: MDIO over I2C driver driver probe dependency issue

On Fri, Jan 08, 2021 at 02:11:31PM -0800, Florian Fainelli wrote:
> On 1/8/2021 1:04 PM, Andrew Lunn wrote:
> > On Fri, Jan 08, 2021 at 03:02:52PM -0500, Brian Silverman wrote:
> >> Thanks for the responses - I now have a more clear picture of what's going on.
> >>  (Note: I'm using Xilinx's 2019.2 kernel (based off 4.19).  I believe it would
> >> be similar to latest kernels, but I could be wrong.)
> > 
> > Hi Brian
> > 
> > macb_main has had a lot of changes with respect to PHYs. Please try
> > something modern, like 5.10.
> 
> It does not seem to me like 5.10 will be much better, because we have
> the following in PHYLINK:
> 
> int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn,
>                              u32 flags)
> ...
>           phy_dev = of_phy_find_device(phy_node);
>           /* We're done with the phy_node handle */
>           of_node_put(phy_node);
>           if (!phy_dev)
>                   return -ENODEV;
> 
> Given Brian's configuration we should be returning -EPROBE_DEFER here,
> but doing that would likely break a number of systems that do expect
> -ENODEV to be returned.

I just looked through the current users of phylink_of_phy_connect().
Most simply do a netdev_err() and return the error code to higher
levels. So apart from the spurious netdev_err() is -EPROBE_DEFER is
returned, they should do the right thing.

mvneta actually looks broken, it prints the error, but keeps going,
plays with WOL setings on the phy and device_set_wake() then returns
the error code.

macb is a bit more complex, but if i'm understanding it correctly, it
should handle -EPROBE_DEFER already, but you will get a spurious
netdev_err() for the -EPROBE_DEFER.

So i think we can fix this, and we should probably do it before there
are more users.

Brian, can you run a modern kernel to test patches, or do you need to
use the Xilinx fork?

    Andrew