[PATCH v1 1/1] platform/surface: Switch to use acpi_evaluate_dsm_typed()

[PATCH v1 1/1] platform/surface: Switch to use acpi_evaluate_dsm_typed()

[Andy Shevchenko]

The acpi_evaluate_dsm_typed() provides a way to check the type of the
object evaluated by _DSM call. Use it instead of open coded variant.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/surface/surface_hotplug.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/surface/surface_hotplug.c b/drivers/platform/surface/surface_hotplug.c
index f004a2495201..7b6d887dccdb 100644
--- a/drivers/platform/surface/surface_hotplug.c
+++ b/drivers/platform/surface/surface_hotplug.c
@@ -101,18 +101,12 @@ static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type
 	param.type = ACPI_TYPE_INTEGER;
 	param.integer.value = value;
 
-	result = acpi_evaluate_dsm(handle, &shps_dsm_guid, SHPS_DSM_REVISION,
-				   shps_dsm_fn_for_irq(type), &param);
-
+	result = acpi_evaluate_dsm_typed(handle, &shps_dsm_guid, SHPS_DSM_REVISION,
+					 shps_dsm_fn_for_irq(type), &param, ACPI_TYPE_BUFFER);
 	if (!result) {
 		dev_err(&pdev->dev, "IRQ notification via DSM failed (irq=%d, gpio=%d)\n",
 			type, value);
 
-	} else if (result->type != ACPI_TYPE_BUFFER) {
-		dev_err(&pdev->dev,
-			"IRQ notification via DSM failed: unexpected result type (irq=%d, gpio=%d)\n",
-			type, value);
-
 	} else if (result->buffer.length != 1 || result->buffer.pointer[0] != 0) {
 		dev_err(&pdev->dev,
 			"IRQ notification via DSM failed: unexpected result value (irq=%d, gpio=%d)\n",
@@ -121,8 +115,7 @@ static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type
 
 	mutex_unlock(&sdev->lock[type]);
 
-	if (result)
-		ACPI_FREE(result);
+	ACPI_FREE(result);
 }
 
 static irqreturn_t shps_handle_irq(int irq, void *data)
-- 
2.39.0

Re: [PATCH v1 1/1] platform/surface: Switch to use acpi_evaluate_dsm_typed()

[Maximilian Luz]

On 1/18/23 10:38, Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I think the reason I went with the non-typed version here was for the
explicit error message. Specifically, to simplify debugging in case we'd
get any new device that behaves differently. But since then I think MS
discontinued the use of this driver (or at least this part) in their
devices (no more detachable GPUs). Also, the likelihood of that
interface changing is pretty small regardless.

So I'm fine with changing that.

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

> ---
>   drivers/platform/surface/surface_hotplug.c | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/surface/surface_hotplug.c b/drivers/platform/surface/surface_hotplug.c
> index f004a2495201..7b6d887dccdb 100644
> --- a/drivers/platform/surface/surface_hotplug.c
> +++ b/drivers/platform/surface/surface_hotplug.c
> @@ -101,18 +101,12 @@ static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type
>   	param.type = ACPI_TYPE_INTEGER;
>   	param.integer.value = value;
>   
> -	result = acpi_evaluate_dsm(handle, &shps_dsm_guid, SHPS_DSM_REVISION,
> -				   shps_dsm_fn_for_irq(type), &param);
> -
> +	result = acpi_evaluate_dsm_typed(handle, &shps_dsm_guid, SHPS_DSM_REVISION,
> +					 shps_dsm_fn_for_irq(type), &param, ACPI_TYPE_BUFFER);
>   	if (!result) {
>   		dev_err(&pdev->dev, "IRQ notification via DSM failed (irq=%d, gpio=%d)\n",
>   			type, value);
>   
> -	} else if (result->type != ACPI_TYPE_BUFFER) {
> -		dev_err(&pdev->dev,
> -			"IRQ notification via DSM failed: unexpected result type (irq=%d, gpio=%d)\n",
> -			type, value);
> -
>   	} else if (result->buffer.length != 1 || result->buffer.pointer[0] != 0) {
>   		dev_err(&pdev->dev,
>   			"IRQ notification via DSM failed: unexpected result value (irq=%d, gpio=%d)\n",
> @@ -121,8 +115,7 @@ static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type
>   
>   	mutex_unlock(&sdev->lock[type]);
>   
> -	if (result)
> -		ACPI_FREE(result);
> +	ACPI_FREE(result);
>   }
>   
>   static irqreturn_t shps_handle_irq(int irq, void *data)

Re: [PATCH v1 1/1] platform/surface: Switch to use acpi_evaluate_dsm_typed()

[Hans de Goede]

Hi Andy,

On 1/18/23 10:38, Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thank you for the 3 patches switching to acpi_evaluate_dsm_typed().
I have added all 3 to my review-hans  branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Regards,

Hans


> ---
>  drivers/platform/surface/surface_hotplug.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/surface/surface_hotplug.c b/drivers/platform/surface/surface_hotplug.c
> index f004a2495201..7b6d887dccdb 100644
> --- a/drivers/platform/surface/surface_hotplug.c
> +++ b/drivers/platform/surface/surface_hotplug.c
> @@ -101,18 +101,12 @@ static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type
>  	param.type = ACPI_TYPE_INTEGER;
>  	param.integer.value = value;
>  
> -	result = acpi_evaluate_dsm(handle, &shps_dsm_guid, SHPS_DSM_REVISION,
> -				   shps_dsm_fn_for_irq(type), &param);
> -
> +	result = acpi_evaluate_dsm_typed(handle, &shps_dsm_guid, SHPS_DSM_REVISION,
> +					 shps_dsm_fn_for_irq(type), &param, ACPI_TYPE_BUFFER);
>  	if (!result) {
>  		dev_err(&pdev->dev, "IRQ notification via DSM failed (irq=%d, gpio=%d)\n",
>  			type, value);
>  
> -	} else if (result->type != ACPI_TYPE_BUFFER) {
> -		dev_err(&pdev->dev,
> -			"IRQ notification via DSM failed: unexpected result type (irq=%d, gpio=%d)\n",
> -			type, value);
> -
>  	} else if (result->buffer.length != 1 || result->buffer.pointer[0] != 0) {
>  		dev_err(&pdev->dev,
>  			"IRQ notification via DSM failed: unexpected result value (irq=%d, gpio=%d)\n",
> @@ -121,8 +115,7 @@ static void shps_dsm_notify_irq(struct platform_device *pdev, enum shps_irq_type
>  
>  	mutex_unlock(&sdev->lock[type]);
>  
> -	if (result)
> -		ACPI_FREE(result);
> +	ACPI_FREE(result);
>  }
>  
>  static irqreturn_t shps_handle_irq(int irq, void *data)