mbox series

[v3,0/3] fw_devlink bug fixes

Message ID 20210915170940.617415-1-saravanak@google.com (mailing list archive)
Headers show
Series fw_devlink bug fixes | expand

Message

Saravana Kannan Sept. 15, 2021, 5:09 p.m. UTC
Intended for 5.15.

Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Vladimir Oltean <olteanv@gmail.com>

v1->v2:
- Added a few Reviewed-by and Tested-by tags
- Addressed Geert's comments in patches 3 and 5
- Dropped the fw_devlink.debug patch
- Added 2 more patches to the series to address other fw_devlink issues

v2->v3:
- Split the logging/debug changes into a separate series

Thanks,
Saravana

Saravana Kannan (3):
  driver core: fw_devlink: Improve handling of cyclic dependencies
  driver core: fw_devlink: Add support for
    FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD
  net: mdiobus: Set FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD for mdiobus
    parents

 drivers/base/core.c        | 36 +++++++++++++++++++++++++++++++-----
 drivers/net/phy/mdio_bus.c |  4 ++++
 include/linux/fwnode.h     | 11 ++++++++---
 3 files changed, 43 insertions(+), 8 deletions(-)

Comments

Greg Kroah-Hartman Sept. 23, 2021, 5:30 p.m. UTC | #1
On Wed, Sep 15, 2021 at 10:09:36AM -0700, Saravana Kannan wrote:
> Intended for 5.15.
> 
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Vladimir Oltean <olteanv@gmail.com>
> 
> v1->v2:
> - Added a few Reviewed-by and Tested-by tags
> - Addressed Geert's comments in patches 3 and 5
> - Dropped the fw_devlink.debug patch
> - Added 2 more patches to the series to address other fw_devlink issues
> 
> v2->v3:
> - Split the logging/debug changes into a separate series

I have taken this now into my tree.

It fixes the real problem where drivers were making the wrong assumption
that if they registered a device, it would be instantly bound to a
driver.  Drivers that did this were getting lucky, as this was never a
guarantee of the driver core (think about if you enabled async
probing, and the mess with the bus specific locks that should be
preventing much of this)

With this new flag, we can mark these drivers/busses that have this
assumption and work to solve correctly over time.  The issue with using
a "generic vs. specific" driver is a bit related, I'm amazed that a
subsystem actually implemented it this way, others of us have been
avoiding this for a very long time due to the complexity involved when
things are built as modules.

thanks,

greg k-h
Vladimir Oltean Sept. 23, 2021, 7:44 p.m. UTC | #2
On Thu, Sep 23, 2021 at 07:30:04PM +0200, Greg Kroah-Hartman wrote:
> It fixes the real problem where drivers were making the wrong assumption
> that if they registered a device, it would be instantly bound to a
> driver.  Drivers that did this were getting lucky, as this was never a
> guarantee of the driver core (think about if you enabled async
> probing, and the mess with the bus specific locks that should be
> preventing much of this)

Since commit d173a137c5bd ("driver-core: enable drivers to opt-out of
async probe") it is possible to opt out of async probing, and PHY
drivers do opt out of it, at the time of writing.
Florian Fainelli Sept. 23, 2021, 10:28 p.m. UTC | #3
On 9/15/21 10:09 AM, Saravana Kannan wrote:
> Intended for 5.15.
> 
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Vladimir Oltean <olteanv@gmail.com>
> 
> v1->v2:
> - Added a few Reviewed-by and Tested-by tags
> - Addressed Geert's comments in patches 3 and 5
> - Dropped the fw_devlink.debug patch
> - Added 2 more patches to the series to address other fw_devlink issues
> 
> v2->v3:
> - Split the logging/debug changes into a separate series
> 
> Thanks,
> Saravana
> 
> Saravana Kannan (3):
>   driver core: fw_devlink: Improve handling of cyclic dependencies
>   driver core: fw_devlink: Add support for
>     FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD
>   net: mdiobus: Set FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD for mdiobus
>     parents

Andrew, did you get a chance to test this patch set on a ZII development
board rev B or C by any chance?
Greg Kroah-Hartman Sept. 24, 2021, 6:09 a.m. UTC | #4
On Thu, Sep 23, 2021 at 10:44:48PM +0300, Vladimir Oltean wrote:
> On Thu, Sep 23, 2021 at 07:30:04PM +0200, Greg Kroah-Hartman wrote:
> > It fixes the real problem where drivers were making the wrong assumption
> > that if they registered a device, it would be instantly bound to a
> > driver.  Drivers that did this were getting lucky, as this was never a
> > guarantee of the driver core (think about if you enabled async
> > probing, and the mess with the bus specific locks that should be
> > preventing much of this)
> 
> Since commit d173a137c5bd ("driver-core: enable drivers to opt-out of
> async probe") it is possible to opt out of async probing, and PHY
> drivers do opt out of it, at the time of writing.

That's good, but we are talking about system-wide enabling of async
probing in the future, which might cause problems here :)

Anyway, let's go with this option for now and Saravana has assured me
that he will work on fixing up these drivers/bus to work properly going
forward.

thanks,

greg k-h