acpi_dev_get_resources() does perform the NULL pointer check against ACPI companion device which is given as function parameter. Thus, there is no need to duplicate this check in the caller.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 24 ++++++++------------ 1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c index 574dcfec9577..6f9e08cda964 100644 --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c @@ -426,23 +426,19 @@ static void i2c_acpi_find_adapter(struct intel_dsi *intel_dsi, { struct drm_device *drm_dev = intel_dsi->base.base.dev; struct device *dev = &drm_dev->pdev->dev; - struct acpi_device *acpi_dev; + struct acpi_device *acpi_dev = ACPI_COMPANION(dev); struct list_head resource_list; struct i2c_adapter_lookup lookup;
- acpi_dev = ACPI_COMPANION(dev); - if (acpi_dev) { - memset(&lookup, 0, sizeof(lookup)); - lookup.slave_addr = slave_addr; - lookup.intel_dsi = intel_dsi; - lookup.dev_handle = acpi_device_handle(acpi_dev); - - INIT_LIST_HEAD(&resource_list); - acpi_dev_get_resources(acpi_dev, &resource_list, - i2c_adapter_lookup, - &lookup); - acpi_dev_free_resource_list(&resource_list); - } + memset(&lookup, 0, sizeof(lookup)); + lookup.slave_addr = slave_addr; + lookup.intel_dsi = intel_dsi; + lookup.dev_handle = acpi_device_handle(acpi_dev); + + INIT_LIST_HEAD(&resource_list); + acpi_dev_get_resources(acpi_dev, &resource_list, + i2c_adapter_lookup, &lookup); + acpi_dev_free_resource_list(&resource_list); } #else static inline void i2c_acpi_find_adapter(struct intel_dsi *intel_dsi,
On Fri, May 29, 2020 at 03:33:17PM +0300, Andy Shevchenko wrote:
acpi_dev_get_resources() does perform the NULL pointer check against ACPI companion device which is given as function parameter. Thus, there is no need to duplicate this check in the caller.
Any comment so far?
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 24 ++++++++------------ 1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c index 574dcfec9577..6f9e08cda964 100644 --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c @@ -426,23 +426,19 @@ static void i2c_acpi_find_adapter(struct intel_dsi *intel_dsi, { struct drm_device *drm_dev = intel_dsi->base.base.dev; struct device *dev = &drm_dev->pdev->dev;
- struct acpi_device *acpi_dev;
- struct acpi_device *acpi_dev = ACPI_COMPANION(dev); struct list_head resource_list; struct i2c_adapter_lookup lookup;
- acpi_dev = ACPI_COMPANION(dev);
- if (acpi_dev) {
memset(&lookup, 0, sizeof(lookup));lookup.slave_addr = slave_addr;lookup.intel_dsi = intel_dsi;lookup.dev_handle = acpi_device_handle(acpi_dev);INIT_LIST_HEAD(&resource_list);acpi_dev_get_resources(acpi_dev, &resource_list,i2c_adapter_lookup,&lookup);acpi_dev_free_resource_list(&resource_list);- }
- memset(&lookup, 0, sizeof(lookup));
- lookup.slave_addr = slave_addr;
- lookup.intel_dsi = intel_dsi;
- lookup.dev_handle = acpi_device_handle(acpi_dev);
- INIT_LIST_HEAD(&resource_list);
- acpi_dev_get_resources(acpi_dev, &resource_list,
i2c_adapter_lookup, &lookup);- acpi_dev_free_resource_list(&resource_list);
} #else static inline void i2c_acpi_find_adapter(struct intel_dsi *intel_dsi, -- 2.26.2
On Fri, May 29, 2020 at 03:33:17PM +0300, Andy Shevchenko wrote:
acpi_dev_get_resources() does perform the NULL pointer check against ACPI companion device which is given as function parameter. Thus, there is no need to duplicate this check in the caller.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
Sorry, I did look at this but apparently forgot to reply...
drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 24 ++++++++------------ 1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c index 574dcfec9577..6f9e08cda964 100644 --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c @@ -426,23 +426,19 @@ static void i2c_acpi_find_adapter(struct intel_dsi *intel_dsi, { struct drm_device *drm_dev = intel_dsi->base.base.dev; struct device *dev = &drm_dev->pdev->dev;
- struct acpi_device *acpi_dev;
- struct acpi_device *acpi_dev = ACPI_COMPANION(dev); struct list_head resource_list; struct i2c_adapter_lookup lookup;
- acpi_dev = ACPI_COMPANION(dev);
- if (acpi_dev) {
memset(&lookup, 0, sizeof(lookup));lookup.slave_addr = slave_addr;lookup.intel_dsi = intel_dsi;lookup.dev_handle = acpi_device_handle(acpi_dev);INIT_LIST_HEAD(&resource_list);acpi_dev_get_resources(acpi_dev, &resource_list,i2c_adapter_lookup,&lookup);acpi_dev_free_resource_list(&resource_list);- }
- memset(&lookup, 0, sizeof(lookup));
- lookup.slave_addr = slave_addr;
- lookup.intel_dsi = intel_dsi;
- lookup.dev_handle = acpi_device_handle(acpi_dev);
struct i2c_adapter_lookup lookup = { .slave_addr = ... };
?
- INIT_LIST_HEAD(&resource_list);
Declare as LIST_HEAD(resource_list); ?
- acpi_dev_get_resources(acpi_dev, &resource_list,
i2c_adapter_lookup, &lookup);- acpi_dev_free_resource_list(&resource_list);
I was very confused by this code since on the first glance it appears to absolutely nothing. After a deeper look it looks like i2c_adapter_lookup() magically mutates intel_dsi->i2c_bus_num. Did I mention I hate functions with side effects? IMO would be much better if i2c_adapter_lookup() did what it says on the tin and just returned the adapter number and let the caller deal with it. But this is a pre-existing issue with the code and so not directly related to your patch.
} #else static inline void i2c_acpi_find_adapter(struct intel_dsi *intel_dsi, -- 2.26.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
dri-devel@lists.freedesktop.org
-
Andy Shevchenko -
Ville Syrjälä