Hi Yakui,

>    1. enumerate the ACPI device tree to find the IPMI system interface
>       The IPMI device type is IPI0001. When the device is found, it
> will continue to parse the corresponding resources.
>         For example:
>               interface type (KCS, BT, SMIC) (SSIF is not supported)

> +     /* Figure out the interface type. */
> +     switch (spmi->interfacetype) {
> +     case 1: /* KCS */
> +             info->si_type = SI_KCS;
> +             break;
> +     case 2: /* SMIC */
> +             info->si_type = SI_SMIC;
> +             break;
> +     case 3: /* BT */
> +             info->si_type = SI_BT;
> +             break;
> +     default:
> +             printk(KERN_INFO "ipmi_si: Unknown ACPI/SPMI SI type %d\n",
> +                     spmi->interfacetype);
> +             kfree(info);
> +             return -EIO;
> +     }

If SSIF is a possible value defined by the standard which specifies ACPI
identification of IPMI, please display it in the error message.  Then if
it starts appearing in the future, it will be obvious what support needs
to be added.  e.g.:

   default:
      printk(KERN_INFO "ipmi_si: ACPI/SPMI SI type %d (%s) is not supported",
             spmi->interfacetype,
             (spmi->interfacetype == ??4?? /* SSIF */) ? "SSIF" : "Unknown");

> +     if (spmi->interrupttype & 1) {
> +             /* We've got a GPE interrupt. */
> +             info->irq = spmi->gpe;
> +             info->irq_setup = acpi_gpe_irq_setup;
> +     } else if (spmi->interrupttype & 2) {
> +             /* We've got an APIC/SAPIC interrupt. */
> +             info->irq = spmi->global_interrupt;
> +             info->irq_setup = std_irq_setup;
> +     } else {
> +             /* Use the default interrupt setting. */

if any other bits are set, should this warn about them?

> +             info->irq = 0;
> +             info->irq_setup = NULL;
> +     }
>
> +     if (spmi->addr.bit_width) {
> +             /* A (hopefully) properly formed register bit width. */

should this be checked (e.g. warn if bit_width % 8 != 0)?

> +             info->io.regspacing = spmi->addr.bit_width / 8;
> +     } else {
> +             info->io.regspacing = DEFAULT_REGSPACING;
> +     }
> +     info->io.regsize = info->io.regspacing;
> +     info->io.regshift = spmi->addr.bit_offset;
> +
> +     if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +             info->io_setup = mem_setup;
> +             info->io.addr_type = IPMI_MEM_ADDR_SPACE;
> +     } else if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> +             info->io_setup = port_setup;
> +             info->io.addr_type = IPMI_IO_ADDR_SPACE;
> +     } else {
> +             kfree(info);
> +             printk(KERN_WARNING
> +                    "ipmi_si: Unknown ACPI I/O Address type\n");

"%d", io.addr_type
(or maybe %x? -- whatever's right in context)

I'm not going to go line-by-line over the rest, but in general,
if the code thinks something is wrong, have it show what it
objects to:

> +                     if (irq->interrupt_count != 1) {
> +                             printk(KERN_WARNING "incorrect IRQ is "
> +                                     "defined in _CRS\n");

"interrupt_count is %d, must be 1", irq->interrupt_count

etc.

>Bela<
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

Reply via email to