Re: [PATCH] ipmi: Remove ACPI SPMI probing from the SSIF (I2C) driver

From: Corey Minyard
Date: Fri Mar 09 2018 - 21:43:07 EST


On 03/09/2018 04:28 PM, Jiandi An wrote:


On 03/08/2018 03:08 PM, minyard@xxxxxxx wrote:
From: Corey Minyard <cminyard@xxxxxxxxxx>

The IPMI spec states:

ÂÂ The purpose of the SPMI Table is to provide a mechanism that can
ÂÂ be used by the OSPM (an ACPI term for âOS Operating System-directed
ÂÂ configuration and Power Managementâ essentially meaning an ACPI-aware
ÂÂ OS or OS loader) very early in the boot process, e.g., before the
ÂÂ ability to execute ACPI control methods in the OS is available.

When we are probing IPMI in Linux, ACPI control methods are available,
so we shouldn't be probing using SPMI. It could cause some confusion
during the probing process.

Signed-off-by: Corey Minyard <cminyard@xxxxxxxxxx>
Cc: Jiandi An <anjiandi@xxxxxxxxxxxxxx>
---

Jiandi, this just yanks out the SPMI code. Your platform
should have an ACPI control method in it's namespace specifying the
IPMI interface. If it doesn't, that's a bug in your platform and
really needs to be fixed. Finding IPMI devices with SMBIOS tables
is kind of risky since there is no real way to know which I2C bus
has the IPMI device if your system has more than one I2C bus. With
an ACPI control method, the IPMI control method will be inside the
I2C bus control method, so it will be unambiguous.

Thanks Corey. I tested this patch. It works for us through the ACPI
control method.

Thanks. May I add a "Tested-by" for you?

By the way, FYI the ipmi_si driver is also having the
SPMI probe code.

[ÂÂ 17.370835] ipmi_si: probing via SPMI


Yes, I've removed that in a separate patch.

-corey

Thanks
- Jiandi


 drivers/char/ipmi/ipmi_ssif.c | 105 ------------------------------------------
 1 file changed, 105 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 4cff4cd..206257b 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1994,108 +1994,6 @@ static const struct acpi_device_id ssif_acpi_match[] = {
ÂÂÂÂÂ { },
 };
 MODULE_DEVICE_TABLE(acpi, ssif_acpi_match);
