lkml.org 
[lkml]   [2024]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v1 2/2] media: atomisp: Replace open-coded i2c_find_device_by_fwnode()
From
Hi Andy,

Thank you for the patch.

On 3/26/24 9:27 PM, Andy Shevchenko wrote:
> gmin_i2c_dev_exists() is using open-coded variant of
> i2c_find_device_by_fwnode(). Replace it with a corresponding call.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index 80aa2211cdc3..b7c477280986 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -394,12 +394,10 @@ static struct i2c_client *gmin_i2c_dev_exists(struct device *dev, char *name,
> if (!adev)
> return NULL;
>
> - d = bus_find_device_by_acpi_dev(&i2c_bus_type, adev);
> + d = get_device(acpi_get_first_physical_node(adev));
> acpi_dev_put(adev);
> - if (!d)
> - return NULL;
>
> - *client = i2c_verify_client(d);
> + *client = i2c_find_device_by_fwnode(dev_fwnode(d));
> put_device(d);
>
> dev_dbg(dev, "found '%s' at address 0x%02x, adapter %d\n",

If we are ging to simplify this I think the change should be:

From ea66d15b9a72fcb8baf22a2ff059f2b842a91b67 Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Wed, 10 Apr 2024 12:32:11 +0200
Subject: [PATCH] media: atomisp: Replace open-coded
i2c_find_device_by_fwnode()

gmin_i2c_dev_exists() is using open-coded variant of
i2c_find_device_by_fwnode(). Replace it with a corresponding call.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Directly use the ACPI fwnode through acpi_fwnode_handle(adev) instead
of first calling acpi_get_first_physical_node(adev)
---
drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
index 804ffff245f3..ed6214327ce5 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
@@ -394,14 +394,11 @@ static struct i2c_client *gmin_i2c_dev_exists(struct device *dev, char *name,
if (!adev)
return NULL;

- d = bus_find_device_by_acpi_dev(&i2c_bus_type, adev);
+ *client = i2c_find_device_by_fwnode(acpi_fwnode_handle(adev));
acpi_dev_put(adev);
- if (!d)
+ if (!*client)
return NULL;

- *client = i2c_verify_client(d);
- put_device(d);
-
dev_dbg(dev, "found '%s' at address 0x%02x, adapter %d\n",
(*client)->name, (*client)->addr, (*client)->adapter->nr);
return *client;
--
2.44.0
Note I added a:

if (!*client)
return NULL;
To fix a bug in the original code where the dev_dbg() arguments
are dereferencing a potential NULL *client ptr.

I'm going to merge this variant into my media-atomisp branch
instead of the orignal.

Regards,

Hans




\
 
 \ /
  Last update: 2024-05-27 16:32    [W:0.078 / U:0.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site