* [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
@ 2023-07-11 15:12 Asmaa Mnebhi
2023-07-11 16:15 ` Andy Shevchenko
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Asmaa Mnebhi @ 2023-07-11 15:12 UTC (permalink / raw)
To: andy.shevchenko, linus.walleij, bgolaszewski, brgl, linux-gpio,
linux-kernel
Cc: Asmaa Mnebhi, davthompson
bgpio_init() uses "sz" argument to populate ngpio, which is not
accurate. Instead, read the "ngpios" property from the DT and if it
doesn't exist, use the "sz" argument. With this change, drivers no
longer need to overwrite the ngpio variable after calling bgpio_init().
If the "ngpios" property is specified, bgpio_bits is calculated
as the round up value of ngpio. At the moment, the only requirement
specified is that the round up value must be a multiple of 8 but
it should also be a power of 2 because we provide accessors based
on the bank size in bgpio_setup_accessors().
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
The following 2 patches were approved in March 2023 but didn't make
it into the tree:
[PATCH v4] gpio: mmio: handle "ngpios" properly in bgpio_init()
[PATCH v1] gpio: mmio: fix calculation of bgpio_bits
They needed a rebase and were combined into a single patch since
"gpio: mmio: fix calculation of bgpio_bits" fixes a bug in
"gpio: mmio: handle "ngpios" properly in bgpio_init()"
v1->v2:
- Added the tags
- Updated the changelog
drivers/gpio/gpio-mmio.c | 9 +++++-
drivers/gpio/gpiolib.c | 68 ++++++++++++++++++++++------------------
drivers/gpio/gpiolib.h | 1 +
3 files changed, 46 insertions(+), 32 deletions(-)
diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index d9dff3dc92ae..74fdf0d87b2c 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -60,6 +60,8 @@ o ` ~~~~\___/~~~~ ` controller in FPGA is ,.`
#include <linux/of.h>
#include <linux/of_device.h>
+#include "gpiolib.h"
+
static void bgpio_write8(void __iomem *reg, unsigned long data)
{
writeb(data, reg);
@@ -614,10 +616,15 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
gc->parent = dev;
gc->label = dev_name(dev);
gc->base = -1;
- gc->ngpio = gc->bgpio_bits;
gc->request = bgpio_request;
gc->be_bits = !!(flags & BGPIOF_BIG_ENDIAN);
+ ret = gpiochip_get_ngpios(gc, dev);
+ if (ret)
+ gc->ngpio = gc->bgpio_bits;
+ else
+ gc->bgpio_bits = roundup_pow_of_two(round_up(gc->ngpio, 8));
+
ret = bgpio_setup_io(gc, dat, set, clr, flags);
if (ret)
return ret;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 251c875b5c34..7dac8bb9905a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -700,6 +700,40 @@ void *gpiochip_get_data(struct gpio_chip *gc)
}
EXPORT_SYMBOL_GPL(gpiochip_get_data);
+int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev)
+{
+ u32 ngpios = gc->ngpio;
+ int ret;
+
+ if (ngpios == 0) {
+ ret = device_property_read_u32(dev, "ngpios", &ngpios);
+ if (ret == -ENODATA)
+ /*
+ * -ENODATA means that there is no property found and
+ * we want to issue the error message to the user.
+ * Besides that, we want to return different error code
+ * to state that supplied value is not valid.
+ */
+ ngpios = 0;
+ else if (ret)
+ return ret;
+
+ gc->ngpio = ngpios;
+ }
+
+ if (gc->ngpio == 0) {
+ chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
+ return -EINVAL;
+ }
+
+ if (gc->ngpio > FASTPATH_NGPIO)
+ chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
+ gc->ngpio, FASTPATH_NGPIO);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(gpiochip_get_ngpios);
+
int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
struct lock_class_key *lock_key,
struct lock_class_key *request_key)
@@ -707,7 +741,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
struct gpio_device *gdev;
unsigned long flags;
unsigned int i;
- u32 ngpios = 0;
int base = 0;
int ret = 0;
@@ -753,36 +786,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
else
gdev->owner = THIS_MODULE;
- /*
- * Try the device properties if the driver didn't supply the number
- * of GPIO lines.
- */
- ngpios = gc->ngpio;
- if (ngpios == 0) {
- ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios);
- if (ret == -ENODATA)
- /*
- * -ENODATA means that there is no property found and
- * we want to issue the error message to the user.
- * Besides that, we want to return different error code
- * to state that supplied value is not valid.
- */
- ngpios = 0;
- else if (ret)
- goto err_free_dev_name;
-
- gc->ngpio = ngpios;
- }
-
- if (gc->ngpio == 0) {
- chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
- ret = -EINVAL;
+ ret = gpiochip_get_ngpios(gc, &gdev->dev);
+ if (ret)
goto err_free_dev_name;
- }
-
- if (gc->ngpio > FASTPATH_NGPIO)
- chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
- gc->ngpio, FASTPATH_NGPIO);
gdev->descs = kcalloc(gc->ngpio, sizeof(*gdev->descs), GFP_KERNEL);
if (!gdev->descs) {
@@ -947,7 +953,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
/* failures here can mean systems won't boot... */
if (ret != -EPROBE_DEFER) {
pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", __func__,
- base, base + (int)ngpios - 1,
+ base, base + (int)gc->ngpio - 1,
gc->label ? : "generic", ret);
}
return ret;
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index cca81375f127..8de748a16d13 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -217,6 +217,7 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce);
int gpiod_hog(struct gpio_desc *desc, const char *name,
unsigned long lflags, enum gpiod_flags dflags);
+int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev);
/*
* Return the GPIO number of the passed descriptor relative to its chip
--
2.30.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
2023-07-11 15:12 [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init() Asmaa Mnebhi
@ 2023-07-11 16:15 ` Andy Shevchenko
2023-07-11 17:10 ` Asmaa Mnebhi
2023-07-16 7:41 ` Linus Walleij
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2023-07-11 16:15 UTC (permalink / raw)
To: Asmaa Mnebhi
Cc: linus.walleij, bgolaszewski, brgl, linux-gpio, linux-kernel,
davthompson
On Tue, Jul 11, 2023 at 6:13 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> bgpio_init() uses "sz" argument to populate ngpio, which is not
> accurate. Instead, read the "ngpios" property from the DT and if it
> doesn't exist, use the "sz" argument. With this change, drivers no
> longer need to overwrite the ngpio variable after calling bgpio_init().
>
> If the "ngpios" property is specified, bgpio_bits is calculated
> as the round up value of ngpio. At the moment, the only requirement
> specified is that the round up value must be a multiple of 8 but
> it should also be a power of 2 because we provide accessors based
> on the bank size in bgpio_setup_accessors().
>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> The following 2 patches were approved in March 2023 but didn't make
> it into the tree:
> [PATCH v4] gpio: mmio: handle "ngpios" properly in bgpio_init()
> [PATCH v1] gpio: mmio: fix calculation of bgpio_bits
>
> They needed a rebase and were combined into a single patch since
> "gpio: mmio: fix calculation of bgpio_bits" fixes a bug in
> "gpio: mmio: handle "ngpios" properly in bgpio_init()"
And hence Linus' tag had been dropped.
LGTM now, thank you for pursuing this!
I hope Linus can review it again and Bart will be okay with the result
to be applied.
> v1->v2:
> - Added the tags
> - Updated the changelog
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
2023-07-11 16:15 ` Andy Shevchenko
@ 2023-07-11 17:10 ` Asmaa Mnebhi
0 siblings, 0 replies; 8+ messages in thread
From: Asmaa Mnebhi @ 2023-07-11 17:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linus.walleij@linaro.org, bgolaszewski@baylibre.com,
brgl@bgdev.pl, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org, David Thompson
> > They needed a rebase and were combined into a single patch since
> > "gpio: mmio: fix calculation of bgpio_bits" fixes a bug in
> > "gpio: mmio: handle "ngpios" properly in bgpio_init()"
>
> And hence Linus' tag had been dropped.
>
> LGTM now, thank you for pursuing this!
> I hope Linus can review it again and Bart will be okay with the result to be
> applied.
Thank you Andy!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
2023-07-11 15:12 [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init() Asmaa Mnebhi
2023-07-11 16:15 ` Andy Shevchenko
@ 2023-07-16 7:41 ` Linus Walleij
2023-07-18 18:20 ` Bartosz Golaszewski
2023-07-18 18:38 ` Asmaa Mnebhi
3 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2023-07-16 7:41 UTC (permalink / raw)
To: Asmaa Mnebhi
Cc: andy.shevchenko, bgolaszewski, brgl, linux-gpio, linux-kernel,
davthompson
On Tue, Jul 11, 2023 at 5:13 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
> bgpio_init() uses "sz" argument to populate ngpio, which is not
> accurate. Instead, read the "ngpios" property from the DT and if it
> doesn't exist, use the "sz" argument. With this change, drivers no
> longer need to overwrite the ngpio variable after calling bgpio_init().
>
> If the "ngpios" property is specified, bgpio_bits is calculated
> as the round up value of ngpio. At the moment, the only requirement
> specified is that the round up value must be a multiple of 8 but
> it should also be a power of 2 because we provide accessors based
> on the bank size in bgpio_setup_accessors().
>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Still looks good! :)
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
2023-07-11 15:12 [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init() Asmaa Mnebhi
2023-07-11 16:15 ` Andy Shevchenko
2023-07-16 7:41 ` Linus Walleij
@ 2023-07-18 18:20 ` Bartosz Golaszewski
2023-07-18 18:34 ` Andy Shevchenko
2023-07-18 18:38 ` Asmaa Mnebhi
3 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2023-07-18 18:20 UTC (permalink / raw)
To: Asmaa Mnebhi
Cc: andy.shevchenko, linus.walleij, bgolaszewski, linux-gpio,
linux-kernel, davthompson
On Tue, Jul 11, 2023 at 5:13 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> bgpio_init() uses "sz" argument to populate ngpio, which is not
> accurate. Instead, read the "ngpios" property from the DT and if it
> doesn't exist, use the "sz" argument. With this change, drivers no
> longer need to overwrite the ngpio variable after calling bgpio_init().
>
> If the "ngpios" property is specified, bgpio_bits is calculated
> as the round up value of ngpio. At the moment, the only requirement
> specified is that the round up value must be a multiple of 8 but
> it should also be a power of 2 because we provide accessors based
> on the bank size in bgpio_setup_accessors().
>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
I'm confused. Is this the final version after all?
Bart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
2023-07-18 18:20 ` Bartosz Golaszewski
@ 2023-07-18 18:34 ` Andy Shevchenko
0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2023-07-18 18:34 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Asmaa Mnebhi, linus.walleij, bgolaszewski, linux-gpio,
linux-kernel, davthompson
On Tue, Jul 18, 2023 at 9:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Tue, Jul 11, 2023 at 5:13 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
...
> I'm confused. Is this the final version after all?
Seems to me so.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
2023-07-11 15:12 [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init() Asmaa Mnebhi
` (2 preceding siblings ...)
2023-07-18 18:20 ` Bartosz Golaszewski
@ 2023-07-18 18:38 ` Asmaa Mnebhi
2023-07-18 19:00 ` Bartosz Golaszewski
3 siblings, 1 reply; 8+ messages in thread
From: Asmaa Mnebhi @ 2023-07-18 18:38 UTC (permalink / raw)
To: andy.shevchenko@gmail.com, linus.walleij@linaro.org,
bgolaszewski@baylibre.com, brgl@bgdev.pl,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: David Thompson
> bgpio_init() uses "sz" argument to populate ngpio, which is not accurate.
> Instead, read the "ngpios" property from the DT and if it doesn't exist, use the
> "sz" argument. With this change, drivers no longer need to overwrite the ngpio
> variable after calling bgpio_init().
>
> If the "ngpios" property is specified, bgpio_bits is calculated as the round up
> value of ngpio. At the moment, the only requirement specified is that the round
> up value must be a multiple of 8 but it should also be a power of 2 because we
> provide accessors based on the bank size in bgpio_setup_accessors().
>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> The following 2 patches were approved in March 2023 but didn't make it into
> the tree:
> [PATCH v4] gpio: mmio: handle "ngpios" properly in bgpio_init() [PATCH v1]
> gpio: mmio: fix calculation of bgpio_bits
>
> They needed a rebase and were combined into a single patch since
> "gpio: mmio: fix calculation of bgpio_bits" fixes a bug in
> "gpio: mmio: handle "ngpios" properly in bgpio_init()"
>
> v1->v2:
> - Added the tags
> - Updated the changelog
>
> drivers/gpio/gpio-mmio.c | 9 +++++-
> drivers/gpio/gpiolib.c | 68 ++++++++++++++++++++++------------------
> drivers/gpio/gpiolib.h | 1 +
> 3 files changed, 46 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c index
> d9dff3dc92ae..74fdf0d87b2c 100644
> --- a/drivers/gpio/gpio-mmio.c
> +++ b/drivers/gpio/gpio-mmio.c
> @@ -60,6 +60,8 @@ o ` ~~~~\___/~~~~ ` controller in FPGA
> is ,.`
> #include <linux/of.h>
> #include <linux/of_device.h>
>
> +#include "gpiolib.h"
> +
> static void bgpio_write8(void __iomem *reg, unsigned long data) {
> writeb(data, reg);
> @@ -614,10 +616,15 @@ int bgpio_init(struct gpio_chip *gc, struct device
> *dev,
> gc->parent = dev;
> gc->label = dev_name(dev);
> gc->base = -1;
> - gc->ngpio = gc->bgpio_bits;
> gc->request = bgpio_request;
> gc->be_bits = !!(flags & BGPIOF_BIG_ENDIAN);
>
> + ret = gpiochip_get_ngpios(gc, dev);
> + if (ret)
> + gc->ngpio = gc->bgpio_bits;
> + else
> + gc->bgpio_bits = roundup_pow_of_two(round_up(gc->ngpio,
> 8));
> +
> ret = bgpio_setup_io(gc, dat, set, clr, flags);
> if (ret)
> return ret;
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index
> 251c875b5c34..7dac8bb9905a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -700,6 +700,40 @@ void *gpiochip_get_data(struct gpio_chip *gc) }
> EXPORT_SYMBOL_GPL(gpiochip_get_data);
>
> +int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev) {
> + u32 ngpios = gc->ngpio;
> + int ret;
> +
> + if (ngpios == 0) {
> + ret = device_property_read_u32(dev, "ngpios", &ngpios);
> + if (ret == -ENODATA)
> + /*
> + * -ENODATA means that there is no property found
> and
> + * we want to issue the error message to the user.
> + * Besides that, we want to return different error code
> + * to state that supplied value is not valid.
> + */
> + ngpios = 0;
> + else if (ret)
> + return ret;
> +
> + gc->ngpio = ngpios;
> + }
> +
> + if (gc->ngpio == 0) {
> + chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
> + return -EINVAL;
> + }
> +
> + if (gc->ngpio > FASTPATH_NGPIO)
> + chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
> + gc->ngpio, FASTPATH_NGPIO);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(gpiochip_get_ngpios);
> +
> int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> struct lock_class_key *lock_key,
> struct lock_class_key *request_key) @@ -707,7
> +741,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> struct gpio_device *gdev;
> unsigned long flags;
> unsigned int i;
> - u32 ngpios = 0;
> int base = 0;
> int ret = 0;
>
> @@ -753,36 +786,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc,
> void *data,
> else
> gdev->owner = THIS_MODULE;
>
> - /*
> - * Try the device properties if the driver didn't supply the number
> - * of GPIO lines.
> - */
> - ngpios = gc->ngpio;
> - if (ngpios == 0) {
> - ret = device_property_read_u32(&gdev->dev, "ngpios",
> &ngpios);
> - if (ret == -ENODATA)
> - /*
> - * -ENODATA means that there is no property found
> and
> - * we want to issue the error message to the user.
> - * Besides that, we want to return different error code
> - * to state that supplied value is not valid.
> - */
> - ngpios = 0;
> - else if (ret)
> - goto err_free_dev_name;
> -
> - gc->ngpio = ngpios;
> - }
> -
> - if (gc->ngpio == 0) {
> - chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
> - ret = -EINVAL;
> + ret = gpiochip_get_ngpios(gc, &gdev->dev);
> + if (ret)
> goto err_free_dev_name;
> - }
> -
> - if (gc->ngpio > FASTPATH_NGPIO)
> - chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
> - gc->ngpio, FASTPATH_NGPIO);
>
> gdev->descs = kcalloc(gc->ngpio, sizeof(*gdev->descs), GFP_KERNEL);
> if (!gdev->descs) {
> @@ -947,7 +953,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc,
> void *data,
> /* failures here can mean systems won't boot... */
> if (ret != -EPROBE_DEFER) {
> pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n",
> __func__,
> - base, base + (int)ngpios - 1,
> + base, base + (int)gc->ngpio - 1,
> gc->label ? : "generic", ret);
> }
> return ret;
> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index
> cca81375f127..8de748a16d13 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -217,6 +217,7 @@ int gpiod_configure_flags(struct gpio_desc *desc, const
> char *con_id, int gpio_set_debounce_timeout(struct gpio_desc *desc,
> unsigned int debounce); int gpiod_hog(struct gpio_desc *desc, const char
> *name,
> unsigned long lflags, enum gpiod_flags dflags);
> +int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev);
>
> /*
> * Return the GPIO number of the passed descriptor relative to its chip
> --
> 2.30.1
Hi Bart,
This is the final approved patch by both Linus and Andy. Please discard all others.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
2023-07-18 18:38 ` Asmaa Mnebhi
@ 2023-07-18 19:00 ` Bartosz Golaszewski
0 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2023-07-18 19:00 UTC (permalink / raw)
To: Asmaa Mnebhi
Cc: andy.shevchenko@gmail.com, linus.walleij@linaro.org,
bgolaszewski@baylibre.com, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org, David Thompson
On Tue, Jul 18, 2023 at 8:38 PM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> > bgpio_init() uses "sz" argument to populate ngpio, which is not accurate.
> > Instead, read the "ngpios" property from the DT and if it doesn't exist, use the
> > "sz" argument. With this change, drivers no longer need to overwrite the ngpio
> > variable after calling bgpio_init().
> >
> > If the "ngpios" property is specified, bgpio_bits is calculated as the round up
> > value of ngpio. At the moment, the only requirement specified is that the round
> > up value must be a multiple of 8 but it should also be a power of 2 because we
> > provide accessors based on the bank size in bgpio_setup_accessors().
> >
> > Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > ---
> > The following 2 patches were approved in March 2023 but didn't make it into
> > the tree:
> > [PATCH v4] gpio: mmio: handle "ngpios" properly in bgpio_init() [PATCH v1]
> > gpio: mmio: fix calculation of bgpio_bits
> >
> > They needed a rebase and were combined into a single patch since
> > "gpio: mmio: fix calculation of bgpio_bits" fixes a bug in
> > "gpio: mmio: handle "ngpios" properly in bgpio_init()"
> >
> > v1->v2:
> > - Added the tags
> > - Updated the changelog
> >
> > drivers/gpio/gpio-mmio.c | 9 +++++-
> > drivers/gpio/gpiolib.c | 68 ++++++++++++++++++++++------------------
> > drivers/gpio/gpiolib.h | 1 +
> > 3 files changed, 46 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c index
> > d9dff3dc92ae..74fdf0d87b2c 100644
> > --- a/drivers/gpio/gpio-mmio.c
> > +++ b/drivers/gpio/gpio-mmio.c
> > @@ -60,6 +60,8 @@ o ` ~~~~\___/~~~~ ` controller in FPGA
> > is ,.`
> > #include <linux/of.h>
> > #include <linux/of_device.h>
> >
> > +#include "gpiolib.h"
> > +
> > static void bgpio_write8(void __iomem *reg, unsigned long data) {
> > writeb(data, reg);
> > @@ -614,10 +616,15 @@ int bgpio_init(struct gpio_chip *gc, struct device
> > *dev,
> > gc->parent = dev;
> > gc->label = dev_name(dev);
> > gc->base = -1;
> > - gc->ngpio = gc->bgpio_bits;
> > gc->request = bgpio_request;
> > gc->be_bits = !!(flags & BGPIOF_BIG_ENDIAN);
> >
> > + ret = gpiochip_get_ngpios(gc, dev);
> > + if (ret)
> > + gc->ngpio = gc->bgpio_bits;
> > + else
> > + gc->bgpio_bits = roundup_pow_of_two(round_up(gc->ngpio,
> > 8));
> > +
> > ret = bgpio_setup_io(gc, dat, set, clr, flags);
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index
> > 251c875b5c34..7dac8bb9905a 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -700,6 +700,40 @@ void *gpiochip_get_data(struct gpio_chip *gc) }
> > EXPORT_SYMBOL_GPL(gpiochip_get_data);
> >
> > +int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev) {
> > + u32 ngpios = gc->ngpio;
> > + int ret;
> > +
> > + if (ngpios == 0) {
> > + ret = device_property_read_u32(dev, "ngpios", &ngpios);
> > + if (ret == -ENODATA)
> > + /*
> > + * -ENODATA means that there is no property found
> > and
> > + * we want to issue the error message to the user.
> > + * Besides that, we want to return different error code
> > + * to state that supplied value is not valid.
> > + */
> > + ngpios = 0;
> > + else if (ret)
> > + return ret;
> > +
> > + gc->ngpio = ngpios;
> > + }
> > +
> > + if (gc->ngpio == 0) {
> > + chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (gc->ngpio > FASTPATH_NGPIO)
> > + chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
> > + gc->ngpio, FASTPATH_NGPIO);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(gpiochip_get_ngpios);
> > +
> > int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> > struct lock_class_key *lock_key,
> > struct lock_class_key *request_key) @@ -707,7
> > +741,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> > struct gpio_device *gdev;
> > unsigned long flags;
> > unsigned int i;
> > - u32 ngpios = 0;
> > int base = 0;
> > int ret = 0;
> >
> > @@ -753,36 +786,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc,
> > void *data,
> > else
> > gdev->owner = THIS_MODULE;
> >
> > - /*
> > - * Try the device properties if the driver didn't supply the number
> > - * of GPIO lines.
> > - */
> > - ngpios = gc->ngpio;
> > - if (ngpios == 0) {
> > - ret = device_property_read_u32(&gdev->dev, "ngpios",
> > &ngpios);
> > - if (ret == -ENODATA)
> > - /*
> > - * -ENODATA means that there is no property found
> > and
> > - * we want to issue the error message to the user.
> > - * Besides that, we want to return different error code
> > - * to state that supplied value is not valid.
> > - */
> > - ngpios = 0;
> > - else if (ret)
> > - goto err_free_dev_name;
> > -
> > - gc->ngpio = ngpios;
> > - }
> > -
> > - if (gc->ngpio == 0) {
> > - chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
> > - ret = -EINVAL;
> > + ret = gpiochip_get_ngpios(gc, &gdev->dev);
> > + if (ret)
> > goto err_free_dev_name;
> > - }
> > -
> > - if (gc->ngpio > FASTPATH_NGPIO)
> > - chip_warn(gc, "line cnt %u is greater than fast path cnt %u\n",
> > - gc->ngpio, FASTPATH_NGPIO);
> >
> > gdev->descs = kcalloc(gc->ngpio, sizeof(*gdev->descs), GFP_KERNEL);
> > if (!gdev->descs) {
> > @@ -947,7 +953,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc,
> > void *data,
> > /* failures here can mean systems won't boot... */
> > if (ret != -EPROBE_DEFER) {
> > pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n",
> > __func__,
> > - base, base + (int)ngpios - 1,
> > + base, base + (int)gc->ngpio - 1,
> > gc->label ? : "generic", ret);
> > }
> > return ret;
> > diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index
> > cca81375f127..8de748a16d13 100644
> > --- a/drivers/gpio/gpiolib.h
> > +++ b/drivers/gpio/gpiolib.h
> > @@ -217,6 +217,7 @@ int gpiod_configure_flags(struct gpio_desc *desc, const
> > char *con_id, int gpio_set_debounce_timeout(struct gpio_desc *desc,
> > unsigned int debounce); int gpiod_hog(struct gpio_desc *desc, const char
> > *name,
> > unsigned long lflags, enum gpiod_flags dflags);
> > +int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev);
> >
> > /*
> > * Return the GPIO number of the passed descriptor relative to its chip
> > --
> > 2.30.1
>
> Hi Bart,
>
> This is the final approved patch by both Linus and Andy. Please discard all others.
>
Ok, I applied this one but you need to get your patch versioning in
order next time.
Bart
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-07-18 19:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-11 15:12 [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init() Asmaa Mnebhi
2023-07-11 16:15 ` Andy Shevchenko
2023-07-11 17:10 ` Asmaa Mnebhi
2023-07-16 7:41 ` Linus Walleij
2023-07-18 18:20 ` Bartosz Golaszewski
2023-07-18 18:34 ` Andy Shevchenko
2023-07-18 18:38 ` Asmaa Mnebhi
2023-07-18 19:00 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox