[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: New ACPI power resource code for testing
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
> 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) ...
> 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
Main Index |
Thread Index |