Hi Armin, On 11/3/23 19:25, Armin Wolf wrote: > Currently, WMI drivers have to use the deprecated GUID-based > interface when setting data blocks. This prevents those > drivers from fully moving away from this interface. > > Provide wmidev_block_set() so drivers using wmi_set_block() can > fully migrate to the modern bus-based interface. > > Tested with a custom SSDT from the Intel Slim Bootloader project. > > Signed-off-by: Armin Wolf <W_Armin@xxxxxx> Thank you for your patch-series, I've applied the series to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans One thing which I noticed during review of patch 3/4 is that some WMI drivers might benefit from having a wmidev_block_query_typed() similar to how we have a acpi_evaluate_dsm_typed() which takes an ACPI type and returns a NULL pointer instead of a wrongly typed ACPI object when the type does not match. Specifically this would allow dropping the return obj type checking from sbl-fw-update.c : get_fwu_request() . Now adding a wmidev_block_query_typed() wrapper around wmidev_block_query () just for this is not really a win, but it might be useful in the future ? Anyways just an idea. Regards, Hans > --- > Changes in v2: > - applies on pdx86/for-next > --- > drivers/platform/x86/wmi.c | 64 ++++++++++++++++++++------------------ > include/linux/wmi.h | 2 ++ > 2 files changed, 36 insertions(+), 30 deletions(-) > > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index 5c27b4aa9690..9d9a050e7086 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -536,41 +536,50 @@ EXPORT_SYMBOL_GPL(wmidev_block_query); > * > * Return: acpi_status signaling success or error. > */ > -acpi_status wmi_set_block(const char *guid_string, u8 instance, > - const struct acpi_buffer *in) > +acpi_status wmi_set_block(const char *guid_string, u8 instance, const struct acpi_buffer *in) > { > - struct wmi_block *wblock; > - struct guid_block *block; > struct wmi_device *wdev; > - acpi_handle handle; > - struct acpi_object_list input; > - union acpi_object params[2]; > - char method[WMI_ACPI_METHOD_NAME_SIZE]; > acpi_status status; > > - if (!in) > - return AE_BAD_DATA; > - > wdev = wmi_find_device_by_guid(guid_string); > if (IS_ERR(wdev)) > return AE_ERROR; > > - wblock = container_of(wdev, struct wmi_block, dev); > - block = &wblock->gblock; > - handle = wblock->acpi_device->handle; > + status = wmidev_block_set(wdev, instance, in); > + wmi_device_put(wdev); > > - if (block->instance_count <= instance) { > - status = AE_BAD_PARAMETER; > + return status; > +} > +EXPORT_SYMBOL_GPL(wmi_set_block); > > - goto err_wdev_put; > - } > +/** > + * wmidev_block_set - Write to a WMI block > + * @wdev: A wmi bus device from a driver > + * @instance: Instance index > + * @in: Buffer containing new values for the data block > + * > + * Write contents of the input buffer to an ACPI-WMI data block. > + * > + * Return: acpi_status signaling success or error. > + */ > +acpi_status wmidev_block_set(struct wmi_device *wdev, u8 instance, const struct acpi_buffer *in) > +{ > + struct wmi_block *wblock = container_of(wdev, struct wmi_block, dev); > + acpi_handle handle = wblock->acpi_device->handle; > + struct guid_block *block = &wblock->gblock; > + char method[WMI_ACPI_METHOD_NAME_SIZE]; > + struct acpi_object_list input; > + union acpi_object params[2]; > > - /* Check GUID is a data block */ > - if (block->flags & (ACPI_WMI_EVENT | ACPI_WMI_METHOD)) { > - status = AE_ERROR; > + if (!in) > + return AE_BAD_DATA; > > - goto err_wdev_put; > - } > + if (block->instance_count <= instance) > + return AE_BAD_PARAMETER; > + > + /* Check GUID is a data block */ > + if (block->flags & (ACPI_WMI_EVENT | ACPI_WMI_METHOD)) > + return AE_ERROR; > > input.count = 2; > input.pointer = params; > @@ -582,14 +591,9 @@ acpi_status wmi_set_block(const char *guid_string, u8 instance, > > get_acpi_method_name(wblock, 'S', method); > > - status = acpi_evaluate_object(handle, method, &input, NULL); > - > -err_wdev_put: > - wmi_device_put(wdev); > - > - return status; > + return acpi_evaluate_object(handle, method, &input, NULL); > } > -EXPORT_SYMBOL_GPL(wmi_set_block); > +EXPORT_SYMBOL_GPL(wmidev_block_set); > > static void wmi_dump_wdg(const struct guid_block *g) > { > diff --git a/include/linux/wmi.h b/include/linux/wmi.h > index 763bd382cf2d..207544968268 100644 > --- a/include/linux/wmi.h > +++ b/include/linux/wmi.h > @@ -35,6 +35,8 @@ extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev, > extern union acpi_object *wmidev_block_query(struct wmi_device *wdev, > u8 instance); > > +acpi_status wmidev_block_set(struct wmi_device *wdev, u8 instance, const struct acpi_buffer *in); > + > u8 wmidev_instance_count(struct wmi_device *wdev); > > extern int set_required_buffer_size(struct wmi_device *wdev, u64 length); > -- > 2.39.2 >
