2023-11-02 04:31:39

by Armin Wolf

[permalink] [raw]
Subject: [PATCH 1/4] platform/x86: wmi: Add wmidev_block_set()

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 <[email protected]>
---
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


2023-11-02 04:32:37

by Armin Wolf

[permalink] [raw]
Subject: [PATCH 2/4] platform/x86: wmi: Add to_wmi_device() helper macro

Add a helper macro for WMI drivers to cast a device to
the corresponding WMI device. This should replace some
boilerplate code.

Signed-off-by: Armin Wolf <[email protected]>
---
include/linux/wmi.h | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index 207544968268..8a643c39fcce 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -27,6 +27,14 @@ struct wmi_device {
bool setable;
};

+/**
+ * to_wmi_device() - Helper macro to cast a device to a wmi_device
+ * @device: device struct
+ *
+ * Cast a struct device to a struct wmi_device.
+ */
+#define to_wmi_device(device) container_of(device, struct wmi_device, dev)
+
extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev,
u8 instance, u32 method_id,
const struct acpi_buffer *in,
--
2.39.2

2023-11-02 04:33:58

by Armin Wolf

[permalink] [raw]
Subject: [PATCH 4/4] platform/x86/intel/wmi: thunderbolt: Use bus-based WMI interface

Currently, the driver still uses the legacy GUID-based interface
to invoke WMI methods. Use the modern bus-based interface instead.

Tested on a Lenovo E51-80.

