2021-05-25 18:51:19

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/2] gpiolib: Split fastpath array to two

Split fastpath array to two, i.e. for mask and for bits.
At the same time declare them as bitmaps.

This makes code better to read and gives a clue about use of
bitmap API.

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

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 97a69362a584..79df075f8b82 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2540,21 +2540,24 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,

while (i < array_size) {
struct gpio_chip *gc = desc_array[i]->gdev->chip;
- unsigned long fastpath[2 * BITS_TO_LONGS(FASTPATH_NGPIO)];
+ DECLARE_BITMAP(fastpath_mask, FASTPATH_NGPIO);
+ DECLARE_BITMAP(fastpath_bits, FASTPATH_NGPIO);
unsigned long *mask, *bits;
int first, j;

if (likely(gc->ngpio <= FASTPATH_NGPIO)) {
- mask = fastpath;
+ mask = fastpath_mask;
+ bits = fastpath_bits;
} else {
mask = kmalloc_array(2 * BITS_TO_LONGS(gc->ngpio),
sizeof(*mask),
can_sleep ? GFP_KERNEL : GFP_ATOMIC);
if (!mask)
return -ENOMEM;
+
+ bits = mask + BITS_TO_LONGS(gc->ngpio);
}

- bits = mask + BITS_TO_LONGS(gc->ngpio);
bitmap_zero(mask, gc->ngpio);

if (!can_sleep)
@@ -2577,7 +2580,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,

ret = gpio_chip_get_multiple(gc, mask, bits);
if (ret) {
- if (mask != fastpath)
+ if (mask != fastpath_mask)
kfree(mask);
return ret;
}
@@ -2598,7 +2601,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
j);
}

- if (mask != fastpath)
+ if (mask != fastpath_mask)
kfree(mask);
}
return 0;
@@ -2823,21 +2826,24 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,

while (i < array_size) {
struct gpio_chip *gc = desc_array[i]->gdev->chip;
- unsigned long fastpath[2 * BITS_TO_LONGS(FASTPATH_NGPIO)];
+ DECLARE_BITMAP(fastpath_mask, FASTPATH_NGPIO);
+ DECLARE_BITMAP(fastpath_bits, FASTPATH_NGPIO);
unsigned long *mask, *bits;
int count = 0;

if (likely(gc->ngpio <= FASTPATH_NGPIO)) {
- mask = fastpath;
+ mask = fastpath_mask;
+ bits = fastpath_bits;
} else {
mask = kmalloc_array(2 * BITS_TO_LONGS(gc->ngpio),
sizeof(*mask),
can_sleep ? GFP_KERNEL : GFP_ATOMIC);
if (!mask)
return -ENOMEM;
+
+ bits = mask + BITS_TO_LONGS(gc->ngpio);
}

- bits = mask + BITS_TO_LONGS(gc->ngpio);
bitmap_zero(mask, gc->ngpio);

if (!can_sleep)
@@ -2882,7 +2888,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
if (count != 0)
gpio_chip_set_multiple(gc, mask, bits);

- if (mask != fastpath)
+ if (mask != fastpath_mask)
kfree(mask);
}
return 0;
--
2.30.2


2021-05-25 21:49:39

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/2] gpiolib: Switch to bitmap_alloc()

Switch to bitmap_alloc() to show clearly what we are allocating.
Besides that it returns pointer of bitmap type instead of opaque void *.

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

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 79df075f8b82..068f18624da0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2549,13 +2549,17 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
mask = fastpath_mask;
bits = fastpath_bits;
} else {
- mask = kmalloc_array(2 * BITS_TO_LONGS(gc->ngpio),
- sizeof(*mask),
- can_sleep ? GFP_KERNEL : GFP_ATOMIC);
+ gfp_t flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
+
+ mask = bitmap_alloc(gc->ngpio, flags);
if (!mask)
return -ENOMEM;

- bits = mask + BITS_TO_LONGS(gc->ngpio);
+ bits = bitmap_alloc(gc->ngpio, flags);
+ if (!bits) {
+ bitmap_free(mask);
+ return -ENOMEM;
+ }
}

bitmap_zero(mask, gc->ngpio);
@@ -2581,7 +2585,9 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
ret = gpio_chip_get_multiple(gc, mask, bits);
if (ret) {
if (mask != fastpath_mask)
- kfree(mask);
+ bitmap_free(mask);
+ if (bits != fastpath_bits)
+ bitmap_free(bits);
return ret;
}

@@ -2602,7 +2608,9 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
}

if (mask != fastpath_mask)
- kfree(mask);
+ bitmap_free(mask);
+ if (bits != fastpath_bits)
+ bitmap_free(bits);
}
return 0;
}
@@ -2835,13 +2843,17 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
mask = fastpath_mask;
bits = fastpath_bits;
} else {
- mask = kmalloc_array(2 * BITS_TO_LONGS(gc->ngpio),
- sizeof(*mask),
- can_sleep ? GFP_KERNEL : GFP_ATOMIC);
+ gfp_t flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
+
+ mask = bitmap_alloc(gc->ngpio, flags);
if (!mask)
return -ENOMEM;

- bits = mask + BITS_TO_LONGS(gc->ngpio);
+ bits = bitmap_alloc(gc->ngpio, flags);
+ if (!bits) {
+ bitmap_free(mask);
+ return -ENOMEM;
+ }
}

bitmap_zero(mask, gc->ngpio);
@@ -2889,7 +2901,9 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
gpio_chip_set_multiple(gc, mask, bits);

if (mask != fastpath_mask)
- kfree(mask);
+ bitmap_free(mask);
+ if (bits != fastpath_bits)
+ bitmap_free(bits);
}
return 0;
}
--
2.30.2

2021-05-28 03:49:42

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] gpiolib: Switch to bitmap_alloc()

On Tue, May 25, 2021 at 8:35 PM Andy Shevchenko
<[email protected]> wrote:

> Switch to bitmap_alloc() to show clearly what we are allocating.
> Besides that it returns pointer of bitmap type instead of opaque void *.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

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

Yours,
Linus Walleij

2021-05-28 03:58:27

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] gpiolib: Split fastpath array to two

On Tue, May 25, 2021 at 8:35 PM Andy Shevchenko
<[email protected]> wrote:

> Split fastpath array to two, i.e. for mask and for bits.
> At the same time declare them as bitmaps.
>
> This makes code better to read and gives a clue about use of
> bitmap API.
>
> Signed-off-by: Andy Shevchenko <[email protected]>

Excellent code!

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

Yours,
Linus Walleij

2021-05-28 14:35:46

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] gpiolib: Switch to bitmap_alloc()

On Tue, May 25, 2021 at 8:35 PM Andy Shevchenko
<[email protected]> wrote:
>
> Switch to bitmap_alloc() to show clearly what we are allocating.
> Besides that it returns pointer of bitmap type instead of opaque void *.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---

Applied, thanks!

Bart

2021-05-28 14:36:06

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] gpiolib: Split fastpath array to two

On Tue, May 25, 2021 at 8:35 PM Andy Shevchenko
<[email protected]> wrote:
>
> Split fastpath array to two, i.e. for mask and for bits.
> At the same time declare them as bitmaps.
>
> This makes code better to read and gives a clue about use of
> bitmap API.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---

Applied, thanks!

Bart