linux-kernel - Re: [PATCH v1 1/1] platform/surface: Switch to use acpi_evaluate_dsm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e40d3977-17bd-be9c-8ea5-1e5f7c2d47b0@gmail.com>
Date:   Wed, 18 Jan 2023 10:48:06 +0100
From:   Maximilian Luz <luzmaximilian@...il.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Mark Gross <markgross@...nel.org>
Subject: Re: [PATCH v1 1/1] platform/surface: Switch to use
 acpi_evaluate_dsm_typed()

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@...ux.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@...il.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)