2021-05-26 13:16:47

by Andy Shevchenko

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

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 <[email protected]>
---
.../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);
--
2.30.2


2021-07-22 09:00:51

by Mauro Carvalho Chehab

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

Hi Andy,

Em Wed, 26 May 2021 15:43:18 +0300
Andy Shevchenko <[email protected]> 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 <[email protected]>

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 <[email protected]>

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

2021-07-22 09:05:21

by Andy Shevchenko

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

On Thu, Jul 22, 2021 at 10:57:44AM +0200, Mauro Carvalho Chehab wrote:
> Em Wed, 26 May 2021 15:43:18 +0300
> Andy Shevchenko <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>

Thanks!

There is a v2 of this where the patch is dropped from.


--
With Best Regards,
Andy Shevchenko