Re: [PATCH v4 2/2] gpio: add a reusable generic gpio_chip using regmap

From: Michael Walle
Date: Tue May 26 2020 - 17:28:03 EST


Am 2020-05-26 19:29, schrieb Pierre-Louis Bossart:
+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;

I wonder if a base is enough, shouldn't there be a 'last' or something
that constrains the range of regmap addresses to be used for gpios?

This should be covered on the regmap, shouldn't it?

related question since I couldn't figure out how to convert my PCM512x
example, where there are 6 GPIOs configured with 3 regmap-visible
registers [1], to this mapping.

GPIO_EN defines if the GPIO is used or not (each bitfield is tied to a

GPIO_CONTROL_1 defines the level (each bitfield is tied to a GPIO)

GPIO_OUTPUT_1+offset defines what signal is routed to each GPIO. I am
really not sure how this part would be handled?

That's 8 registers total to deal with GPIOs.

Looks like you need a custom xlate function:

int pcm512x_gpio_regmap_xlate(struct gpio_regmap *gpio, unsigned int base,
unsigned int offset, unsigned int *reg,
unsigned int *mask)
switch (base)
case GPIO_EN:
*reg = base;
*mask = (1 << offset);
*reg = base + offset;
*mask = ...

base is always one of the xx_base properties in the "struct

+ * struct gpio_regmap_config - Description of a generic regmap
+ *
+ * @parent: The parent device
+ * @regmap: The regmap used to access the registers
+ * given, the name of the device is used
+ * @label: (Optional) Descriptive name for GPIO controller.
+ * If not given, the name of the device is used.
+ * @ngpio: Number of GPIOs
+ * @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
+ * @reg_stride: (Optional) May be set if the registers (of
+ * same type, dat, set, etc) are not consecutive.
+ * @ngpio_per_reg: Number of GPIOs per register
+ * @irq_domain: (Optional) IRQ domain if the controller is
+ * interrupt-capable
+ * @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.
+ *
+ * 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
+ *
+ * All base addresses may have the special value
+ * which forces the address to the value 0.
+ */
+struct gpio_regmap_config {
+ struct device *parent;
+ struct regmap *regmap;
+ const char *label;
+ int ngpio;

could we add a .names field for the gpio_chip, I found this useful for
PCM512x GPIO support, e.g.

Sure, I have the names in the device tree.

But I'd prefer that you'd do a patch on top of this (assuming it is
applied soon), because you can actually test it and there might be
missing more.