On Tue, Jul 18, 2023 at 8:38 PM Asmaa Mnebhi <asmaa@xxxxxxxxxx> 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@xxxxxxxxxx> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > --- > > 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