Re: [PATCH_v8 1/2] IPMI/ACPI: Define acpi_ipmi notifier hook functions

[Corey Minyard <minyard@acm.org>]

On 07/26/2010 11:52 AM, Bjorn Helgaas wrote:
> On Monday, July 26, 2010 08:46:04 am yakui.zhao@intel.com wrote:
>    
>> From: Zhao yakui<yakui.zhao@intel.com>
>>
>>      
> This needs a changelog.
>
> This seems like a complicated solution to a simple problem.
>
> I don't understand why the ACPI IPMI opregion stuff can't be made an
> optional feature of the ACPI IPMI driver.  Trying to completely decouple
> things is just going to add corner cases and weird dependencies.
>    
Well, ipmi_si_intf is pretty darn big as it is.  I would like to break 
it up.

I spent a little time reading the ACPI spec to understand just what the 
heck this thing is.  So now my head hurts a little.  But from what I can 
tell, this provides a way for the ACPI system to specify operations that 
are done by sending IPMI messages.  For instance, if power control was 
done via IPMI, the various control methods for power control would work 
their way down the to access to opregion interfaces mapping to IPMI 
functions, and that's where this piece of code takes over.

If so, this code has nothing to do with the IPMI system interface.  It's 
really more ACPI than IPMI.  It's sending messages at the very top of 
the IPMI stack, where it should.  The only reason it cares about ipmi_si 
at all is the discovery of the IPMI interfaces.

I agree that a notifier framework seems like massive overkill for this 
interface.  I will note that there are already interfaces for 
registering to receive callbacks when an IPMI device is added or 
removed.  What's missing is a way to ask "Is this an ACPI PNP device?".

Since this same function will be needed for IPMI SMBus interfaces, if 
that ever becomes a reality in the kernel, it seems more reasonable to 
provide some type of addition to the IPMI interface to be able to store 
this in the low-level code and retrieve this from the IPMI user 
interface.  So you can use the standard mechanisms to watch for devices 
being added, and then query to see if they are PNP at that point.

Does that make sense?

-corey