Re: [PATCH v2 4/9] i2c: i801: Improve is_dell_system_with_lis3lv02d

[Jean Delvare <jdelvare@suse.de>]

Hi Heiner,

On Fri, 06 Aug 2021 23:15:15 +0200, Heiner Kallweit wrote:
> Replace the ugly cast of the return_value pointer with proper usage.
> In addition use dmi_match() instead of open-coding it.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
>   - avoid assigning potentially dangling pointer to *return_value
> ---
>  drivers/i2c/busses/i2c-i801.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 89ae78ef1..f56060fcf 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1192,7 +1192,7 @@ static acpi_status check_acpi_smo88xx_device(acpi_handle obj_handle,
>  
>  	kfree(info);
>  
> -	*((bool *)return_value) = true;
> +	*return_value = NULL;
>  	return AE_CTRL_TERMINATE;
>  
>  smo88xx_not_found:
> @@ -1202,11 +1202,9 @@ static acpi_status check_acpi_smo88xx_device(acpi_handle obj_handle,
>  
>  static bool is_dell_system_with_lis3lv02d(void)
>  {
> -	bool found;
> -	const char *vendor;
> +	void *err = ERR_PTR(-ENOENT);
>  
> -	vendor = dmi_get_system_info(DMI_SYS_VENDOR);
> -	if (!vendor || strcmp(vendor, "Dell Inc."))
> +	if (!dmi_match(DMI_SYS_VENDOR, "Dell Inc."))
>  		return false;
>  
>  	/*
> @@ -1217,11 +1215,9 @@ static bool is_dell_system_with_lis3lv02d(void)
>  	 * accelerometer but unfortunately ACPI does not provide any other
>  	 * information (like I2C address).
>  	 */
> -	found = false;
> -	acpi_get_devices(NULL, check_acpi_smo88xx_device, NULL,
> -			 (void **)&found);
> +	acpi_get_devices(NULL, check_acpi_smo88xx_device, NULL, &err);
>  
> -	return found;
> +	return !IS_ERR(err);
>  }
>  
>  /*

OK, it's a bit different from what I proposed but it still addresses the
concerns that had been raised. So fine with me.

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support