| 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: <CH2PR12MB389544F08A0A20AAB06123D3D738A@CH2PR12MB3895.namprd12.prod.outlook.com>
Date: Tue, 18 Jul 2023 18:38:20 +0000
From: Asmaa Mnebhi <asmaa@...dia.com>
To: "andy.shevchenko@...il.com" <andy.shevchenko@...il.com>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
"brgl@...ev.pl" <brgl@...ev.pl>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: David Thompson <davthompson@...dia.com>
Subject: RE: [PATCH v2 1/1] gpio: mmio: handle "ngpios" properly in
bgpio_init()
> 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.
Powered by blists - more mailing lists