Re: [PATCH v1 3/5] ipmi: remove unused PCI probe coded

[Corey Minyard <minyard@acm.org>]

Bjorn Helgaas wrote:
> I guess you're referring to b0defcdbd2b7d?
>
> Prior to that commit, we did this:
>
> 	int              fe_rmc = 0;
> 	...
>               if (pci_dev && (pci_dev->subsystem_vendor == PCI_HP_VENDOR_ID))
>                       fe_rmc = 1;
> 	...
>         if (! fe_rmc)
>               /* Data register starts at base address + 1 in eRMC */
>               ++base_addr;
> 	...
> 	if (! is_new_interface(-1, IPMI_IO_ADDR_SPACE, base_addr)) {
>
> Your patch above reverses the sense of this adjustment -- the old code
> increments the base for everything *except* HP, while the new code
> increments the base for *only* HP.
>   
You are correct.

> The original 5-patch series leaves the PCI base address alone.  That's
> the same as the old behavior for HP devices, and we verified that it
> works on an HP DL380G6 by disabling SMBIOS/SMPI/PNP detection.  (We
> also verified that, as you would expect, it did NOT work if we increment
> the base address).
>   
Ok, then your patch is fine as stands.

> Using the address straight from the PCI BAR, we located the IPMI interface
> correctly, and we were able to exercise it with ipmitool:
>
>     ipmi message handler version 39.2
>     ipmi device interface
>     IPMI System Interface driver.
>     ACPI: PCI Interrupt Link [LNKF] enabled at IRQ 10
>     PCI: setting IRQ 10 as level-triggered
>     ipmi_si 0000:01:04.6: PCI INT A -> Link[LNKF] -> GSI 10 (level, low) -> IRQ 10
>     ipmi_si: Trying PCI-specified kcs state machine at mem address 0xf1ef0000, slave address 0x0, irq 10
>     IRQ 10/ipmi_si: IRQF_DISABLED is not guaranteed on shared IRQs
>       Using irq 10
>     ipmi: Found new BMC (man_id: 0x00000b,  prod_id: 0x0000, dev_id: 0x11)
>     IPMI kcs interface initialized
>
>     dl380g6a:~# ipmitool sensor
>     UID Light        | 0x0        | discrete   | 0x0080| na        | na        | na        | na        | na        | na
>     Sys. Health LED  | 0x0        | discrete   | 0x0080| na        | na        | na        | na        | na        | na
>     ...
>
> So the question is what to do about non-HP PCI IPMI interfaces.  The
> pre-b0defcdbd2b7d code increments the base address, but that's been
> gone for several years.  Since we've had no complaints, and we don't
> know about any non-HP PCI interfaces, I propose that we just remove
> that HP-specific adjustment completely, i.e., use this series as-is.
>   
To tell you the truth, I don't think there are any.  If there are, no 
one has complained.

So I'm happy with your original patch.

-corey