Re: [PATCH v6 6/8] gpio: regmap: Add set_config callback

[Michael Walle]

On Tue Jul 21, 2026 at 12:46 PM CEST, Yu-Chun Lin [林祐君] wrote:
> Hi Michael,
>
>>On Tue Jul 21, 2026 at 8:58 AM CEST, Yu-Chun Lin wrote:
>
> (...)
>
>>> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h 
>>> index 45a30f50043f..23460995c1d1 100644
>>> --- a/include/linux/gpio/regmap.h
>>> +++ b/include/linux/gpio/regmap.h
>>> @@ -89,6 +89,9 @@ enum gpio_regmap_operation {
>>>   *			mask before writing. This allows driver-specific logic
>>>   *			to append additional bits (like write-enable masks)
>>>   *			dynamically based on the current operation.
>>> + * @set_config:		(Optional) Callback for setting GPIO configuration such
>>> + *			as debounce, drive strength, or other hardware specific
>>> + *			settings.
>>>   * @drvdata:		(Optional) Pointer to driver specific data which is
>>>   *			not used by gpio-remap but is provided "as is" to the
>>>   *			driver callback(s).
>>> @@ -150,6 +153,8 @@ struct gpio_regmap_config {
>>>  			   unsigned int base, unsigned int offset, unsigned int reg,
>>>  			   unsigned int *mask, unsigned int *val);
>>>  
>>> +	int (*set_config)(struct gpio_chip *gc, unsigned int offset, 
>>> +unsigned long config);
>>> +
>>
>> Do we need an additional struct gpio_regmap pointer as a first argument? How
>> does the driver fetch the attached opaque data pointer? I see that your driver is doing
>>
>> struct rtd1625_gpio *data = gpiochip_get_data(chip)
>>
>> But it is the gpio-regmap core which will attach it's own data pointer to gpio_chip, no?
>>
>> -michael
>
> You are right. I mistakenly used gpiochip_get_data() here.
>
> My current plan to fix this is:
> The set_config callback in 'struct gpio_regmap_config' and 'struct gpio_regmap'
> should take 'struct gpio_regmap *' as the first argument (replacing
> 'struct gpio_chip *'), and the driver should use 'gpio_regmap_get_drvdata()'
> to retrieve its own data pointer.

You also probably need the struct gpio_chip, to maybe call into the
gpio core, no?

If so, what about

int (*set_config)(struct gpio_regmap *gpio, struct gpio_chip *gc,
                  unsigned int offset, unsigned long config)

> I'll add a wrapper in gpio-regmap.c that bridges gpio_chip to gpio_regmap and
> handles the fallback to gpiochip_generic_config().

Yes.

-michael

Attachment: signature.asc
Description: PGP signature