Re: PM domain change on unbound devices warning on ipmi_si unload

[Corey Minyard]

Tomeu, you added that check in

[989561de9b5112999475b406557d9c7e9e59c041] PM / Domains: add setter for 
dev.pm_domain

and either something is wrong in the platform device handling or 
elsewhere in the device code, if
that check is valid.

-corey

On 01/28/2016 07:56 AM, Corey Minyard wrote:
> On 01/27/2016 10:45 PM, Joe Lawrence wrote:
> > Starting in 4.5-rc1, I noticed this warning on ipmi_si driver removal:
> >
> > % modprobe ipmi_si
> > % rmmod ipmi_si
>
> Yes, I had just noticed this yesterday and I was going to try to trace 
> it down today.
> I had assumed it was an IPMI driver problem, but after looking at it a 
> bit, I don't think
> it is.  I'll try to bisect to see what caused this, nothing obvious 
> jumps out
>
> -corey
>
> > bus: 'platform': driver_probe_device: matched device IPI0001:00 with 
> > driver ipmi_si
> > bus: 'platform': really_probe: probing driver ipmi_si with device 
> > IPI0001:00
> > ipmi_si IPI0001:00: ipmi_si: probing via ACPI
> > ipmi_si IPI0001:00: [io  0x0ca2-0x0ca3] regsize 1 spacing 1 irq 0
> > ipmi_si: Adding ACPI-specified kcs state machine
> > driver: 'ipmi_si': driver_bound: bound to device 'IPI0001:00'
> > bus: 'platform': really_probe: bound device IPI0001:00 to driver ipmi_si
> > IPMI System Interface driver.
> > ipmi_si: probing via SMBIOS
> > ipmi_si: SMBIOS: io 0xda2 regsize 1 spacing 1 irq 0
> > ipmi_si: Adding SMBIOS-specified kcs state machine
> > ipmi_si: Trying ACPI-specified kcs state machine at i/o address 
> > 0xca2, slave address 0x0, irq 0
> > Registering platform device 'ipmi_bmc.0674.66'. Parent at platform
> > driver: 'ipmi': driver_bound: bound to device 'ipmi_bmc.0674.66'
> > ipmi_si IPI0001:00: Found new BMC (man_id: 0x000077, prod_id: 0x0674, 
> > dev_id: 0x42)
> > ipmi_si IPI0001:00: IPMI kcs interface initialized
> > ------------[ cut here ]------------
> > WARNING: CPU: 39 PID: 3678 at drivers/base/power/common.c:150 
> > dev_pm_domain_set+0x52/0x60()
> > PM domains can only be changed for unbound devices
> > [ ... snip ... ]
> > CPU: 39 PID: 3678 Comm: rmmod Tainted: G           OE 4.5.0-rc1+ #57
> > Hardware name: Stratus ftServer 6800/G7LYY, BIOS BIOS Version 8.1:61 
> > 09/10/2015
> > 0000000000000000 000000003883b68d ffff8820334d7d30 ffffffff8132caf0
> > ffff8820334d7d78 ffff8820334d7d68 ffffffff8107f1b6 ffff8810351eca00
> > 0000000000000000 0000000000000001 0000000001c2a330 0000000001c29010
> > Call Trace:
> > [<ffffffff8132caf0>] dump_stack+0x44/0x64
> > [<ffffffff8107f1b6>] warn_slowpath_common+0x86/0xc0
> > [<ffffffff8107f24c>] warn_slowpath_fmt+0x5c/0x80
> > [<ffffffff8145e152>] dev_pm_domain_set+0x52/0x60
> > [<ffffffff813a38ae>] acpi_dev_pm_detach+0x3f/0x84
> > [<ffffffff8145e0d7>] dev_pm_domain_detach+0x27/0x30
> > [<ffffffff814575f8>] platform_drv_remove+0x38/0x40
> > [<ffffffff814557ba>] __device_release_driver+0x9a/0x140
> > [<ffffffff81455968>] driver_detach+0xb8/0xc0
> > [<ffffffff814547b5>] bus_remove_driver+0x55/0xd0
> > [<ffffffff814560cc>] driver_unregister+0x2c/0x50
> > [<ffffffff814576b2>] platform_driver_unregister+0x12/0x20
> > [<ffffffffa02586c9>] cleanup_ipmi_si+0x29/0xa0 [ipmi_si]
> > [<ffffffff81102100>] SyS_delete_module+0x190/0x220
> > [<ffffffff8167ffee>] entry_SYSCALL_64_fastpath+0x12/0x71
> > ---[ end trace 671ca97b9ac15462 ]---
> >
> > My platform has two BMCs (perhaps this is messing with a refcount
> > somewhere), but I wonder about the ordering of this code:
> >
> > __device_release_driver(struct device *dev)
> >
> >    drv->remove(dev);
> >      [ platform_drv_remove ]
> >        ...
> >        dev_pm_domain_detach
> >          device_is_bound
> >            return dev->p && klist_node_attached(&dev->p->knode_driver)
> >    ...
> >    klist_remove(&dev->p->knode_driver);
> >
> > Is the klist_remove at the bottom of __device_release_driver necessary
> > to satisfy the earlier check in dev_pm_domain_detach's device_is_bound
> > assertion?  If so, could these be out of order?
> >
> > This is core driver code, so I'm assuming it's not something as simple
> > as the following (which avoided the warning on unload at least).  Any
> > suggestions or extra debugging ideas welcome!  This occurs on every
> > unload, so I'd be glad to test real solutions :)
> >
> > Thanks,
> >
> > -- Joe
> >
> > -->8--
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index c4da2df..bba54e1 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -756,6 +756,7 @@ static void __device_release_driver(struct device 
> > *dev)
> >
> >           pm_runtime_put_sync(dev);
> >
> > +        klist_remove(&dev->p->knode_driver);
> >           if (dev->bus && dev->bus->remove)
> >               dev->bus->remove(dev);
> >           else if (drv->remove)
> > @@ -767,7 +768,6 @@ static void __device_release_driver(struct device 
> > *dev)
> >               dev->pm_domain->dismiss(dev);
> >           pm_runtime_reinit(dev);
> >
> > -        klist_remove(&dev->p->knode_driver);
> >           device_pm_check_callbacks(dev);
> >           if (dev->bus)
> > blocking_notifier_call_chain(&dev->bus->p->bus_notifier,