The reason for the acpi_bus_register/unregister is to get the handle of the 
IPMI ACPI object.  There isn't a way to get the device handle otherwise.  Once 
it gets the handle it doesn't need the ACPI device anymore.  It's not the best 
solution, but the current way that the SI driver initializes (at least with 
SLES and older RHEL3/4) doesn't match up well with the way that the ACPI 
drivers initialize.  Actually for RHEL5 I could make this cleaner, moving the 
try_init_acpidev code into the acpi_ipmi_add; then set a global variable to 
show that it initialized properly to pass back to try_init_xxxx.  For older 
OpenIPMI driver code this doesn't mesh so well as the try_init_xxx routines 
take a smi_info ** argument and we can't pass this to the ACPI add routine.

--jordan hargrave
Dell Enterprise Linux Engineering



-----Original Message-----
From: Corey Minyard [mailto:[EMAIL PROTECTED]
Sent: Wed 2/28/2007 3:42 PM
To: Hargrave, Jordan
Cc: Domsch, Matt; [EMAIL PROTECTED]; linux-acpi@vger.kernel.org; 
openipmi-developer@lists.sourceforge.net; [EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: Re: [Openipmi-developer] acpi_find_bmc() and acpi_get_table()
 
[EMAIL PROTECTED] wrote:
> Corey,
>
>   Here is the patch (RHEL5 base code) I've been testing that detects the ACPI 
> namespace object.
> The IPI0001 device doesn't contain the register spacing directly; it has a 
> _CRS resource object that
> (for KCS) has two I/O port entries.  I save the first port in io.addr_data, 
> then calculate the register spacing based on subtracting the 2nd port address 
> and the 1st.
>   
This patch looks pretty good, comments inline below.
> I'm thinking of changing the register spacing to using an array of 
> port/memory addresses to make calculating interface addresses more generic.
>   
Ok, I'll wait until you are sure here.  The changes below are fine, they
neaten things up a little.  It would probably need to be a separate patch.
> eg.
> static unsigned char port_inb(struct si_sm_io *io, unsigned int offset)
> {
>         unsigned int addr = io->addr_data;
>
>         return inb(addr + (offset * io->regspacing));
> }
>
> would become something like:
> static unsigned char port_inb(struct si_sm_io *io, unsigned int offset)
> {
>         unsigned int addr = io->addr_data[offset];
>
>         return inb(addr);
> }
>
> --jordan hargrave
> Dell Enterprise Linux Engineering
>
>
>   
> +
> +static struct acpi_driver acpi_ipmi_driver = {
> +  .name = ACPI_IPMI_DRIVER_NAME,
> +  .class = ACPI_IPMI_CLASS,
> +  .ids = ACPI_IPMI_HID,
> +  .ops = {
> +    .add = acpi_ipmi_add,
> +    .remove = acpi_ipmi_remove,
> +  },
> +};
>   
Need to fix the indention above.

Also, we need to delete the old code, I think.
> @@ -1576,6 +1694,12 @@ static __devinit void acpi_find_bmc(void
>       if (acpi_failure)
>               return;
>  
> +     /* Really we only need to get the IPMI device handle here */
> +     acpi_bus_register_driver(&acpi_ipmi_driver);
> +     acpi_bus_unregister_driver(&acpi_ipmi_driver);
>   
Again, I don't know ACPI, but the above code looks rather strange.

Thanks,

-Corey
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to