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