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

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

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael@kernel.org>

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 <rafael@kernel.org>
---
 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))

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

[Maximilian Luz]

On 10/12/21 19:44, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael@kernel.org>
> 
> 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 <rafael@kernel.org>

Looks good to me.

Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>

> ---
>   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))
> 
> 
> 

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

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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 <rafael.j.wysocki@intel.com>
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
---

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))

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

[Hans de Goede]

Hi,

On 10/13/21 18:10, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <rafael.j.wysocki@intel.com>
> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>

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))
> 
> 
> 

[PATCH v1 5/7] surface: surface3_power: Use ACPI_COMPANION() directly

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael@kernel.org>

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 mshw0011_notify() accordingly (no intentional functional
impact).

Signed-off-by: Rafael J. Wysocki <rafael@kernel.org>
---
 drivers/platform/surface/surface3_power.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/platform/surface/surface3_power.c
===================================================================
--- linux-pm.orig/drivers/platform/surface/surface3_power.c
+++ linux-pm/drivers/platform/surface/surface3_power.c
@@ -160,15 +160,14 @@ mshw0011_notify(struct mshw0011_data *cd
 {
 	union acpi_object *obj;
 	struct acpi_device *adev;
-	acpi_handle handle;
 	unsigned int i;
 
-	handle = ACPI_HANDLE(&cdata->adp1->dev);
-	if (!handle || acpi_bus_get_device(handle, &adev))
+	adev = ACPI_COMPANION(&cdata->adp1->dev);
+	if (!adev)
 		return -ENODEV;
 
-	obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL,
-				      ACPI_TYPE_BUFFER);
+	obj = acpi_evaluate_dsm_typed(adev->handle, &mshw0011_guid, arg1, arg2,
+				      NULL, ACPI_TYPE_BUFFER);
 	if (!obj) {
 		dev_err(&cdata->adp1->dev, "device _DSM execution failed\n");
 		return -ENODEV;

Re: [PATCH v1 5/7] surface: surface3_power: Use ACPI_COMPANION() directly

[Maximilian Luz]

On 10/12/21 19:46, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael@kernel.org>
> 
> 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 mshw0011_notify() accordingly (no intentional functional
> impact).
> 
> Signed-off-by: Rafael J. Wysocki <rafael@kernel.org>

Looks mostly good to me, small comment/question inline.

> ---
>   drivers/platform/surface/surface3_power.c |    9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> Index: linux-pm/drivers/platform/surface/surface3_power.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/surface/surface3_power.c
> +++ linux-pm/drivers/platform/surface/surface3_power.c
> @@ -160,15 +160,14 @@ mshw0011_notify(struct mshw0011_data *cd
>   {
>   	union acpi_object *obj;
>   	struct acpi_device *adev;
> -	acpi_handle handle;
>   	unsigned int i;
>   
> -	handle = ACPI_HANDLE(&cdata->adp1->dev);
> -	if (!handle || acpi_bus_get_device(handle, &adev))
> +	adev = ACPI_COMPANION(&cdata->adp1->dev);
> +	if (!adev)
>   		return -ENODEV;

Do we need to get the ACPI device (adev) here? To me it looks like only
its handle is actually used so why not keep ACPI_HANDLE() and remove the
acpi_bus_get_device() call instead?

>   
> -	obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL,
> -				      ACPI_TYPE_BUFFER);
> +	obj = acpi_evaluate_dsm_typed(adev->handle, &mshw0011_guid, arg1, arg2,
> +				      NULL, ACPI_TYPE_BUFFER);
>   	if (!obj) {
>   		dev_err(&cdata->adp1->dev, "device _DSM execution failed\n");
>   		return -ENODEV;
> 
> 
> 

Regards,
Max

Re: [PATCH v1 5/7] surface: surface3_power: Use ACPI_COMPANION() directly

[Rafael J. Wysocki]

On Tue, Oct 12, 2021 at 8:21 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>
> On 10/12/21 19:46, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael@kernel.org>
> >
> > 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 mshw0011_notify() accordingly (no intentional functional
> > impact).
> >
> > Signed-off-by: Rafael J. Wysocki <rafael@kernel.org>
>
> Looks mostly good to me, small comment/question inline.
>
> > ---
> >   drivers/platform/surface/surface3_power.c |    9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > Index: linux-pm/drivers/platform/surface/surface3_power.c
> > ===================================================================
> > --- linux-pm.orig/drivers/platform/surface/surface3_power.c
> > +++ linux-pm/drivers/platform/surface/surface3_power.c
> > @@ -160,15 +160,14 @@ mshw0011_notify(struct mshw0011_data *cd
> >   {
> >       union acpi_object *obj;
> >       struct acpi_device *adev;
> > -     acpi_handle handle;
> >       unsigned int i;
> >
> > -     handle = ACPI_HANDLE(&cdata->adp1->dev);
> > -     if (!handle || acpi_bus_get_device(handle, &adev))
> > +     adev = ACPI_COMPANION(&cdata->adp1->dev);
> > +     if (!adev)
> >               return -ENODEV;
>
> Do we need to get the ACPI device (adev) here? To me it looks like only
> its handle is actually used so why not keep ACPI_HANDLE() and remove the
> acpi_bus_get_device() call instead?

It actually doesn't really matter, but you're right,
acpi_bus_get_device() is simply redundant here, so ACPI_HANDLE() is
sufficient.

I'll send a v2 of this one.

> >
> > -     obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL,
> > -                                   ACPI_TYPE_BUFFER);
> > +     obj = acpi_evaluate_dsm_typed(adev->handle, &mshw0011_guid, arg1, arg2,
> > +                                   NULL, ACPI_TYPE_BUFFER);
> >       if (!obj) {
> >               dev_err(&cdata->adp1->dev, "device _DSM execution failed\n");
> >               return -ENODEV;
> >
> >
> >
>
> Regards,
> Max

[PATCH v2 5/7] surface: surface3_power: Drop redundant acpi_bus_get_device() call

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael@kernel.org>

If the ACPI companion of a given device is not present, the result
of the ACPI_HANDLE() evaluation for it will be NULL, so calling
acpi_bus_get_device() on ACPI_HANDLE() result in order to validate
it is redundant.

Drop the redundant acpi_bus_get_device() call from mshw0011_notify()
along with a local variable related to it.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael@kernel.org>
---

v1 -> v2:
   * Instead of switching over to using ACPI_COMPANION(), just drop the
     redundant acpi_bus_get_device() call from mshw0011_notify() and
     update the subject and changelog accordingly.

---
 drivers/platform/surface/surface3_power.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-pm/drivers/platform/surface/surface3_power.c
===================================================================
--- linux-pm.orig/drivers/platform/surface/surface3_power.c
+++ linux-pm/drivers/platform/surface/surface3_power.c
@@ -159,12 +159,11 @@ mshw0011_notify(struct mshw0011_data *cd
 		unsigned int *ret_value)
 {
 	union acpi_object *obj;
-	struct acpi_device *adev;
 	acpi_handle handle;
 	unsigned int i;
 
 	handle = ACPI_HANDLE(&cdata->adp1->dev);
-	if (!handle || acpi_bus_get_device(handle, &adev))
+	if (!handle)
 		return -ENODEV;
 
 	obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL,

Re: [PATCH v2 5/7] surface: surface3_power: Drop redundant acpi_bus_get_device() call

[Maximilian Luz]

On 10/12/21 21:32, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael@kernel.org>
> 
> If the ACPI companion of a given device is not present, the result
> of the ACPI_HANDLE() evaluation for it will be NULL, so calling
> acpi_bus_get_device() on ACPI_HANDLE() result in order to validate
> it is redundant.
> 
> Drop the redundant acpi_bus_get_device() call from mshw0011_notify()
> along with a local variable related to it.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael@kernel.org>
> ---
> 
> v1 -> v2:
>     * Instead of switching over to using ACPI_COMPANION(), just drop the
>       redundant acpi_bus_get_device() call from mshw0011_notify() and
>       update the subject and changelog accordingly.
> 

Thanks, looks good to me!

Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>

> ---
>   drivers/platform/surface/surface3_power.c |    3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: linux-pm/drivers/platform/surface/surface3_power.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/surface/surface3_power.c
> +++ linux-pm/drivers/platform/surface/surface3_power.c
> @@ -159,12 +159,11 @@ mshw0011_notify(struct mshw0011_data *cd
>   		unsigned int *ret_value)
>   {
>   	union acpi_object *obj;
> -	struct acpi_device *adev;
>   	acpi_handle handle;
>   	unsigned int i;
>   
>   	handle = ACPI_HANDLE(&cdata->adp1->dev);
> -	if (!handle || acpi_bus_get_device(handle, &adev))
> +	if (!handle)
>   		return -ENODEV;
>   
>   	obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL,
> 
> 
> 

[PATCH v3 5/7] surface: surface3_power: Drop redundant acpi_bus_get_device() call

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

If the ACPI companion of a given device is not present, the result
of the ACPI_HANDLE() evaluation for it will be NULL, so calling
acpi_bus_get_device() on ACPI_HANDLE() result in order to validate
it is redundant.

Drop the redundant acpi_bus_get_device() call from mshw0011_notify()
along with a local variable related to it.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
---

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

---
 drivers/platform/surface/surface3_power.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-pm/drivers/platform/surface/surface3_power.c
===================================================================
--- linux-pm.orig/drivers/platform/surface/surface3_power.c
+++ linux-pm/drivers/platform/surface/surface3_power.c
@@ -159,12 +159,11 @@ mshw0011_notify(struct mshw0011_data *cd
 		unsigned int *ret_value)
 {
 	union acpi_object *obj;
-	struct acpi_device *adev;
 	acpi_handle handle;
 	unsigned int i;
 
 	handle = ACPI_HANDLE(&cdata->adp1->dev);
-	if (!handle || acpi_bus_get_device(handle, &adev))
+	if (!handle)
 		return -ENODEV;
 
 	obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL,

Re: [PATCH v3 5/7] surface: surface3_power: Drop redundant acpi_bus_get_device() call

[Hans de Goede]

Hi,

On 10/13/21 18:12, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> If the ACPI companion of a given device is not present, the result
> of the ACPI_HANDLE() evaluation for it will be NULL, so calling
> acpi_bus_get_device() on ACPI_HANDLE() result in order to validate
> it is redundant.
> 
> Drop the redundant acpi_bus_get_device() call from mshw0011_notify()
> along with a local variable related to it.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>

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


> ---
> 
> v2 -> v3:
>    * Resend with a different From and S-o-b address and with R-by from
>      Maximilian.  No other changes.
> 
> ---
>  drivers/platform/surface/surface3_power.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: linux-pm/drivers/platform/surface/surface3_power.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/surface/surface3_power.c
> +++ linux-pm/drivers/platform/surface/surface3_power.c
> @@ -159,12 +159,11 @@ mshw0011_notify(struct mshw0011_data *cd
>  		unsigned int *ret_value)
>  {
>  	union acpi_object *obj;
> -	struct acpi_device *adev;
>  	acpi_handle handle;
>  	unsigned int i;
>  
>  	handle = ACPI_HANDLE(&cdata->adp1->dev);
> -	if (!handle || acpi_bus_get_device(handle, &adev))
> +	if (!handle)
>  		return -ENODEV;
>  
>  	obj = acpi_evaluate_dsm_typed(handle, &mshw0011_guid, arg1, arg2, NULL,
> 
> 
> 

[PATCH v1 6/7] platform: x86: ideapad-laptop: Use ACPI_COMPANION() directly

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael@kernel.org>

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 ideapad_acpi_add() accordingly (no intentional functional
impact).

Signed-off-by: Rafael J. Wysocki <rafael@kernel.org>
---
 drivers/platform/x86/ideapad-laptop.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/platform/x86/ideapad-laptop.c
===================================================================
--- linux-pm.orig/drivers/platform/x86/ideapad-laptop.c
+++ linux-pm/drivers/platform/x86/ideapad-laptop.c
@@ -1534,17 +1534,13 @@ static void ideapad_check_features(struc
 
 static int ideapad_acpi_add(struct platform_device *pdev)
 {
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
 	struct ideapad_private *priv;
-	struct acpi_device *adev;
 	acpi_status status;
 	unsigned long cfg;
 	int err, i;
 
-	err = acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev);
-	if (err)
-		return -ENODEV;
-
-	if (eval_int(adev->handle, "_CFG", &cfg))
+	if (!adev || eval_int(adev->handle, "_CFG", &cfg))
 		return -ENODEV;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);

[PATCH v2 6/7] platform: x86: ideapad-laptop: Use ACPI_COMPANION() directly

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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 ideapad_acpi_add() accordingly (no intentional functional
impact).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

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

---
 drivers/platform/x86/ideapad-laptop.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/platform/x86/ideapad-laptop.c
===================================================================
--- linux-pm.orig/drivers/platform/x86/ideapad-laptop.c
+++ linux-pm/drivers/platform/x86/ideapad-laptop.c
@@ -1534,17 +1534,13 @@ static void ideapad_check_features(struc
 
 static int ideapad_acpi_add(struct platform_device *pdev)
 {
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
 	struct ideapad_private *priv;
-	struct acpi_device *adev;
 	acpi_status status;
 	unsigned long cfg;
 	int err, i;
 
-	err = acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev);
-	if (err)
-		return -ENODEV;
-
-	if (eval_int(adev->handle, "_CFG", &cfg))
+	if (!adev || eval_int(adev->handle, "_CFG", &cfg))
 		return -ENODEV;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);

Re: [PATCH v2 6/7] platform: x86: ideapad-laptop: Use ACPI_COMPANION() directly

[Hans de Goede]

Hi,

On 10/13/21 18:13, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 ideapad_acpi_add() accordingly (no intentional functional
> impact).
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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.  No other changes.
> 
> ---
>  drivers/platform/x86/ideapad-laptop.c |    8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> Index: linux-pm/drivers/platform/x86/ideapad-laptop.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/x86/ideapad-laptop.c
> +++ linux-pm/drivers/platform/x86/ideapad-laptop.c
> @@ -1534,17 +1534,13 @@ static void ideapad_check_features(struc
>  
>  static int ideapad_acpi_add(struct platform_device *pdev)
>  {
> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>  	struct ideapad_private *priv;
> -	struct acpi_device *adev;
>  	acpi_status status;
>  	unsigned long cfg;
>  	int err, i;
>  
> -	err = acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev);
> -	if (err)
> -		return -ENODEV;
> -
> -	if (eval_int(adev->handle, "_CFG", &cfg))
> +	if (!adev || eval_int(adev->handle, "_CFG", &cfg))
>  		return -ENODEV;
>  
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> 
> 
>