Re: [PATCH] gpio: regmap: move struct gpio_regmap definition

[Álvaro Fernández Rojas]

Hi Michael,

El 02/03/2021 a las 19:10, Michael Walle escribió:
> Hi,
>
> Am 2021-03-02 19:06, schrieb Álvaro Fernández Rojas:
> > struct gpio_regmap should be declared in gpio/regmap.h.
> > This way other drivers can access the structure data when registering 
> > a gpio
> > regmap controller.
>
> The intention was really to keep this private to the gpio-regmap
> driver. Why do you need to access to the properties?

I'm trying to add support for bcm63xx pin controllers, and Linus 
suggested that I could use gpio regmap instead of adding duplicated code.
However, I need to access gpio_chip inside gpio_regmap to call 
pinctrl_add_gpio_range() with gpio_chip.base.

> -michael
>
> > Fixes: ebe363197e52 ("gpio: add a reusable generic gpio_chip using 
> > regmap")
> > Signed-off-by: Álvaro Fernández Rojas <noltari@xxxxxxxxx>
> > ---
> >  drivers/gpio/gpio-regmap.c  | 20 ------------------
> >  include/linux/gpio/regmap.h | 41 ++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 40 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> > index 5412cb3b0b2a..23b0a8572f53 100644
> > --- a/drivers/gpio/gpio-regmap.c
> > +++ b/drivers/gpio/gpio-regmap.c
> > @@ -11,26 +11,6 @@
> >  #include <linux/module.h>
> >  #include <linux/regmap.h>
> >
> > -struct gpio_regmap {
> > -    struct device *parent;
> > -    struct regmap *regmap;
> > -    struct gpio_chip gpio_chip;
> > -
> > -    int reg_stride;
> > -    int ngpio_per_reg;
> > -    unsigned int reg_dat_base;
> > -    unsigned int reg_set_base;
> > -    unsigned int reg_clr_base;
> > -    unsigned int reg_dir_in_base;
> > -    unsigned int reg_dir_out_base;
> > -
> > -    int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> > -                  unsigned int offset, unsigned int *reg,
> > -                  unsigned int *mask);
> > -
> > -    void *driver_data;
> > -};
> > -
> >  static unsigned int gpio_regmap_addr(unsigned int addr)
> >  {
> >      if (addr == GPIO_REGMAP_ADDR_ZERO)
> > diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
> > index ad76f3d0a6ba..ce2fc6e9b62b 100644
> > --- a/include/linux/gpio/regmap.h
> > +++ b/include/linux/gpio/regmap.h
> > @@ -4,13 +4,52 @@
> >  #define _LINUX_GPIO_REGMAP_H
> >
> >  struct device;
> > -struct gpio_regmap;
> >  struct irq_domain;
> >  struct regmap;
> >
> >  #define GPIO_REGMAP_ADDR_ZERO ((unsigned int)(-1))
> >  #define GPIO_REGMAP_ADDR(addr) ((addr) ? : GPIO_REGMAP_ADDR_ZERO)
> >
> > +/**
> > + * struct gpio_regmap - GPIO regmap controller
> > + * @parent:        The parent device
> > + * @regmap:        The regmap used to access the registers
> > + *            given, the name of the device is used
> > + * @gpio_chip:        GPIO chip controller
> > + * @ngpio_per_reg:    Number of GPIOs per register
> > + * @reg_stride:        (Optional) May be set if the registers (of the
> > + *            same type, dat, set, etc) are not consecutive.
> > + * @reg_dat_base:    (Optional) (in) register base address
> > + * @reg_set_base:    (Optional) set register base address
> > + * @reg_clr_base:    (Optional) clear register base address
> > + * @reg_dir_in_base:    (Optional) in setting register base address
> > + * @reg_dir_out_base:    (Optional) out setting register base address
> > + * @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.
> > + * @driver_data:    (Optional) driver-private data
> > + */
> > +struct gpio_regmap {
> > +    struct device *parent;
> > +    struct regmap *regmap;
> > +    struct gpio_chip gpio_chip;
> > +
> > +    int reg_stride;
> > +    int ngpio_per_reg;
> > +    unsigned int reg_dat_base;
> > +    unsigned int reg_set_base;
> > +    unsigned int reg_clr_base;
> > +    unsigned int reg_dir_in_base;
> > +    unsigned int reg_dir_out_base;
> > +
> > +    int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> > +                  unsigned int offset, unsigned int *reg,
> > +                  unsigned int *mask);
> > +
> > +    void *driver_data;
> > +};
> > +
> >  /**
> >   * struct gpio_regmap_config - Description of a generic regmap 
> > gpio_chip.
> >   * @parent:        The parent device

Best regards,
Álvaro.