Signed-off-by: Armin Wolf <[email protected]>
---
drivers/platform/x86/intel/wmi/thunderbolt.c | 3 +--
drivers/platform/x86/wmi.c | 1 +
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel/wmi/thunderbolt.c b/drivers/platform/x86/intel/wmi/thunderbolt.c
index fc333ff82d1e..e2ad3f46f356 100644
--- a/drivers/platform/x86/intel/wmi/thunderbolt.c
+++ b/drivers/platform/x86/intel/wmi/thunderbolt.c
@@ -32,8 +32,7 @@ static ssize_t force_power_store(struct device *dev,
mode = hex_to_bin(buf[0]);
dev_dbg(dev, "force_power: storing %#x\n", mode);
if (mode == 0 || mode == 1) {
- status = wmi_evaluate_method(INTEL_WMI_THUNDERBOLT_GUID, 0, 1,
- &input, NULL);
+ status = wmidev_evaluate_method(to_wmi_device(dev), 0, 1, &input, NULL);
if (ACPI_FAILURE(status)) {
dev_dbg(dev, "force_power: failed to evaluate ACPI method\n");
return -ENODEV;
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 4c4effc883ae..58e7d5d535ba 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -107,6 +107,7 @@ static const char * const allow_duplicates[] = {
"05901221-D566-11D1-B2F0-00A0C9062910", /* wmi-bmof */
"8A42EA14-4F2A-FD45-6422-0087F7A7E608", /* dell-wmi-ddv */
"44FADEB1-B204-40F2-8581-394BBDC1B651", /* intel-wmi-sbl-fw-update */
+ "86CCFD48-205E-4A77-9C48-2021CBEDE341", /* intel-wmi-thunderbold */
NULL
};

--
2.39.2

2023-11-02 04:34:00

by Armin Wolf

[permalink] [raw]
Subject: [PATCH 3/4] platform/x86: intel-wmi-sbl-fw-update: Use bus-based WMI interface

Currently, the driver was still using the deprecated GUID-based
interface to query/set data blocks. Use the modern bus-based
interface for this.

Tested with a custom SSDT from the Intel Slim Bootloader project.

Signed-off-by: Armin Wolf <[email protected]>
---
drivers/platform/x86/intel/wmi/sbl-fw-update.c | 13 ++++---------
drivers/platform/x86/wmi.c | 1 +
2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/intel/wmi/sbl-fw-update.c b/drivers/platform/x86/intel/wmi/sbl-fw-update.c
index 3c86e0108a24..9cf5ed0f8dc2 100644
--- a/drivers/platform/x86/intel/wmi/sbl-fw-update.c
+++ b/drivers/platform/x86/intel/wmi/sbl-fw-update.c
@@ -25,18 +25,13 @@

static int get_fwu_request(struct device *dev, u32 *out)
{
- struct acpi_buffer result = {ACPI_ALLOCATE_BUFFER, NULL};
union acpi_object *obj;
- acpi_status status;

- status = wmi_query_block(INTEL_WMI_SBL_GUID, 0, &result);
- if (ACPI_FAILURE(status)) {
- dev_err(dev, "wmi_query_block failed\n");
+ obj = wmidev_block_query(to_wmi_device(dev), 0);
+ if (!obj)
return -ENODEV;
- }

- obj = (union acpi_object *)result.pointer;
- if (!obj || obj->type != ACPI_TYPE_INTEGER) {
+ if (obj->type != ACPI_TYPE_INTEGER) {
dev_warn(dev, "wmi_query_block returned invalid value\n");
kfree(obj);
return -EINVAL;
@@ -58,7 +53,7 @@ static int set_fwu_request(struct device *dev, u32 in)
input.length = sizeof(u32);
input.pointer = &value;

- status = wmi_set_block(INTEL_WMI_SBL_GUID, 0, &input);
+ status = wmidev_block_set(to_wmi_device(dev), 0, &input);
if (ACPI_FAILURE(status)) {
dev_err(dev, "wmi_set_block failed\n");
return -ENODEV;
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 9d9a050e7086..4c4effc883ae 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -106,6 +106,7 @@ MODULE_DEVICE_TABLE(acpi, wmi_device_ids);
static const char * const allow_duplicates[] = {
"05901221-D566-11D1-B2F0-00A0C9062910", /* wmi-bmof */
"8A42EA14-4F2A-FD45-6422-0087F7A7E608", /* dell-wmi-ddv */
+ "44FADEB1-B204-40F2-8581-394BBDC1B651", /* intel-wmi-sbl-fw-update */
NULL
};

--
2.39.2

2023-11-02 13:28:34

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 4/4] platform/x86/intel/wmi: thunderbolt: Use bus-based WMI interface

On 11/1/2023 23:29, Armin Wolf wrote:
> Currently, the driver still uses the legacy GUID-based interface
> to invoke WMI methods. Use the modern bus-based interface instead.
>
> Tested on a Lenovo E51-80.
>
> Signed-off-by: Armin Wolf <[email protected]>
> ---
> drivers/platform/x86/intel/wmi/thunderbolt.c | 3 +--
> drivers/platform/x86/wmi.c | 1 +
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/wmi/thunderbolt.c b/drivers/platform/x86/intel/wmi/thunderbolt.c
> index fc333ff82d1e..e2ad3f46f356 100644
> --- a/drivers/platform/x86/intel/wmi/thunderbolt.c
> +++ b/drivers/platform/x86/intel/wmi/thunderbolt.c
> @@ -32,8 +32,7 @@ static ssize_t force_power_store(struct device *dev,
> mode = hex_to_bin(buf[0]);
> dev_dbg(dev, "force_power: storing %#x\n", mode);
> if (mode == 0 || mode == 1) {
> - status = wmi_evaluate_method(INTEL_WMI_THUNDERBOLT_GUID, 0, 1,
> - &input, NULL);
> + status = wmidev_evaluate_method(to_wmi_device(dev), 0, 1, &input, NULL);
> if (ACPI_FAILURE(status)) {
> dev_dbg(dev, "force_power: failed to evaluate ACPI method\n");
> return -ENODEV;
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 4c4effc883ae..58e7d5d535ba 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -107,6 +107,7 @@ static const char * const allow_duplicates[] = {
> "05901221-D566-11D1-B2F0-00A0C9062910", /* wmi-bmof */
> "8A42EA14-4F2A-FD45-6422-0087F7A7E608", /* dell-wmi-ddv */
> "44FADEB1-B204-40F2-8581-394BBDC1B651", /* intel-wmi-sbl-fw-update */
> + "86CCFD48-205E-4A77-9C48-2021CBEDE341", /* intel-wmi-thunderbold */

s/thunderbold/thunderbolt/

> NULL
> };
>
> --
> 2.39.2
>

2023-11-02 18:50:14

by Joseph, Jithu

[permalink] [raw]
Subject: Re: [PATCH 3/4] platform/x86: intel-wmi-sbl-fw-update: Use bus-based WMI interface



On 11/1/2023 9:29 PM, Armin Wolf wrote:
> Currently, the driver was still using the deprecated GUID-based
> interface to query/set data blocks. Use the modern bus-based
> interface for this.
>
> Tested with a custom SSDT from the Intel Slim Bootloader project.
>
> Signed-off-by: Armin Wolf <[email protected]>
> ---
> drivers/platform/x86/intel/wmi/sbl-fw-update.c | 13 ++++---------
> drivers/platform/x86/wmi.c | 1 +
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/wmi/sbl-fw-update.c b/drivers/platform/x86/intel/wmi/sbl-fw-update.c
> index 3c86e0108a24..9cf5ed0f8dc2 100644
> --- a/drivers/platform/x86/intel/wmi/sbl-fw-update.c
> +++ b/drivers/platform/x86/intel/wmi/sbl-fw-update.c
> @@ -25,18 +25,13 @@
>
> static int get_fwu_request(struct device *dev, u32 *out)
> {
> - struct acpi_buffer result = {ACPI_ALLOCATE_BUFFER, NULL};
> union acpi_object *obj;
> - acpi_status status;
>
> - status = wmi_query_block(INTEL_WMI_SBL_GUID, 0, &result);
> - if (ACPI_FAILURE(status)) {
> - dev_err(dev, "wmi_query_block failed\n");
> + obj = wmidev_block_query(to_wmi_device(dev), 0);
> + if (!obj)
> return -ENODEV;
> - }
>
> - obj = (union acpi_object *)result.pointer;
> - if (!obj || obj->type != ACPI_TYPE_INTEGER) {
> + if (obj->type != ACPI_TYPE_INTEGER) {
> dev_warn(dev, "wmi_query_block returned invalid value\n");
> kfree(obj);
> return -EINVAL;
> @@ -58,7 +53,7 @@ static int set_fwu_request(struct device *dev, u32 in)
> input.length = sizeof(u32);
> input.pointer = &value;
>
> - status = wmi_set_block(INTEL_WMI_SBL_GUID, 0, &input);
> + status = wmidev_block_set(to_wmi_device(dev), 0, &input);
> if (ACPI_FAILURE(status)) {
> dev_err(dev, "wmi_set_block failed\n");
> return -ENODEV;
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 9d9a050e7086..4c4effc883ae 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -106,6 +106,7 @@ MODULE_DEVICE_TABLE(acpi, wmi_device_ids);
> static const char * const allow_duplicates[] = {
> "05901221-D566-11D1-B2F0-00A0C9062910", /* wmi-bmof */
> "8A42EA14-4F2A-FD45-6422-0087F7A7E608", /* dell-wmi-ddv */
> + "44FADEB1-B204-40F2-8581-394BBDC1B651", /* intel-wmi-sbl-fw-update */
> NULL
> };
>

sbl driver specific changes looks okay to me
Reviewed-by: Jithu Joseph <[email protected]>

Jithu

2023-11-02 19:08:42

by Joseph, Jithu

[permalink] [raw]
Subject: Re: [PATCH 1/4] platform/x86: wmi: Add wmidev_block_set()



On 11/1/2023 9:29 PM, 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 <[email protected]>
> ---
> drivers/platform/x86/wmi.c | 64 ++++++++++++++++++++------------------
> include/linux/wmi.h | 2 ++
> 2 files changed, 36 insertions(+), 30 deletions(-)
>

I noticed that this patch doesn't apply ontop of the latest released tag (v6.6) (the other 3 applies)
Just a minor suggestion :
Perhaps a note on which tree/base-commit the patch would apply would be helpful for reviewers

(After some digging around I found that the patch does indeed apply ontop of your patches submitted for the merge window)

Jithu