gpiolib: Split out for_each_gpio_desc() macro
PATCH 1/3 gpiolib: Split out for_each_gpio_desc() macro
In some cases we want to traverse all GPIO descriptors for given chip, let's split out for_each_gpio_desc() macro for such cases.
Signed-off-by Andy Shevchenko <[email protected]>
--- drivers/gpio/gpiolib.c | 10 +++------- drivers/gpio/gpiolib.h | 7 +++++-- 2 files changed, 8 insertions(+), 9 deletions(-)
drivers/gpio/gpiolib.c +3 -7
@@ -310,15 +310,11 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)spin_lock_irqsave (&gpio_lock, flags);list_for_each_entry (gdev, &gpio_devices, list) { +struct gpio_desc *desc;int i; -for (i =0 ; i != gdev->ngpio; ++i) { -struct gpio_desc *desc = &gdev->descs[i]; - -if (!desc->name) -continue ; - -if (!strcmp (desc->name, name)) { +for_each_gpio_desc (i, gdev->chip, desc) { +if (desc->name && !strcmp (desc->name, name)) {spin_unlock_irqrestore (&gpio_lock, flags);return desc; }
drivers/gpio/gpiolib.h +5 -2
@@ -100,10 +100,13 @@ struct gpio_array {struct gpio_desc *gpiochip_get_desc (struct gpio_chip *gc,unsigned int hwnum); -#define for_each_gpio_desc_with_flag (i, gc, desc, flag) \ +#define for_each_gpio_desc (i, gc, desc) \ for (i = 0, desc = gpiochip_get_desc(gc, i); \ i < gc->ngpio; \ - i++, desc = gpiochip_get_desc(gc, i)) \ + i++, desc = gpiochip_get_desc(gc, i)) + +#define for_each_gpio_desc_with_flag (i, gc, desc, flag) \ + for_each_gpio_desc(i, gc, desc) \ if (!test_bit(flag, &desc->flags)) {} else int gpiod_get_array_value_complex(bool raw, bool can_sleep,
Signature · 1 line
2.35.1
PATCH 2/3 gpiolib: Refactor gpiolib_dbg_show() with help of for_each_gpio_desc()
Use for_each_gpio_desc() and since we would need to touch the entire conditionals, do the following:
- rename last occurrence of gdesc to desc
- use short ternary operator ?:
- join two seq_printf() calls into single one
- fix indentation of the seq_printf() parameters
Signed-off-by Andy Shevchenko <[email protected]>
--- drivers/gpio/gpiolib.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-)
drivers/gpio/gpiolib.c +16 -19
@@ -4381,32 +4381,29 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev) {unsigned i;struct gpio_chip *gc = gdev->chip; +struct gpio_desc *desc;unsigned gpio = gdev->base; -struct gpio_desc *gdesc = &gdev->descs[0 ];bool is_out;bool is_irq;bool active_low; -for (i =0 ; i < gdev->ngpio; i++, gpio++, gdesc++) { -if (!test_bit (FLAG_REQUESTED , &gdesc->flags)) { -if (gdesc->name) { -seq_printf (s," gpio-%-3d (%-20.20s)\n" , - gpio, gdesc->name); - } -continue ; +for_each_gpio_desc (i, gc, desc) { +if (test_bit (FLAG_REQUESTED , &desc->flags)) { +gpiod_get_direction (desc); + is_out =test_bit (FLAG_IS_OUT , &desc->flags); + is_irq =test_bit (FLAG_USED_AS_IRQ , &desc->flags); + active_low =test_bit (FLAG_ACTIVE_LOW , &desc->flags); +seq_printf (s," gpio-%-3d (%-20.20s|%-20.20s) %s %s %s%s\n" , + gpio, desc->name ?:"" , desc->label, + is_out ?"out" :"in " , + gc->get ? (gc->get (gc, i) ?"hi" :"lo" ) :"? " , + is_irq ?"IRQ " :"" , + active_low ?"ACTIVE LOW" :"" ); + }else if (desc->name) { +seq_printf (s," gpio-%-3d (%-20.20s)\n" , gpio, desc->name); } -gpiod_get_direction (gdesc); - is_out =test_bit (FLAG_IS_OUT , &gdesc->flags); - is_irq =test_bit (FLAG_USED_AS_IRQ , &gdesc->flags); - active_low =test_bit (FLAG_ACTIVE_LOW , &gdesc->flags); -seq_printf (s," gpio-%-3d (%-20.20s|%-20.20s) %s %s %s%s" , - gpio, gdesc->name ? gdesc->name :"" , gdesc->label, - is_out ?"out" :"in " , - gc->get ? (gc->get (gc, i) ?"hi" :"lo" ) :"? " , - is_irq ?"IRQ " :"" , - active_low ?"ACTIVE LOW" :"" ); -seq_printf (s,"\n" ); + gpio++; } }
Signature · 1 line
2.35.1
PATCH 3/3 gpiolib: Move error message out of a spinlock
In reply to Andy ShevchenkoAn error path is a slow path, no need to block other CPUs when printing error messages.
Signed-off-by Andy Shevchenko <[email protected]>
--- drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpiolib.c +1 -1
@@ -289,7 +289,6 @@ static int gpiodev_add_to_list(struct gpio_device *gdev) } } -dev_err (&gdev->dev,"GPIO integer space overlap, cannot add chip\n" );return -EBUSY ; }
@@ -724,6 +723,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, ret =gpiodev_add_to_list (gdev);if (ret) {spin_unlock_irqrestore (&gpio_lock, flags); +chip_err (gc,"GPIO integer space overlap, cannot add chip\n" ); goto err_free_label; }
Signature · 1 line
2.35.1
Bartosz Golaszewski
AppliedRe: [PATCH v1 3/3] gpiolib: Move error message out of a spinlock
Quoted 30 lines · Andy Shevchenko
An error path is a slow path, no need to block other CPUs when printing error messages.
Signed-off-by Andy Shevchenko <[email protected]>--- drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpio/gpiolib.c +1
-2@@ -289,7 +289,6 @@ static int gpiodev_add_to_list(struct gpio_device *gdev) } } -dev_err (&gdev->dev,"GPIO integer space overlap, cannot add chip\n" );return -EBUSY ; }@@ -724,6 +723,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, ret =gpiodev_add_to_list (gdev);if (ret) {spin_unlock_irqrestore (&gpio_lock, flags); +chip_err (gc,"GPIO integer space overlap, cannot add chip\n" ); goto err_free_label; } --
Quoted 2 lines
2.35.1
Applied, thanks!
Bart
Quoted 38 lines · Andy Shevchenko
In some cases we want to traverse all GPIO descriptors for given chip, let's split out
for_each_gpio_desc()macro for such cases.Signed-off-by Andy Shevchenko <[email protected]>--- drivers/gpio/gpiolib.c | 10 +++------- drivers/gpio/gpiolib.h | 7 +++++-- 2 files changed, 8 insertions(+), 9 deletions(-)drivers/gpio/gpiolib.c +3
-7@@ -310,15 +310,11 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)spin_lock_irqsave (&gpio_lock, flags);list_for_each_entry (gdev, &gpio_devices, list) { +struct gpio_desc *desc;int i; -for (i =0 ; i != gdev->ngpio; ++i) { -struct gpio_desc *desc = &gdev->descs[i]; - -if (!desc->name) -continue ; - -if (!strcmp (desc->name, name)) { +for_each_gpio_desc (i, gdev->chip, desc) { +if (desc->name && !strcmp (desc->name, name)) {spin_unlock_irqrestore (&gpio_lock, flags);return desc; }drivers/gpio/gpiolib.h +0
-0
drivers/gpio/gpiolib.h struct gpio_array { · line 103
Quoted 5 lines
drivers/gpio/gpiolib.h +1
-1@@ -100,10 +100,13 @@ struct gpio_array {struct gpio_desc *gpiochip_get_desc (struct gpio_chip *gc,unsigned int hwnum); -#define for_each_gpio_desc_with_flag (i, gc, desc, flag) \ +#define for_each_gpio_desc (i, gc, desc) \
While at it: how about declaring the i variable in the loop definition?
Bart
Quoted 13 lines
drivers/gpio/gpiolib.h +4
-2for (i =0 , desc =gpiochip_get_desc (gc, i); \ i < gc->ngpio; \ - i++, desc =gpiochip_get_desc (gc, i)) \ + i++, desc = gpiochip_get_desc(gc, i)) + +#define for_each_gpio_desc_with_flag (i, gc, desc, flag) \ + for_each_gpio_desc(i, gc, desc) \ if (!test_bit(flag, &desc->flags)) {} else int gpiod_get_array_value_complex(bool raw, bool can_sleep, --2.35.1
Quoted 2 lines · Bartosz Golaszewski
On Wed, Mar 30, 2022 at 4:59 PM Andy Shevchenko
<[email protected]> wrote:
...
Quoted 4 lines
drivers/gpio/gpiolib.h struct gpio_array { · line 103
Quoted 2 lines
drivers/gpio/gpiolib.h +1
-1-#define for_each_gpio_desc_with_flag (i, gc, desc, flag) \ +#define for_each_gpio_desc (i, gc, desc) \While at it: how about declaring the i variable in the loop definition?
It will require changes in the users, so, I will do it as a prerequisite separate change.
Quoted 4 lines
Quoted 4 lines
for (i = 0, desc = gpiochip_get_desc(gc, i); \ i < gc->ngpio; \drivers/gpio/gpiolib.h +1
-1- i++, desc = gpiochip_get_desc(gc, i)) \ + i++, desc = gpiochip_get_desc(gc, i))
Signature · 2 lines
With Best Regards, Andy Shevchenko