| Message ID | 20220703111057.23246-3-aidanmacdonald.0x0@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpio-regmap support for register fields and other hooks | expand |
On Sun, Jul 3, 2022 at 1:11 PM Aidan MacDonald <aidanmacdonald.0x0@gmail.com> wrote: > > Allow gpio-regmap to be used for the GPIO portion of a combined > pin control and GPIO driver by setting the has_pinctrl flag. This > flag will cause GPIO direction set ops to be implemented as calls > to pinctrl_gpio_direction_input/output() instead of updating the > direction set registers directly. > > Note that reg_dir_out/in_base is still required for implementing > the GPIO chip's ->get_direction() callback. ... > + /* > + * we need a direction register for implementing ->get_direction > + * even if ->direction_input/output is handled by pin control > + */ /* * Multi-line comments go with this format * or style. Pay attention to the capitalization * and English grammar, e.g. period at the end of sentence(s). */ ... > + if (config->has_pinctrl && !(config->reg_dir_in_base || > + config->reg_dir_out_base)) Can you re-indent this either to be one line or put the second part of the conditional onto the second line? And why not use && everywhere? > + return ERR_PTR(-EINVAL);
Andy Shevchenko <andy.shevchenko@gmail.com> writes: > On Sun, Jul 3, 2022 at 1:11 PM Aidan MacDonald > <aidanmacdonald.0x0@gmail.com> wrote: >> >> Allow gpio-regmap to be used for the GPIO portion of a combined >> pin control and GPIO driver by setting the has_pinctrl flag. This >> flag will cause GPIO direction set ops to be implemented as calls >> to pinctrl_gpio_direction_input/output() instead of updating the >> direction set registers directly. >> >> Note that reg_dir_out/in_base is still required for implementing >> the GPIO chip's ->get_direction() callback. > > ... > >> + /* >> + * we need a direction register for implementing ->get_direction >> + * even if ->direction_input/output is handled by pin control >> + */ > > /* > * Multi-line comments go with this format > * or style. Pay attention to the capitalization > * and English grammar, e.g. period at the end of sentence(s). > */ > I used this "style" to match the surrounding code, but I suppose I might as well fix the other comments while I'm here. >> + if (config->has_pinctrl && !(config->reg_dir_in_base || >> + config->reg_dir_out_base)) > > Can you re-indent this either to be one line or put the second part of > the conditional onto the second line? Yep. > > And why not use && everywhere? > No reason to be honest, but maybe it's easier to understand? "has pin control and doesn't set reg_dir_in_base or reg_dir_out_base". Using && is more like this: "has pin control, doesn't set reg_dir_in_base, and doesn't set reg_dir_out_base".
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c index 9256b922c654..4bc01329fb14 100644 --- a/drivers/gpio/gpio-regmap.c +++ b/drivers/gpio/gpio-regmap.c @@ -24,6 +24,8 @@ struct gpio_regmap { unsigned int reg_dir_in_base; unsigned int reg_dir_out_base; + unsigned int has_pinctrl:1; + int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base, unsigned int offset, unsigned int *reg, unsigned int *mask); @@ -170,14 +172,24 @@ static int gpio_regmap_set_direction(struct gpio_chip *chip, static int gpio_regmap_direction_input(struct gpio_chip *chip, unsigned int offset) { + struct gpio_regmap *gpio = gpiochip_get_data(chip); + + if (gpio->has_pinctrl) + return pinctrl_gpio_direction_input(chip->base + offset); + return gpio_regmap_set_direction(chip, offset, false); } static int gpio_regmap_direction_output(struct gpio_chip *chip, unsigned int offset, int value) { + struct gpio_regmap *gpio = gpiochip_get_data(chip); + gpio_regmap_set(chip, offset, value); + if (gpio->has_pinctrl) + return pinctrl_gpio_direction_output(chip->base + offset); + return gpio_regmap_set_direction(chip, offset, true); } @@ -218,6 +230,14 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config if (config->reg_dir_out_base && config->reg_dir_in_base) return ERR_PTR(-EINVAL); + /* + * we need a direction register for implementing ->get_direction + * even if ->direction_input/output is handled by pin control + */ + if (config->has_pinctrl && !(config->reg_dir_in_base || + config->reg_dir_out_base)) + return ERR_PTR(-EINVAL); + /* only one of these should be provided */ if (config->reg_field_xlate && config->reg_mask_xlate) return ERR_PTR(-EINVAL); diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h index a673dbfe88a3..47acea8cca32 100644 --- a/include/linux/gpio/regmap.h +++ b/include/linux/gpio/regmap.h @@ -33,6 +33,10 @@ struct regmap; * @ngpio_per_reg: Number of GPIOs per register * @irq_domain: (Optional) IRQ domain if the controller is * interrupt-capable + * @has_pinctrl: If set, the GPIO chip is part of a combined pin control + * and GPIO driver; use pinctrl_gpio_direction_input() and + * pinctrl_gpio_direction_output() to implement direction + * set operations. * @reg_mask_xlate: (Optional) Translates base address and GPIO * offset to a register/bitmask pair. If not * given the default gpio_regmap_simple_xlate() @@ -88,6 +92,8 @@ struct gpio_regmap_config { int ngpio_per_reg; struct irq_domain *irq_domain; + unsigned int has_pinctrl:1; + int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base, unsigned int offset, unsigned int *reg, unsigned int *mask);
Allow gpio-regmap to be used for the GPIO portion of a combined pin control and GPIO driver by setting the has_pinctrl flag. This flag will cause GPIO direction set ops to be implemented as calls to pinctrl_gpio_direction_input/output() instead of updating the direction set registers directly. Note that reg_dir_out/in_base is still required for implementing the GPIO chip's ->get_direction() callback. Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> --- drivers/gpio/gpio-regmap.c | 20 ++++++++++++++++++++ include/linux/gpio/regmap.h | 6 ++++++ 2 files changed, 26 insertions(+)