| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 18 Jul 2023 21:00:05 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Asmaa Mnebhi <asmaa@...dia.com>
Cc: "andy.shevchenko@...il.com" <andy.shevchenko@...il.com>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
David Thompson <davthompson@...dia.com>
Subject: Re: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in bgpio_init()
On Tue, Jul 18, 2023 at 8:38 PM Asmaa Mnebhi <asmaa@...dia.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@...dia.com>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.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
Powered by blists - more mailing lists