On Tue, Jan 9, 2024 at 5:12 AM Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote: > > On Dell servers, many APCI methods of acpi_power_meter module evaluate > variables inside IPMI region, so the region handler needs to be > installed. In addition to that, the handler needs to be fully > functional, and that depends on SMI being selected. > > So add a helper to let acpi_power_meter know when the handler is > installed and ready to be used. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> and I'm expecting this to be routed along with patch [2/2] that has been posted elsewhere. > --- > v4: > - Wording. > - Define and comment on timeout value. > - Move the completion to driver_data. > - Remove the tenary operator. > > v3: > - New patch. > > drivers/acpi/acpi_ipmi.c | 23 ++++++++++++++++++++++- > include/acpi/acpi_bus.h | 5 +++++ > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c > index 0555f68c2dfd..5fba4dab5d08 100644 > --- a/drivers/acpi/acpi_ipmi.c > +++ b/drivers/acpi/acpi_ipmi.c > @@ -22,6 +22,8 @@ MODULE_LICENSE("GPL"); > /* the IPMI timeout is 5s */ > #define IPMI_TIMEOUT (5000) > #define ACPI_IPMI_MAX_MSG_LENGTH 64 > +/* 2s should be suffient for SMI being selected */ > +#define ACPI_IPMI_SMI_SELECTION_TIMEOUT (2 * HZ) > > struct acpi_ipmi_device { > /* the device list attached to driver_data.ipmi_devices */ > @@ -54,6 +56,7 @@ struct ipmi_driver_data { > * to this selected global IPMI system interface. > */ > struct acpi_ipmi_device *selected_smi; > + struct completion smi_selection_done; > }; > > struct acpi_ipmi_msg { > @@ -463,8 +466,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) > if (temp->handle == handle) > goto err_lock; > } > - if (!driver_data.selected_smi) > + if (!driver_data.selected_smi) { > driver_data.selected_smi = ipmi_device; > + complete(&driver_data.smi_selection_done); > + } > list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); > mutex_unlock(&driver_data.ipmi_lock); > > @@ -578,6 +583,20 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, > return status; > } > > +int acpi_wait_for_acpi_ipmi(void) > +{ > + long ret; > + > + ret = wait_for_completion_interruptible_timeout(&driver_data.smi_selection_done, > + ACPI_IPMI_SMI_SELECTION_TIMEOUT); > + > + if (ret <= 0) > + return -ETIMEDOUT; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(acpi_wait_for_acpi_ipmi); > + > static int __init acpi_ipmi_init(void) > { > int result; > @@ -586,6 +605,8 @@ static int __init acpi_ipmi_init(void) > if (acpi_disabled) > return 0; > > + init_completion(&driver_data.smi_selection_done); > + > status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, > ACPI_ADR_SPACE_IPMI, > &acpi_ipmi_space_handler, > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 1216d72c650f..afa6e4d4bf46 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -821,11 +821,16 @@ static inline void acpi_put_acpi_dev(struct acpi_device *adev) > { > acpi_dev_put(adev); > } > + > +int acpi_wait_for_acpi_ipmi(void); > + > #else /* CONFIG_ACPI */ > > static inline int register_acpi_bus_type(void *bus) { return 0; } > static inline int unregister_acpi_bus_type(void *bus) { return 0; } > > +static inline int acpi_wait_for_acpi_ipmi(void) { return 0; } > + > #endif /* CONFIG_ACPI */ > > #endif /*__ACPI_BUS_H__*/ > --