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

Mauro Carvalho Chehab
2021-07-22T08:57:56Z

Hi Andy,

Em Wed, 26 May 2021 15:43:18 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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@linux.intel.com>

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

> ---
> .../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