lkml.org 
[lkml]   [2020]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 2/2] gpio: add a reusable generic gpio_chip using regmap
Hi,

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)
>
> 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:
case GPIO_CONTROL_1:
*reg = base;
*mask = (1 << offset);
break;
case GPIO_OUTPUT_1:
*reg = base + offset;
*mask = ...
break;
}

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

>> +/**
>> + * struct gpio_regmap_config - Description of a generic regmap
>> gpio_chip.
>> + *
>> + * @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
>> address
>> + * @reg_stride: (Optional) May be set if the registers (of
>> the
>> + * 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
>> reg_*_base.
>> + *
>> + * All base addresses may have the special value
>> GPIO_REGMAP_ADDR_ZERO
>> + * 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.

[snip]

-michael

\
 
 \ /
  Last update: 2020-05-26 23:29    [W:0.086 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site