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@xxxxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- 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
