2021-10-12 17:54:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 4/7] surface: surface3-wmi: Use ACPI_COMPANION() directly

From: Rafael J. Wysocki <[email protected]>

The ACPI_HANDLE() macro is a wrapper arond the ACPI_COMPANION()
macro and the ACPI handle produced by the former comes from the
ACPI device object produced by the latter, so it is way more
straightforward to evaluate the latter directly instead of passing
the handle produced by the former to acpi_bus_get_device().

Modify s3_wmi_check_platform_device() accordingly (no intentional
functional impact).

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/platform/surface/surface3-wmi.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/platform/surface/surface3-wmi.c
===================================================================
--- linux-pm.orig/drivers/platform/surface/surface3-wmi.c
+++ linux-pm/drivers/platform/surface/surface3-wmi.c
@@ -139,13 +139,12 @@ static acpi_status s3_wmi_attach_spi_dev

static int s3_wmi_check_platform_device(struct device *dev, void *data)
{
- struct acpi_device *adev, *ts_adev = NULL;
- acpi_handle handle;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ struct acpi_device *ts_adev = NULL;
acpi_status status;

/* ignore non ACPI devices */
- handle = ACPI_HANDLE(dev);
- if (!handle || acpi_bus_get_device(handle, &adev))
+ if (!adev)
return 0;

/* check for LID ACPI switch */
@@ -159,7 +158,7 @@ static int s3_wmi_check_platform_device(
strlen(SPI_CTL_OBJ_NAME)))
return 0;

- status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
+ status = acpi_walk_namespace(ACPI_TYPE_DEVICE, adev->handle, 1,
s3_wmi_attach_spi_device, NULL,
&ts_adev, NULL);
if (ACPI_FAILURE(status))




2021-10-12 18:17:31

by Maximilian Luz

[permalink] [raw]
Subject: Re: [PATCH v1 4/7] surface: surface3-wmi: Use ACPI_COMPANION() directly

On 10/12/21 19:44, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The ACPI_HANDLE() macro is a wrapper arond the ACPI_COMPANION()
> macro and the ACPI handle produced by the former comes from the
> ACPI device object produced by the latter, so it is way more
> straightforward to evaluate the latter directly instead of passing
> the handle produced by the former to acpi_bus_get_device().
>
> Modify s3_wmi_check_platform_device() accordingly (no intentional
> functional impact).
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

Looks good to me.

Reviewed-by: Maximilian Luz <[email protected]>

> ---
> drivers/platform/surface/surface3-wmi.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> Index: linux-pm/drivers/platform/surface/surface3-wmi.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/surface/surface3-wmi.c
> +++ linux-pm/drivers/platform/surface/surface3-wmi.c
> @@ -139,13 +139,12 @@ static acpi_status s3_wmi_attach_spi_dev
>
> static int s3_wmi_check_platform_device(struct device *dev, void *data)
> {
> - struct acpi_device *adev, *ts_adev = NULL;
> - acpi_handle handle;
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> + struct acpi_device *ts_adev = NULL;
> acpi_status status;
>
> /* ignore non ACPI devices */
> - handle = ACPI_HANDLE(dev);
> - if (!handle || acpi_bus_get_device(handle, &adev))
> + if (!adev)
> return 0;
>
> /* check for LID ACPI switch */
> @@ -159,7 +158,7 @@ static int s3_wmi_check_platform_device(
> strlen(SPI_CTL_OBJ_NAME)))
> return 0;
>
> - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
> + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, adev->handle, 1,
> s3_wmi_attach_spi_device, NULL,
> &ts_adev, NULL);
> if (ACPI_FAILURE(status))
>
>
>

2021-10-13 16:21:05

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v2 4/7] surface: surface3-wmi: Use ACPI_COMPANION() directly

From: Rafael J. Wysocki <[email protected]>

The ACPI_HANDLE() macro is a wrapper arond the ACPI_COMPANION()
macro and the ACPI handle produced by the former comes from the
ACPI device object produced by the latter, so it is way more
straightforward to evaluate the latter directly instead of passing
the handle produced by the former to acpi_bus_get_device().

Modify s3_wmi_check_platform_device() accordingly (no intentional
functional impact).

Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Maximilian Luz <[email protected]>
---

v1 -> v2:
* Resend with a different From and S-o-b address and with R-by from
Maximilian. No other changes.

---
drivers/platform/surface/surface3-wmi.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/platform/surface/surface3-wmi.c
===================================================================
--- linux-pm.orig/drivers/platform/surface/surface3-wmi.c
+++ linux-pm/drivers/platform/surface/surface3-wmi.c
@@ -139,13 +139,12 @@ static acpi_status s3_wmi_attach_spi_dev

static int s3_wmi_check_platform_device(struct device *dev, void *data)
{
- struct acpi_device *adev, *ts_adev = NULL;
- acpi_handle handle;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ struct acpi_device *ts_adev = NULL;
acpi_status status;

/* ignore non ACPI devices */
- handle = ACPI_HANDLE(dev);
- if (!handle || acpi_bus_get_device(handle, &adev))
+ if (!adev)
return 0;

/* check for LID ACPI switch */
@@ -159,7 +158,7 @@ static int s3_wmi_check_platform_device(
strlen(SPI_CTL_OBJ_NAME)))
return 0;

- status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
+ status = acpi_walk_namespace(ACPI_TYPE_DEVICE, adev->handle, 1,
s3_wmi_attach_spi_device, NULL,
&ts_adev, NULL);
if (ACPI_FAILURE(status))



2021-10-19 14:59:10

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 4/7] surface: surface3-wmi: Use ACPI_COMPANION() directly

Hi,

On 10/13/21 18:10, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The ACPI_HANDLE() macro is a wrapper arond the ACPI_COMPANION()
> macro and the ACPI handle produced by the former comes from the
> ACPI device object produced by the latter, so it is way more
> straightforward to evaluate the latter directly instead of passing
> the handle produced by the former to acpi_bus_get_device().
>
> Modify s3_wmi_check_platform_device() accordingly (no intentional
> functional impact).
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> Reviewed-by: Maximilian Luz <[email protected]>

Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans


> ---
>
> v1 -> v2:
> * Resend with a different From and S-o-b address and with R-by from
> Maximilian. No other changes.
>
> ---
> drivers/platform/surface/surface3-wmi.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> Index: linux-pm/drivers/platform/surface/surface3-wmi.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/surface/surface3-wmi.c
> +++ linux-pm/drivers/platform/surface/surface3-wmi.c
> @@ -139,13 +139,12 @@ static acpi_status s3_wmi_attach_spi_dev
>
> static int s3_wmi_check_platform_device(struct device *dev, void *data)
> {
> - struct acpi_device *adev, *ts_adev = NULL;
> - acpi_handle handle;
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> + struct acpi_device *ts_adev = NULL;
> acpi_status status;
>
> /* ignore non ACPI devices */
> - handle = ACPI_HANDLE(dev);
> - if (!handle || acpi_bus_get_device(handle, &adev))
> + if (!adev)
> return 0;
>
> /* check for LID ACPI switch */
> @@ -159,7 +158,7 @@ static int s3_wmi_check_platform_device(
> strlen(SPI_CTL_OBJ_NAME)))
> return 0;
>
> - status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
> + status = acpi_walk_namespace(ACPI_TYPE_DEVICE, adev->handle, 1,
> s3_wmi_attach_spi_device, NULL,
> &ts_adev, NULL);
> if (ACPI_FAILURE(status))
>
>
>