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

Corey Minyard
2009-12-02T21:34:55Z

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