On Wed, Jul 2, 2025 at 11:22 AM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote: > > On Fri, Jun 13, 2025 at 3:48 PM Herve Codina <herve.codina@xxxxxxxxxxx> wrote: > > > > During driver removal, the following warning can appear: > > WARNING: CPU: 1 PID: 139 at drivers/base/core.c:1497 __device_links_no_driver+0xcc/0xfc > > ... > > Call trace: > > __device_links_no_driver+0xcc/0xfc (P) > > device_links_driver_cleanup+0xa8/0xf0 > > device_release_driver_internal+0x208/0x23c > > device_links_unbind_consumers+0xe0/0x108 > > device_release_driver_internal+0xec/0x23c > > device_links_unbind_consumers+0xe0/0x108 > > device_release_driver_internal+0xec/0x23c > > device_links_unbind_consumers+0xe0/0x108 > > device_release_driver_internal+0xec/0x23c > > driver_detach+0xa0/0x12c > > bus_remove_driver+0x6c/0xbc > > driver_unregister+0x30/0x60 > > pci_unregister_driver+0x20/0x9c > > lan966x_pci_driver_exit+0x18/0xa90 [lan966x_pci] > > > > This warning is triggered when a consumer is removed because the links > > status of its supplier is not DL_DEV_DRIVER_BOUND and the link flag > > DL_FLAG_SYNC_STATE_ONLY is not set. > > > > The topology in terms of consumers/suppliers used was the following > > (consumer ---> supplier): > > > > i2c -----------> OIC ----> PCI device > > | ^ > > | | > > +---> pinctrl ---+ > > > > When the PCI device is removed, the OIC (interrupt controller) has to be > > removed. In order to remove the OIC, pinctrl and i2c need to be removed > > and to remove pinctrl, i2c need to be removed. The removal order is: > > 1) i2c > > 2) pinctrl > > 3) OIC > > 4) PCI device > > > > In details, the removal sequence is the following (with 0000:01:00.0 the > > PCI device): > > driver_detach: call device_release_driver_internal(0000:01:00.0)... > > device_links_busy(0000:01:00.0): > > links->status = DL_DEV_UNBINDING > > device_links_unbind_consumers(0000:01:00.0): > > 0000:01:00.0--oic link->status = DL_STATE_SUPPLIER_UNBIND > > call device_release_driver_internal(oic)... > > device_links_busy(oic): > > links->status = DL_DEV_UNBINDING > > device_links_unbind_consumers(oic): > > oic--pinctrl link->status = DL_STATE_SUPPLIER_UNBIND > > call device_release_driver_internal(pinctrl)... > > device_links_busy(pinctrl): > > links->status = DL_DEV_UNBINDING > > device_links_unbind_consumers(pinctrl): > > pinctrl--i2c link->status = DL_STATE_SUPPLIER_UNBIND > > call device_release_driver_internal(i2c)... > > device_links_busy(i2c): links->status = DL_DEV_UNBINDING > > __device_links_no_driver(i2c)... > > pinctrl--i2c link->status is DL_STATE_SUPPLIER_UNBIND > > oic--i2c link->status is DL_STATE_ACTIVE > > oic--i2c link->supplier->links.status is DL_DEV_UNBINDING > > > > The warning is triggered by the i2c removal because the OIC (supplier) > > links status is not DL_DEV_DRIVER_BOUND. Its links status is indeed set > > to DL_DEV_UNBINDING. > > > > It is perfectly legit to have the links status set to DL_DEV_UNBINDING > > in that case. Indeed we had started to unbind the OIC which triggered > > the consumer unbinding and didn't finish yet when the i2c is unbound. > > > > Avoid the warning when the supplier links status is set to > > DL_DEV_UNBINDING and thus support this removal sequence without any > > warnings. > > > > Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx> > > The change is OK, so > > Reviewed-by: Rafael J. Wysocki <rafael@xxxxxxxxxx> You can add mine too. Reviewed-by: Saravana Kannan <saravanak@xxxxxxxxxx> -Saravana > > but it will clash with > > https://lore.kernel.org/linux-pm/2793309.mvXUDI8C0e@xxxxxxxxxxxxx/ > > that's been queued up by Greg AFAICS. > > > --- > > drivers/base/core.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > index 8fead097c404..ce367c44f642 100644 > > --- a/drivers/base/core.c > > +++ b/drivers/base/core.c > > @@ -1494,7 +1494,8 @@ static void __device_links_no_driver(struct device *dev) > > if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) { > > WRITE_ONCE(link->status, DL_STATE_AVAILABLE); > > } else { > > - WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY)); > > + WARN_ON(link->supplier->links.status != DL_DEV_UNBINDING && > > + !(link->flags & DL_FLAG_SYNC_STATE_ONLY)); > > WRITE_ONCE(link->status, DL_STATE_DORMANT); > > } > > } > > -- > > 2.49.0 > >
