2023-05-27 11:51:33

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/2] gpiolib: Consolidate the allocated mask freeing APIs

There is a common API to allocate a mask, but more than one duplicative
counterparts. Consolidate the latter into a single common API beneath.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/gpio/gpiolib.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e62db4fc85cc..6c8580d2454d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -466,6 +466,12 @@ static unsigned long *gpiochip_allocate_mask(struct gpio_chip *gc)
return p;
}

+static void gpiochip_free_mask(unsigned long **p)
+{
+ bitmap_free(*p);
+ *p = NULL;
+}
+
static unsigned int gpiochip_count_reserved_ranges(struct gpio_chip *gc)
{
struct device *dev = &gc->gpiodev->dev;
@@ -545,8 +551,7 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gc)

static void gpiochip_free_valid_mask(struct gpio_chip *gc)
{
- bitmap_free(gc->valid_mask);
- gc->valid_mask = NULL;
+ gpiochip_free_mask(&gc->valid_mask);
}

static int gpiochip_add_pin_ranges(struct gpio_chip *gc)
@@ -1090,8 +1095,7 @@ static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gc)

static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gc)
{
- bitmap_free(gc->irq.valid_mask);
- gc->irq.valid_mask = NULL;
+ gpiochip_free_mask(&gc->irq.valid_mask);
}

bool gpiochip_irqchip_irq_valid(const struct gpio_chip *gc,
--
2.40.0.1.gaa8946217a0b



2023-05-27 11:51:33

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/2] gpiolib: Unify allocation and initialization of GPIO valid mask

Now that the of_gpiochip_add() doesn't use valid mask, we may
unify GPIO valid mask allocation and initialization. With this
it makes a symmetry to the similar which we done for IRQ chip.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/gpio/gpiolib.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6c8580d2454d..02bfe0f89871 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -485,18 +485,6 @@ static unsigned int gpiochip_count_reserved_ranges(struct gpio_chip *gc)
return 0;
}

-static int gpiochip_alloc_valid_mask(struct gpio_chip *gc)
-{
- if (!(gpiochip_count_reserved_ranges(gc) || gc->init_valid_mask))
- return 0;
-
- gc->valid_mask = gpiochip_allocate_mask(gc);
- if (!gc->valid_mask)
- return -ENOMEM;
-
- return 0;
-}
-
static int gpiochip_apply_reserved_ranges(struct gpio_chip *gc)
{
struct device *dev = &gc->gpiodev->dev;
@@ -537,6 +525,13 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gc)
{
int ret;

+ if (!(gpiochip_count_reserved_ranges(gc) || gc->init_valid_mask))
+ return 0;
+
+ gc->valid_mask = gpiochip_allocate_mask(gc);
+ if (!gc->valid_mask)
+ return -ENOMEM;
+
ret = gpiochip_apply_reserved_ranges(gc);
if (ret)
return ret;
@@ -863,7 +858,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (ret)
goto err_remove_from_list;

- ret = gpiochip_alloc_valid_mask(gc);
+ ret = gpiochip_init_valid_mask(gc);
if (ret)
goto err_remove_from_list;

@@ -871,10 +866,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (ret)
goto err_free_gpiochip_mask;

- ret = gpiochip_init_valid_mask(gc);
- if (ret)
- goto err_remove_of_chip;
-
for (i = 0; i < gc->ngpio; i++) {
struct gpio_desc *desc = &gdev->descs[i];

--
2.40.0.1.gaa8946217a0b


2023-05-30 12:04:34

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] gpiolib: Unify allocation and initialization of GPIO valid mask

On Sat, May 27, 2023 at 1:41 PM Andy Shevchenko
<[email protected]> wrote:

> Now that the of_gpiochip_add() doesn't use valid mask, we may
> unify GPIO valid mask allocation and initialization. With this
> it makes a symmetry to the similar which we done for IRQ chip.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

Neat!
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2023-05-30 12:04:34

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] gpiolib: Consolidate the allocated mask freeing APIs

On Sat, May 27, 2023 at 1:41 PM Andy Shevchenko
<[email protected]> wrote:

> There is a common API to allocate a mask, but more than one duplicative
> counterparts. Consolidate the latter into a single common API beneath.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

Microoptimization, but I'm game for this one because it has a good
descriptive name making the code more readable.
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2023-06-01 08:15:09

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] gpiolib: Consolidate the allocated mask freeing APIs

On Sat, May 27, 2023 at 1:40 PM Andy Shevchenko
<[email protected]> wrote:
>
> There is a common API to allocate a mask, but more than one duplicative
> counterparts. Consolidate the latter into a single common API beneath.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/gpio/gpiolib.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e62db4fc85cc..6c8580d2454d 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -466,6 +466,12 @@ static unsigned long *gpiochip_allocate_mask(struct gpio_chip *gc)
> return p;
> }
>
> +static void gpiochip_free_mask(unsigned long **p)
> +{
> + bitmap_free(*p);
> + *p = NULL;
> +}
> +
> static unsigned int gpiochip_count_reserved_ranges(struct gpio_chip *gc)
> {
> struct device *dev = &gc->gpiodev->dev;
> @@ -545,8 +551,7 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gc)
>
> static void gpiochip_free_valid_mask(struct gpio_chip *gc)
> {
> - bitmap_free(gc->valid_mask);
> - gc->valid_mask = NULL;
> + gpiochip_free_mask(&gc->valid_mask);
> }
>
> static int gpiochip_add_pin_ranges(struct gpio_chip *gc)
> @@ -1090,8 +1095,7 @@ static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gc)
>
> static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gc)
> {
> - bitmap_free(gc->irq.valid_mask);
> - gc->irq.valid_mask = NULL;
> + gpiochip_free_mask(&gc->irq.valid_mask);
> }
>
> bool gpiochip_irqchip_irq_valid(const struct gpio_chip *gc,
> --
> 2.40.0.1.gaa8946217a0b
>

Both applied, thanks!

Bart