Message ID | 20230812071423.3481790-1-ruanjinjie@huawei.com |
---|---|
State | New |
Headers | show |
Series | [-next] gpiolib: acpi: Use LIST_HEAD() to initialize the list_head | expand |
On Sat, Aug 12, 2023 at 9:14 AM Ruan Jinjie <ruanjinjie@huawei.com> wrote: > > Use LIST_HEAD() to initialize the list_head instead of open-coding it. > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > --- > drivers/gpio/gpiolib-acpi.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c > index fbda452fb4d6..db8e8e967bda 100644 > --- a/drivers/gpio/gpiolib-acpi.c > +++ b/drivers/gpio/gpiolib-acpi.c > @@ -809,11 +809,9 @@ static int acpi_gpio_resource_lookup(struct acpi_gpio_lookup *lookup, > struct acpi_gpio_info *info) > { > struct acpi_device *adev = lookup->info.adev; > - struct list_head res_list; > + LIST_HEAD(res_list); > int ret; > > - INIT_LIST_HEAD(&res_list); > - > ret = acpi_dev_get_resources(adev, &res_list, > acpi_populate_gpio_lookup, > lookup); > @@ -1472,13 +1470,12 @@ int acpi_gpio_count(struct device *dev, const char *con_id) > > /* Then from plain _CRS GPIOs */ > if (count < 0) { > - struct list_head resource_list; > + LIST_HEAD(resource_list); > unsigned int crs_count = 0; > > if (!acpi_can_fallback_to_crs(adev, con_id)) > return count; > > - INIT_LIST_HEAD(&resource_list); > acpi_dev_get_resources(adev, &resource_list, > acpi_find_gpio_count, &crs_count); > acpi_dev_free_resource_list(&resource_list); > -- > 2.34.1 > My opinion: this is actually less readable than what is there now and I doubt the compiler would generate less code. With this change I now need to check what LIST_HEAD() actually does, while defining a temp variable of a clear type and then using a macro to initialize it is much more obvious. I'll let Andy decide as ACPI is his domain but for me it's a NAK. Bart
On Sun, Aug 13, 2023 at 08:49:28PM +0200, Bartosz Golaszewski wrote: > On Sat, Aug 12, 2023 at 9:14 AM Ruan Jinjie <ruanjinjie@huawei.com> wrote: > > > > Use LIST_HEAD() to initialize the list_head instead of open-coding it. ... > My opinion: this is actually less readable than what is there now and > I doubt the compiler would generate less code. With this change I now > need to check what LIST_HEAD() actually does, while defining a temp > variable of a clear type and then using a macro to initialize it is > much more obvious. > > I'll let Andy decide as ACPI is his domain but for me it's a NAK. The code is identical, I have no strong opinion (I'm fine with the either), since you already have an opinion, let it be NAK then.
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index fbda452fb4d6..db8e8e967bda 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -809,11 +809,9 @@ static int acpi_gpio_resource_lookup(struct acpi_gpio_lookup *lookup, struct acpi_gpio_info *info) { struct acpi_device *adev = lookup->info.adev; - struct list_head res_list; + LIST_HEAD(res_list); int ret; - INIT_LIST_HEAD(&res_list); - ret = acpi_dev_get_resources(adev, &res_list, acpi_populate_gpio_lookup, lookup); @@ -1472,13 +1470,12 @@ int acpi_gpio_count(struct device *dev, const char *con_id) /* Then from plain _CRS GPIOs */ if (count < 0) { - struct list_head resource_list; + LIST_HEAD(resource_list); unsigned int crs_count = 0; if (!acpi_can_fallback_to_crs(adev, con_id)) return count; - INIT_LIST_HEAD(&resource_list); acpi_dev_get_resources(adev, &resource_list, acpi_find_gpio_count, &crs_count); acpi_dev_free_resource_list(&resource_list);
Use LIST_HEAD() to initialize the list_head instead of open-coding it. Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> --- drivers/gpio/gpiolib-acpi.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)