-
-/*
- * Once we get an ACPI failure, we don't try any more, because we go
- * through the tables sequentially. Once we don't find a table, there
- * are no more.
- */
-static int acpi_failure;
-
-/*
- * Defined in the IPMI 2.0 spec.
- */
-struct SPMITable {
-ÂÂÂ s8ÂÂÂ Signature[4];
-ÂÂÂ u32ÂÂÂ Length;
-ÂÂÂ u8ÂÂÂ Revision;
-ÂÂÂ u8ÂÂÂ Checksum;
-ÂÂÂ s8ÂÂÂ OEMID[6];
-ÂÂÂ s8ÂÂÂ OEMTableID[8];
-ÂÂÂ s8ÂÂÂ OEMRevision[4];
-ÂÂÂ s8ÂÂÂ CreatorID[4];
-ÂÂÂ s8ÂÂÂ CreatorRevision[4];
-ÂÂÂ u8ÂÂÂ InterfaceType;
-ÂÂÂ u8ÂÂÂ IPMIlegacy;
-ÂÂÂ s16ÂÂÂ SpecificationRevision;
-
-ÂÂÂ /*
-ÂÂÂÂ * Bit 0 - SCI interrupt supported
-ÂÂÂÂ * Bit 1 - I/O APIC/SAPIC
-ÂÂÂÂ */
-ÂÂÂ u8ÂÂÂ InterruptType;
-
-ÂÂÂ /*
-ÂÂÂÂ * If bit 0 of InterruptType is set, then this is the SCI
-ÂÂÂÂ * interrupt in the GPEx_STS register.
-ÂÂÂÂ */
-ÂÂÂ u8ÂÂÂ GPE;
-
-ÂÂÂ s16ÂÂÂ Reserved;
-
-ÂÂÂ /*
-ÂÂÂÂ * If bit 1 of InterruptType is set, then this is the I/O
-ÂÂÂÂ * APIC/SAPIC interrupt.
-ÂÂÂÂ */
-ÂÂÂ u32ÂÂÂ GlobalSystemInterrupt;
-
-ÂÂÂ /* The actual register address. */
-ÂÂÂ struct acpi_generic_address addr;
-
-ÂÂÂ u8ÂÂÂ UID[4];
-
-ÂÂÂ s8ÂÂÂÂÂ spmi_id[1]; /* A '\0' terminated array starts here. */
-};
-
-static int try_init_spmi(struct SPMITable *spmi)
-{
-ÂÂÂ unsigned short myaddr;
-
-ÂÂÂ if (num_addrs >= MAX_SSIF_BMCS)
-ÂÂÂÂÂÂÂ return -1;
-
-ÂÂÂ if (spmi->IPMIlegacy != 1) {
-ÂÂÂÂÂÂÂ pr_warn("IPMI: Bad SPMI legacy: %d\n", spmi->IPMIlegacy);
-ÂÂÂÂÂÂÂ return -ENODEV;
-ÂÂÂ }
-
-ÂÂÂ if (spmi->InterfaceType != 4)
-ÂÂÂÂÂÂÂ return -ENODEV;
-
-ÂÂÂ if (spmi->addr.space_id != ACPI_ADR_SPACE_SMBUS) {
-ÂÂÂÂÂÂÂ pr_warn(PFX "Invalid ACPI SSIF I/O Address type: %d\n",
-ÂÂÂÂÂÂÂÂÂÂÂ spmi->addr.space_id);
-ÂÂÂÂÂÂÂ return -EIO;
-ÂÂÂ }
-
-ÂÂÂ myaddr = spmi->addr.address & 0x7f;
-
-ÂÂÂ return new_ssif_client(myaddr, NULL, 0, 0, SI_SPMI, NULL);
-}
-
-static void spmi_find_bmc(void)
-{
-ÂÂÂ acpi_statusÂÂÂÂÂ status;
-ÂÂÂ struct SPMITable *spmi;
-ÂÂÂ intÂÂÂÂÂÂÂÂÂÂÂÂÂ i;
-
-ÂÂÂ if (acpi_disabled)
-ÂÂÂÂÂÂÂ return;
-
-ÂÂÂ if (acpi_failure)
-ÂÂÂÂÂÂÂ return;
-
-ÂÂÂ for (i = 0; ; i++) {
-ÂÂÂÂÂÂÂ status = acpi_get_table(ACPI_SIG_SPMI, i+1,
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ (struct acpi_table_header **)&spmi);
-ÂÂÂÂÂÂÂ if (status != AE_OK)
-ÂÂÂÂÂÂÂÂÂÂÂ return;
-
-ÂÂÂÂÂÂÂ try_init_spmi(spmi);
-ÂÂÂ }
-}
-#else
-static void spmi_find_bmc(void) { }
 #endif
  #ifdef CONFIG_DMI
@@ -2200,9 +2098,6 @@ static int init_ipmi_ssif(void)
ÂÂÂÂÂÂÂÂÂ ssif_i2c_driver.driver.acpi_match_tableÂÂÂ =
ÂÂÂÂÂÂÂÂÂÂÂÂÂ ACPI_PTR(ssif_acpi_match);
 - if (ssif_tryacpi)
-ÂÂÂÂÂÂÂ spmi_find_bmc();
-
ÂÂÂÂÂ if (ssif_trydmi) {
ÂÂÂÂÂÂÂÂÂ rv = platform_driver_register(&ipmi_driver);
ÂÂÂÂÂÂÂÂÂ if (rv)