On Fri, 20 Jul 2007, Andrew Doran wrote:
> 1) +	callout_init(&dev->dv_dd.dd_idle, CALLOUT_MPSAFE);
>
> From what I remember seeing when I looked, the callout doesn't do locking so
> it's not MP safe. Until the soft interrupt changes from the vmlocking branch
> go in it's actually quite difficult to do that since there is an ordering
> constraint between the kernel_lock and all spinlocks in the kernel.

Alright, that could explain some things :-)

> 2) +MALLOC_DEFINE(M_DEVPM, "devpm", "device power management memory");
>
> If you're not going to allocate or free memory from an interrupt handler,
> then kmem_alloc/kmem_free are a better choice.

Consider it done.

> I have some more general comments, and I'm going to display my ignorance of
> power management here.
>
> In what order do devices get notified of events? Does the bus driver get
> notified first or its children first? Naively "leaf drivers first, nexus
> drivers after that" seems to be the right order.

Depends on the event. If you're shutting down, you need to do leaf -> 
nexus; on resume, you need to run them nexus -> leaf. I have the code to 
do this now, and have it integrated with ACPI so it runs power hooks in 
the proper order.

> What kind of overlap is there with the shutdown hooks?

None, although it would be possible to do this also.

> I've given the whole hooks framework a bit of thought over time and I really
> don't like it. From the perspective of providing a stable driver API and
> making loadable drivers work well, it's crummy. It encodes information in
> the driver that the driver shouldn't know about: calls to establish the
> hooks. And, I think that any shutdown/power hook established outside a
> device driver is evidence of a missing driver. :-)
>
> What I was thinking of is to have the power management bits be something
> that every device in the system has (so they are part of device_t), and have
> an optional device entry point in cfattach_t that handles these kinds of
> requests - some kind of basic message passing interface, so that we can
> extend or version it easily enough later.

With my current patches, I whine and complain at boot if a driver doesn't 
support power management. Once I get a bit further, I plan on making the 
"put system to sleep" path depend on every single driver instance 
supporting it or the request to go to sleep will be denied.

I also thought of making it part of cfattach_t; I do need to run some 
things at attach time depending on device class, but that call could 
easily be made from subr_autoconf.

As for 'basic message passing interface', that's kind've where I've gone 
with this since the last patch I posted -- so much that I'm tempted to 
change the name of this science project. Apart from being able to shut 
down and restore a device's state at any time, I also have things like 
volume buttons, lid switches, and LCD backlights talking to each other. 
For example, if the lid switch is pressed, a SET_DISPLAY_POWER message is 
broadcast to all display devices with off as an argument vesafb's power 
handler handles this, and turns off my display when I close the lid. 
Whenever a key is pressed on my keyboard, the display is powered on (if 
not already), and can slowly power back down when idle. The volume buttons 
broadcast a SET_AUDIO_VOLUME message, which is handled by DV_AUDIODEV 
class devices -- basically, any instance of audio(4).

It's because of this that I was thinking of renaming this to kern_pnp, 
although some have suggested kern_jfw :-)

> Where that may also help is when we clean up the locking protocol around
> autoconf (e.g. preventing devices from appearing or disappearing when
> something's talking with them).

Cheers,
Jared