Re: [PATCH v1 2/6] staging: atomisp: Replace open-coded i2c_acpi_find_client_by_adev()

[Mauro Carvalho Chehab]

Hi Andy,

Em Wed, 26 May 2021 15:43:18 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> escreveu:

> gmin_i2c_dev_exists() is using open-coded variant of
> i2c_acpi_find_client_by_adev(). Replace it with a corresponding call.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

At least on the top of v5.14-rc1, this patch causes a compilation
issue:

	drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c: In function ‘gmin_i2c_dev_exists’:
	drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c:386:19: error: implicit declaration of function ‘i2c_acpi_find_client_by_adev’; did you mean ‘i2c_acpi_find_adapter_by_handle’? [-Werror=implicit-function-declaration]
	  386 |         *client = i2c_acpi_find_client_by_adev(adev);
	      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
	      |                   i2c_acpi_find_adapter_by_handle
	drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c:386:17: warning: assignment to ‘struct i2c_client *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
	  386 |         *client = i2c_acpi_find_client_by_adev(adev);
	      |                 ^

The reason is because such function is static:

	$ git grep i2c_acpi_find_client_by_adev
	drivers/i2c/i2c-core-acpi.c:static struct i2c_client *i2c_acpi_find_client_by_adev(struct acpi_device *adev)

IMO, a patch like that should be applied at the same tree as a patch
dropping "static" from drivers/i2c/i2c-core-acpi.c. If you want to do
so, feel free to add:

Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>

> ---
>  .../staging/media/atomisp/pci/atomisp_gmin_platform.c    | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index 135994d44802..a1064d1a3d6b 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -378,19 +378,14 @@ static struct i2c_client *gmin_i2c_dev_exists(struct device *dev, char *name,
>  					      struct i2c_client **client)
>  {
>  	struct acpi_device *adev;
> -	struct device *d;
>  
>  	adev = acpi_dev_get_first_match_dev(name, NULL, -1);
>  	if (!adev)
>  		return NULL;
>  
> -	d = bus_find_device_by_acpi_dev(&i2c_bus_type, adev);
> -	acpi_dev_put(adev);
> -	if (!d)
> -		return NULL;
> +	*client = i2c_acpi_find_client_by_adev(adev);
>  
> -	*client = i2c_verify_client(d);
> -	put_device(d);
> +	acpi_dev_put(adev);
>  
>  	dev_dbg(dev, "found '%s' at address 0x%02x, adapter %d\n",
>  		(*client)->name, (*client)->addr, (*client)->adapter->nr);



Thanks,
Mauro