| 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
| ||
|
Message-ID: <ZAZrKPw38ERSbzXg@surfacebook>
Date: Tue, 7 Mar 2023 00:37:28 +0200
From: andy.shevchenko@...il.com
To: Asmaa Mnebhi <asmaa@...dia.com>
Cc: andy.shevchenko@...il.com, linus.walleij@...aro.org,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] gpio: mmio: handle "ngpios" properly in bgpio_init()
Mon, Mar 06, 2023 at 04:12:37PM -0500, Asmaa Mnebhi kirjoitti:
> 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().
Thank you!
Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
Sorry it took a bit longer for us to settle on this.
> Signed-off-by: Asmaa Mnebhi <asmaa@...dia.com>
> ---
> 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..b52a3dd511ca 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 = 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 939c776b9488..1964ec64e356 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -647,6 +647,40 @@ static void gpiochip_setup_devs(void)
> }
> }
>
> +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)
> @@ -655,7 +689,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;
>
> @@ -704,36 +737,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) {
> @@ -903,7 +909,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 b3c2db6eba80..c38cbf1b753b 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -207,6 +207,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
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists