Re: New ACPI power resource code for testing

[Jukka Ruohonen <jruohonen%iki.fi@localhost>]

First, thanks for comments!

On Mon, Mar 01, 2010 at 06:14:37AM -0800, Paul Goyette wrote:
>       * Called from acpi framework before attachment of ACPI
>       * device nodes

Will do.

> In acpi_power_register() the DPRINTF(()) hardwires the text "acpi0" in 
> the message;  might be better to grab this from the current device's 
> parent?  Same for several other DPRINTF(())s...

That could be done, yes. Though these are only DPRINTFs...

I thought about requiring a device_t as a parameter to all exposed
functions, but then again, it is not entirely clear if all possible users of
power resources are actual devices with a device_t. So, perhaps no.

> In acpi_power_switch(), since variable "on" is a boolean, why compare it 
> to another boolean?
> 
>       if (on != false) ...
>    vs
>       if (on) ...

This is just my personal coding style. I always like to explicitly compare
things, and despise a style that, say, does things like (!p), especially if
pointers are involved. But each to their own.

> As you've currently set things up, the user MUST include
> 
>       acpipower* at acpi?
> 
> in their config file if they have acpi at all - even if their system 
> does not have any acpipower resources.  (Otherwise, the call to 
> acpi_power_init() will refer to an undefined symbol.)  Either this 
> requirement should be removed, or it should be explicitly documented.

Yes, these attach as normal ACPI devices (the version you reviewed earlier
did not do this). I thought this would have some benefits (e.g. easier
modularization and possible integration with e.g. pmf(9), etc.), but
changing this is only a matter of small refactoring.

Yet, conceptually these are sort-of "[device] nodes", even though the
current power resource code is builtin to the core ACPI.

> (Would it be valid to use RUN_ONCE(9) here?  And then call this from 
> the attach routine?)

I thought about this, but it won't fly as the globals need to be initialized
even if no power resources are available.

> dependency with acpitz.  Is it conceivable that an acpitz can be fully 
> functional without needing the acpipower device?  (I'm thinking of 
> passive-only situations.)  If so, then the dependency should be removed.

Yes, it is possible, even likely. Perhaps some autoconfiguration glue would
help here?

- Jukka.