Re: PCI PM & compatibility

[Russell King <rmk@arm.linux.org.uk>]

On Tue, Aug 26, 2003 at 08:31:55AM -0700, Patrick Mochel wrote:
> > So in one case, we have PARENT SIBLING SIBLING SIBLING and in the
> > other case we have SIBLING SIBLING SIBLING PARENT.
> 
> I haven't been ignoring you. I've been trying to figure exactly what 
> you're talking about. Do you have a concrete example? Is it some PCMCIA 
> driver, or some platform driver? 

It's a multi-function chip (again):

            sa1111 bus
    SA1111------+-------Interrupt controller
                +-------USB host
                +-------PS/2 ports

The SA1111 probe function registers devices whose parent is the SA1111
device.

In this case, we end up placing these devices on the list in the wrong
order.

At the point where the SA1111 probe function is called, the SA1111 device
is not on the dpm lists, because of the ordering in device_add().

When we add the USB host, PS/2 ports etc, their parent device has not
been added to the dpm lists.  Only after the SA1111 probe function
completes will the SA1111 device appear on the dpm lists, and in this
case it will appear _after_ the USB and PS/2 ports.

The effect of this is that when we do a device suspend, we suspend
the SA1111, then the PS/2 ports, then the USB host.  Unfortunately
this can't work because suspending the SA1111 prevents access to
these sub-devices.

> I fail to see the problem, in theory, besides the driver itself. In the
> case where the driver registers first, ->probe() will not be called until
> it matches a device, which implies the device will have been added before
> ->probe() is called.

This doesn't happen - take a careful look at device_add(), specifically
the order of bus_add_device() and device_pm_add().  We probe the device
_before_ we add it to the PM lists.

> > Now, we do have this pm_users thing which seems to imply that it
> > describes the relationship between two devices.  However, its non-
> > functional.  It operates on a per-device variable called "pm_users"
> > which is only ever _written_ !
> 
> Hey, I hadn't finished that part, because I considered fixing the system 
> power management interface more important that day. 
> 
> It works like this: 
> 
> - A driver is able to set ->power.pm_parent for a device by calling 
>   device_pm_set_parent() after its registered. By default, pm_parent is 
>   set to the phsyical parent of the device. 
> 
> - A device with other devices that are dependent on it for power has a 
>   positive pm_users count. 
> 
> - A device may only be suspended when pm_users == 0. 
> 
> - When a device is suspended, the pm_users count of its pm_parent is 
>   decremented, and incremented when the device is resumed. 
> 
> - device_suspend() makes multiple passes over the device list, in case
>   power dependencies cause some devices to be deferred. It fails with an 
>   error (and resumes all suspended devices) if a pass was made in which 
>   no devicse were suspended, but there are still devices with a positive
>   pm_users count. 
> 
> Sound good? 

It sounds more complicated than our old system, but I'm willing to give
it a go.  However, some drivers only want to be notified of resume events,
so this would involve more code to explicitly handle the suspend case in
such drivers.

Dunno, I'll wait until there's a patch available and see how well it fits.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html