| 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: Thu, 20 May 2021 13:42:54 +0200
From: Michael Walle <michael@...le.cc>
To: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Cc: Matti Vaittinen <mazziesaccount@...il.com>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-power@...rohmeurope.com
Subject: Re: [PATCH 1/2] gpio: regmap: Support few IC specific operations
Hi Matti,
Am 2021-05-20 13:28, schrieb Matti Vaittinen:
> The set_config and init_valid_mask GPIO operations are usually very IC
> specific. Allow IC drivers to provide these custom operations at
> gpio-regmap registration.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
> ---
> drivers/gpio/gpio-regmap.c | 49 +++++++++++++++++++++++++++++++++++++
> include/linux/gpio/regmap.h | 13 ++++++++++
> 2 files changed, 62 insertions(+)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 134cedf151a7..315285cacd3f 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -27,6 +27,10 @@ struct gpio_regmap {
> int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> unsigned int offset, unsigned int *reg,
> unsigned int *mask);
> + int (*set_config)(struct regmap *regmap, void *drvdata,
> + unsigned int offset, unsigned long config);
> + int (*init_valid_mask)(struct regmap *regmap, void *drvdata,
> + unsigned long *valid_mask, unsigned int ngpios);
Maybe we should also make the first argument a "struct gpio_regmap"
and provide a new gpio_regmap_get_regmap(struct gpio_regmap). Thus
having a similar api as for the reg_mask_xlate(). Andy?
>
> void *driver_data;
> };
> @@ -39,6 +43,43 @@ static unsigned int gpio_regmap_addr(unsigned int
> addr)
> return addr;
> }
>
> +static int regmap_gpio_init_valid_mask(struct gpio_chip *gc,
> + unsigned long *valid_mask,
> + unsigned int ngpios)
> +{
> + struct gpio_regmap *gpio;
> + void *drvdata;
> +
> + gpio = gpiochip_get_data(gc);
> +
> + if (!gpio->init_valid_mask) {
> + WARN_ON(!gpio->init_valid_mask);
> + return -EINVAL;
> + }
Why not the following?
if (!gpio->init_valid_mask)
return 0;
Thus copying the behavior of gpiolib.
> +
> + drvdata = gpio_regmap_get_drvdata(gpio);
> +
> + return gpio->init_valid_mask(gpio->regmap, drvdata, valid_mask,
> ngpios);
> +}
> +
> +static int gpio_regmap_set_config(struct gpio_chip *gc, unsigned int
> offset,
> + unsigned long config)
> +{
> + struct gpio_regmap *gpio;
> + void *drvdata;
> +
> + gpio = gpiochip_get_data(gc);
> +
> + if (!gpio->set_config) {
> + WARN_ON(!gpio->set_config);
> + return -EINVAL;
> + }
same here, return -ENOTSUPP.
> +
> + drvdata = gpio_regmap_get_drvdata(gpio);
> +
> + return gpio->set_config(gpio->regmap, drvdata, offset, config);
> +}
> +
> static int gpio_regmap_simple_xlate(struct gpio_regmap *gpio,
> unsigned int base, unsigned int offset,
> unsigned int *reg, unsigned int *mask)
> @@ -235,6 +276,8 @@ struct gpio_regmap *gpio_regmap_register(const
> struct gpio_regmap_config *config
> gpio->reg_clr_base = config->reg_clr_base;
> gpio->reg_dir_in_base = config->reg_dir_in_base;
> gpio->reg_dir_out_base = config->reg_dir_out_base;
> + gpio->set_config = config->set_config;
> + gpio->init_valid_mask = config->init_valid_mask;
>
> /* if not set, assume there is only one register */
> if (!gpio->ngpio_per_reg)
> @@ -253,6 +296,10 @@ struct gpio_regmap *gpio_regmap_register(const
> struct gpio_regmap_config *config
> chip->ngpio = config->ngpio;
> chip->names = config->names;
> chip->label = config->label ?: dev_name(config->parent);
> + if (gpio->set_config)
> + chip->set_config = gpio_regmap_set_config;
> + if (gpio->init_valid_mask)
> + chip->init_valid_mask = regmap_gpio_init_valid_mask;
>
> #if defined(CONFIG_OF_GPIO)
> /* gpiolib will use of_node of the parent if chip->of_node is NULL */
> @@ -280,6 +327,8 @@ struct gpio_regmap *gpio_regmap_register(const
> struct gpio_regmap_config *config
> chip->direction_output = gpio_regmap_direction_output;
> }
>
> + gpio_regmap_set_drvdata(gpio, config->drvdata);
I'm wondering if we need the gpio_regmap_set_drvdata() anymore or if
we can just drop it entirely.
> +
> ret = gpiochip_add_data(chip, gpio);
> if (ret < 0)
> goto err_free_gpio;
> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
> index 334dd928042b..c382a3caefc3 100644
> --- a/include/linux/gpio/regmap.h
> +++ b/include/linux/gpio/regmap.h
> @@ -33,10 +33,18 @@ struct regmap;
> * @ngpio_per_reg: Number of GPIOs per register
> * @irq_domain: (Optional) IRQ domain if the controller is
> * interrupt-capable
> + * @drvdata: (Optional) Pointer to IC specific data which is
> + * not used by gpio-remap but is provided "as is" to
> + * the driver callback(s).
> + *
> * @reg_mask_xlate: (Optional) Translates base address and GPIO
> * offset to a register/bitmask pair. If not
> * given the default gpio_regmap_simple_xlate()
> * is used.
> + * @set_config: (Optional) hook for all kinds of settings. Uses
> + * the same packed config format as generic pinconf.
> + * @init_valid_mask: (Optional) routine to initialize @valid_mask, to
> + * be used if not all GPIOs are valid.
> *
> * The ->reg_mask_xlate translates a given base address and GPIO
> offset to
> * register and mask pair. The base address is one of the given
> register
> @@ -74,10 +82,15 @@ struct gpio_regmap_config {
> int reg_stride;
> int ngpio_per_reg;
> struct irq_domain *irq_domain;
> + void *drvdata;
>
> int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> unsigned int offset, unsigned int *reg,
> unsigned int *mask);
> + int (*set_config)(struct regmap *regmap, void *drvdata,
> + unsigned int offset, unsigned long config);
> + int (*init_valid_mask)(struct regmap *regmap, void *drvdata,
> + unsigned long *valid_mask, unsigned int ngpios);
> };
>
> struct gpio_regmap *gpio_regmap_register(const struct
> gpio_regmap_config *config);
>
> base-commit: d07f6ca923ea0927a1024dfccafc5b53b61cfecc
> --
> 2.25.4
-michael
Powered by blists - more mailing lists