* [PATCH v2 1/2] gpiolib: improve coding style for local variables @ 2021-11-18 13:23 Bartosz Golaszewski 2021-11-18 13:23 ` [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code Bartosz Golaszewski ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Bartosz Golaszewski @ 2021-11-18 13:23 UTC (permalink / raw) To: Linus Walleij, Andy Shevchenko Cc: linux-gpio, linux-kernel, Bartosz Golaszewski Drop unneeded whitespaces and put the variables of the same type together for consistency with the rest of the code. Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/gpio/gpiolib.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index abfbf546d159..20d63028b85c 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -594,11 +594,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, struct lock_class_key *request_key) { struct fwnode_handle *fwnode = gc->parent ? dev_fwnode(gc->parent) : NULL; - unsigned long flags; - int ret = 0; - unsigned i; - int base = gc->base; + int ret = 0, base = gc->base; struct gpio_device *gdev; + unsigned long flags; + unsigned int i; /* * First: allocate and populate the internal stat container, and -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 13:23 [PATCH v2 1/2] gpiolib: improve coding style for local variables Bartosz Golaszewski @ 2021-11-18 13:23 ` Bartosz Golaszewski 2021-11-18 15:44 ` Andy Shevchenko 2021-11-18 17:05 ` Andy Shevchenko 2021-11-18 15:44 ` [PATCH v2 1/2] gpiolib: improve coding style for local variables Andy Shevchenko 2021-11-19 7:57 ` Johan Hovold 2 siblings, 2 replies; 13+ messages in thread From: Bartosz Golaszewski @ 2021-11-18 13:23 UTC (permalink / raw) To: Linus Walleij, Andy Shevchenko Cc: linux-gpio, linux-kernel, Bartosz Golaszewski Several drivers read the 'ngpios' device property on their own, but since it's defined as a standard GPIO property in the device tree bindings anyway, it's a good candidate for generalization. If the driver didn't set its gc->ngpio, try to read the 'ngpios' property from the GPIO device's firmware node before bailing out. Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> --- v1 -> v2: - use device_property_read_u32() instead of fwnode_property_read_u32() - reverse the error check logic drivers/gpio/gpiolib.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 20d63028b85c..0746615073c6 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -598,6 +598,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, struct gpio_device *gdev; unsigned long flags; unsigned int i; + u32 ngpios; /* * First: allocate and populate the internal stat container, and @@ -646,9 +647,14 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, } if (gc->ngpio == 0) { - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); - ret = -EINVAL; - goto err_free_descs; + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); + if (ret) { + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); + ret = -EINVAL; + goto err_free_descs; + } + + gc->ngpio = ngpios; } if (gc->ngpio > FASTPATH_NGPIO) -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 13:23 ` [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code Bartosz Golaszewski @ 2021-11-18 15:44 ` Andy Shevchenko 2021-11-18 16:38 ` Bartosz Golaszewski 2021-11-18 17:05 ` Andy Shevchenko 1 sibling, 1 reply; 13+ messages in thread From: Andy Shevchenko @ 2021-11-18 15:44 UTC (permalink / raw) To: Bartosz Golaszewski; +Cc: Linus Walleij, linux-gpio, linux-kernel On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: > Several drivers read the 'ngpios' device property on their own, but > since it's defined as a standard GPIO property in the device tree bindings > anyway, it's a good candidate for generalization. If the driver didn't > set its gc->ngpio, try to read the 'ngpios' property from the GPIO > device's firmware node before bailing out. Thanks for update, my comment below. ... > if (gc->ngpio == 0) { > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > - ret = -EINVAL; > - goto err_free_descs; > + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > + if (ret) { > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > + ret = -EINVAL; Sorry, forgot to ask, why this is needed? > + goto err_free_descs; > + } > + > + gc->ngpio = ngpios; > } -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 15:44 ` Andy Shevchenko @ 2021-11-18 16:38 ` Bartosz Golaszewski 2021-11-18 17:01 ` Andy Shevchenko 0 siblings, 1 reply; 13+ messages in thread From: Bartosz Golaszewski @ 2021-11-18 16:38 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Thu, Nov 18, 2021 at 4:46 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: > > Several drivers read the 'ngpios' device property on their own, but > > since it's defined as a standard GPIO property in the device tree bindings > > anyway, it's a good candidate for generalization. If the driver didn't > > set its gc->ngpio, try to read the 'ngpios' property from the GPIO > > device's firmware node before bailing out. > > Thanks for update, my comment below. > > ... > > > if (gc->ngpio == 0) { > > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > - ret = -EINVAL; > > - goto err_free_descs; > > + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > + if (ret) { > > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > + ret = -EINVAL; > > Sorry, forgot to ask, why this is needed? > What do you mean? 0 lines doesn't sound like a valid value so -EINVAL is in order. Bart ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 16:38 ` Bartosz Golaszewski @ 2021-11-18 17:01 ` Andy Shevchenko 2021-11-18 17:06 ` Andy Shevchenko 0 siblings, 1 reply; 13+ messages in thread From: Andy Shevchenko @ 2021-11-18 17:01 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Thu, Nov 18, 2021 at 05:38:14PM +0100, Bartosz Golaszewski wrote: > On Thu, Nov 18, 2021 at 4:46 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: > > > Several drivers read the 'ngpios' device property on their own, but > > > since it's defined as a standard GPIO property in the device tree bindings > > > anyway, it's a good candidate for generalization. If the driver didn't > > > set its gc->ngpio, try to read the 'ngpios' property from the GPIO > > > device's firmware node before bailing out. > > > > Thanks for update, my comment below. > > > > ... > > > > > if (gc->ngpio == 0) { > > > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > - ret = -EINVAL; > > > - goto err_free_descs; > > > + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > > + if (ret) { > > > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > > > + ret = -EINVAL; > > > > Sorry, forgot to ask, why this is needed? > > What do you mean? 0 lines doesn't sound like a valid value so -EINVAL > is in order. What is so special about -EINVAL? Why ret can't be returned? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 17:01 ` Andy Shevchenko @ 2021-11-18 17:06 ` Andy Shevchenko 0 siblings, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2021-11-18 17:06 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Thu, Nov 18, 2021 at 07:01:42PM +0200, Andy Shevchenko wrote: > On Thu, Nov 18, 2021 at 05:38:14PM +0100, Bartosz Golaszewski wrote: > > On Thu, Nov 18, 2021 at 4:46 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: ... > > > > + if (ret) { > > > > + ret = -EINVAL; > > > > > > Sorry, forgot to ask, why this is needed? > > > > What do you mean? 0 lines doesn't sound like a valid value so -EINVAL > > is in order. > > What is so special about -EINVAL? Why ret can't be returned? See another mail, I explained how the code should be. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 13:23 ` [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code Bartosz Golaszewski 2021-11-18 15:44 ` Andy Shevchenko @ 2021-11-18 17:05 ` Andy Shevchenko 2021-11-18 20:12 ` Bartosz Golaszewski 1 sibling, 1 reply; 13+ messages in thread From: Andy Shevchenko @ 2021-11-18 17:05 UTC (permalink / raw) To: Bartosz Golaszewski; +Cc: Linus Walleij, linux-gpio, linux-kernel On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: > Several drivers read the 'ngpios' device property on their own, but > since it's defined as a standard GPIO property in the device tree bindings > anyway, it's a good candidate for generalization. If the driver didn't > set its gc->ngpio, try to read the 'ngpios' property from the GPIO > device's firmware node before bailing out. ... > if (gc->ngpio == 0) { > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > - ret = -EINVAL; > - goto err_free_descs; > + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > + if (ret) { > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > + ret = -EINVAL; > + goto err_free_descs; > + } > + > + gc->ngpio = ngpios; > } This should be if (gc->ngpio == 0) { ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); if (ret) return ret; gc->ngpio = ngpios; } if (gc->ngpio == 0) { chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); ret = -EINVAL; goto err_free_descs; } -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 17:05 ` Andy Shevchenko @ 2021-11-18 20:12 ` Bartosz Golaszewski 2021-11-18 21:16 ` Andy Shevchenko 0 siblings, 1 reply; 13+ messages in thread From: Bartosz Golaszewski @ 2021-11-18 20:12 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Thu, Nov 18, 2021 at 6:06 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: > > Several drivers read the 'ngpios' device property on their own, but > > since it's defined as a standard GPIO property in the device tree bindings > > anyway, it's a good candidate for generalization. If the driver didn't > > set its gc->ngpio, try to read the 'ngpios' property from the GPIO > > device's firmware node before bailing out. > > ... > > > if (gc->ngpio == 0) { > > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > - ret = -EINVAL; > > - goto err_free_descs; > > + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > + if (ret) { > > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > + ret = -EINVAL; > > + goto err_free_descs; > > + } > > + > > + gc->ngpio = ngpios; > > } > > This should be > > if (gc->ngpio == 0) { > ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > if (ret) > return ret; But device_property_read_u32() returning -ENODATA means there's no such property, which should actually be converted to -EINVAL as the caller wanting to create the chip provided invalid configuration - in this case: a chip with 0 lines. In case of the non-array variant of read_u32 that's also the only error that can be returned so this bit looks right to me. Bart > > gc->ngpio = ngpios; > } > > if (gc->ngpio == 0) { > chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > ret = -EINVAL; > goto err_free_descs; > } > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 20:12 ` Bartosz Golaszewski @ 2021-11-18 21:16 ` Andy Shevchenko 2021-11-19 19:35 ` Bartosz Golaszewski 0 siblings, 1 reply; 13+ messages in thread From: Andy Shevchenko @ 2021-11-18 21:16 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Thu, Nov 18, 2021 at 09:12:59PM +0100, Bartosz Golaszewski wrote: > On Thu, Nov 18, 2021 at 6:06 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: > > > Several drivers read the 'ngpios' device property on their own, but > > > since it's defined as a standard GPIO property in the device tree bindings > > > anyway, it's a good candidate for generalization. If the driver didn't > > > set its gc->ngpio, try to read the 'ngpios' property from the GPIO > > > device's firmware node before bailing out. > > > > ... > > > > > if (gc->ngpio == 0) { > > > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > - ret = -EINVAL; > > > - goto err_free_descs; > > > + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > > + if (ret) { > > > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > + ret = -EINVAL; > > > + goto err_free_descs; > > > + } > > > + > > > + gc->ngpio = ngpios; > > > } > > > > This should be > > > > if (gc->ngpio == 0) { > > ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > if (ret) > > return ret; > > But device_property_read_u32() returning -ENODATA means there's no > such property, which should actually be converted to -EINVAL as the > caller wanting to create the chip provided invalid configuration - in > this case: a chip with 0 lines. In case of the non-array variant of > read_u32 that's also the only error that can be returned so this bit > looks right to me. So, what is so special about -EINVAL? Why -ENODATA is not good enough which will exactly explain to the caller what's going on, no? > > gc->ngpio = ngpios; > > } > > > > if (gc->ngpio == 0) { > > chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > ret = -EINVAL; > > goto err_free_descs; When the caller intended to create a chip with 0 GPIOs they will get an error as you wish with an error message. > > } -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-18 21:16 ` Andy Shevchenko @ 2021-11-19 19:35 ` Bartosz Golaszewski 2021-11-22 11:20 ` Andy Shevchenko 0 siblings, 1 reply; 13+ messages in thread From: Bartosz Golaszewski @ 2021-11-19 19:35 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Thu, Nov 18, 2021 at 10:16 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Nov 18, 2021 at 09:12:59PM +0100, Bartosz Golaszewski wrote: > > On Thu, Nov 18, 2021 at 6:06 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: > > > > Several drivers read the 'ngpios' device property on their own, but > > > > since it's defined as a standard GPIO property in the device tree bindings > > > > anyway, it's a good candidate for generalization. If the driver didn't > > > > set its gc->ngpio, try to read the 'ngpios' property from the GPIO > > > > device's firmware node before bailing out. > > > > > > ... > > > > > > > if (gc->ngpio == 0) { > > > > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > > - ret = -EINVAL; > > > > - goto err_free_descs; > > > > + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > > > + if (ret) { > > > > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > > + ret = -EINVAL; > > > > + goto err_free_descs; > > > > + } > > > > + > > > > + gc->ngpio = ngpios; > > > > } > > > > > > This should be > > > > > > if (gc->ngpio == 0) { > > > ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > > if (ret) > > > return ret; > > > > But device_property_read_u32() returning -ENODATA means there's no > > such property, which should actually be converted to -EINVAL as the > > caller wanting to create the chip provided invalid configuration - in > > this case: a chip with 0 lines. In case of the non-array variant of > > read_u32 that's also the only error that can be returned so this bit > > looks right to me. > > So, what is so special about -EINVAL? Why -ENODATA is not good enough which > will exactly explain to the caller what's going on, no? > Let's imagine the user sets gc->ngpio = 0 incorrectly thinking it'll make gpiolib set it to some sane default. Then gpiochip_add_data() returns -ENODATA (No data available). This is confusing IMO. But if we convert it to -EINVAL, it now says "Invalid value" which points to the wrong configuration. ENODATA means "device tree property is not present" in this case but the problem is that user supplies the gpiolib with invalid configuration. EINVAL is the right error here. Bart > > > gc->ngpio = ngpios; > > > } > > > > > > if (gc->ngpio == 0) { > > > chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > ret = -EINVAL; > > > goto err_free_descs; > > When the caller intended to create a chip with 0 GPIOs they will get an error > as you wish with an error message. > Yes, as it was before. Bart > > > } > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code 2021-11-19 19:35 ` Bartosz Golaszewski @ 2021-11-22 11:20 ` Andy Shevchenko 0 siblings, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2021-11-22 11:20 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Fri, Nov 19, 2021 at 08:35:33PM +0100, Bartosz Golaszewski wrote: > On Thu, Nov 18, 2021 at 10:16 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Nov 18, 2021 at 09:12:59PM +0100, Bartosz Golaszewski wrote: > > > On Thu, Nov 18, 2021 at 6:06 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Thu, Nov 18, 2021 at 02:23:17PM +0100, Bartosz Golaszewski wrote: ... > > > > > if (gc->ngpio == 0) { > > > > > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > > > - ret = -EINVAL; > > > > > - goto err_free_descs; > > > > > + ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > > > > + if (ret) { > > > > > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > > > + ret = -EINVAL; > > > > > + goto err_free_descs; > > > > > + } > > > > > + > > > > > + gc->ngpio = ngpios; > > > > > } > > > > > > > > This should be > > > > > > > > if (gc->ngpio == 0) { > > > > ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios); > > > > if (ret) > > > > return ret; > > > > > > But device_property_read_u32() returning -ENODATA means there's no > > > such property, which should actually be converted to -EINVAL as the > > > caller wanting to create the chip provided invalid configuration - in > > > this case: a chip with 0 lines. In case of the non-array variant of > > > read_u32 that's also the only error that can be returned so this bit > > > looks right to me. > > > > So, what is so special about -EINVAL? Why -ENODATA is not good enough which > > will exactly explain to the caller what's going on, no? > > > > Let's imagine the user sets gc->ngpio = 0 incorrectly thinking it'll > make gpiolib set it to some sane default. Then gpiochip_add_data() > returns -ENODATA (No data available). This is confusing IMO. But if we > convert it to -EINVAL, it now says "Invalid value" which points to the > wrong configuration. > > ENODATA means "device tree property is not present" in this case but > the problem is that user supplies the gpiolib with invalid > configuration. EINVAL is the right error here. Then be explicit, don't shadow other error codes from fwnode API. if (ret && ret != -ENODATA) > > > > gc->ngpio = ngpios; > > > > } > > > > > > > > if (gc->ngpio == 0) { > > > > chip_err(gc, "tried to insert a GPIO chip with zero lines\n"); > > > > ret = -EINVAL; > > > > goto err_free_descs; > > > > When the caller intended to create a chip with 0 GPIOs they will get an error > > as you wish with an error message. > > Yes, as it was before. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] gpiolib: improve coding style for local variables 2021-11-18 13:23 [PATCH v2 1/2] gpiolib: improve coding style for local variables Bartosz Golaszewski 2021-11-18 13:23 ` [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code Bartosz Golaszewski @ 2021-11-18 15:44 ` Andy Shevchenko 2021-11-19 7:57 ` Johan Hovold 2 siblings, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2021-11-18 15:44 UTC (permalink / raw) To: Bartosz Golaszewski; +Cc: Linus Walleij, linux-gpio, linux-kernel On Thu, Nov 18, 2021 at 02:23:16PM +0100, Bartosz Golaszewski wrote: > Drop unneeded whitespaces and put the variables of the same type > together for consistency with the rest of the code. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/gpio/gpiolib.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index abfbf546d159..20d63028b85c 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -594,11 +594,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, > struct lock_class_key *request_key) > { > struct fwnode_handle *fwnode = gc->parent ? dev_fwnode(gc->parent) : NULL; > - unsigned long flags; > - int ret = 0; > - unsigned i; > - int base = gc->base; > + int ret = 0, base = gc->base; > struct gpio_device *gdev; > + unsigned long flags; > + unsigned int i; > > /* > * First: allocate and populate the internal stat container, and > -- > 2.25.1 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] gpiolib: improve coding style for local variables 2021-11-18 13:23 [PATCH v2 1/2] gpiolib: improve coding style for local variables Bartosz Golaszewski 2021-11-18 13:23 ` [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code Bartosz Golaszewski 2021-11-18 15:44 ` [PATCH v2 1/2] gpiolib: improve coding style for local variables Andy Shevchenko @ 2021-11-19 7:57 ` Johan Hovold 2 siblings, 0 replies; 13+ messages in thread From: Johan Hovold @ 2021-11-19 7:57 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, Andy Shevchenko, linux-gpio, linux-kernel On Thu, Nov 18, 2021 at 02:23:16PM +0100, Bartosz Golaszewski wrote: > Drop unneeded whitespaces and put the variables of the same type > together for consistency with the rest of the code. > > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/gpio/gpiolib.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index abfbf546d159..20d63028b85c 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -594,11 +594,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, > struct lock_class_key *request_key) > { > struct fwnode_handle *fwnode = gc->parent ? dev_fwnode(gc->parent) : NULL; > - unsigned long flags; > - int ret = 0; > - unsigned i; > - int base = gc->base; > + int ret = 0, base = gc->base; This only makes the code harder to read for no good reason. Keep declarations on separate lines, especially if also initialising. Note that all but one function in this file initialises return value variables on their own lines too (as they should). > struct gpio_device *gdev; > + unsigned long flags; > + unsigned int i; > > /* > * First: allocate and populate the internal stat container, and Johan ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-11-22 11:20 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-18 13:23 [PATCH v2 1/2] gpiolib: improve coding style for local variables Bartosz Golaszewski 2021-11-18 13:23 ` [PATCH v2 2/2] gpiolib: check the 'ngpios' property in core gpiolib code Bartosz Golaszewski 2021-11-18 15:44 ` Andy Shevchenko 2021-11-18 16:38 ` Bartosz Golaszewski 2021-11-18 17:01 ` Andy Shevchenko 2021-11-18 17:06 ` Andy Shevchenko 2021-11-18 17:05 ` Andy Shevchenko 2021-11-18 20:12 ` Bartosz Golaszewski 2021-11-18 21:16 ` Andy Shevchenko 2021-11-19 19:35 ` Bartosz Golaszewski 2021-11-22 11:20 ` Andy Shevchenko 2021-11-18 15:44 ` [PATCH v2 1/2] gpiolib: improve coding style for local variables Andy Shevchenko 2021-11-19 7:57 ` Johan Hovold
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).