| Message ID | 20250214-mdb-max7360-support-v4-4-8a35c6dbb966@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for MAX7360 | expand |
On Fri, Feb 14, 2025 at 12:49:54PM +0100, Mathieu Dubois-Briand wrote: > Allows to populate the gpio_regmap_config structure with request() and > free() callbacks to set on the final gpio_chip structure. Yeah, I understand the intention, but I have mixed feelings about this. OOH it helps with the cases like yours, OTO it sounds like a hack instead of proper implementation of the pin muxing. Yes, I know there are some drivers in the kernel do similar things, but it most likely historically and not always having the same HW capabilities as this chip.
Hi Mathieu, On Fri, 2025-02-14 at 12:49 +0100, Mathieu Dubois-Briand wrote: > Allows to populate the gpio_regmap_config structure with request() and > free() callbacks to set on the final gpio_chip structure. > > Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> > --- > drivers/gpio/gpio-regmap.c | 2 ++ > include/linux/gpio/regmap.h | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c > index 05f8781b5204..ba72c23bcf97 100644 > --- a/drivers/gpio/gpio-regmap.c > +++ b/drivers/gpio/gpio-regmap.c > @@ -261,6 +261,8 @@ struct gpio_regmap *gpio_regmap_register(const struct > gpio_regmap_config *config > chip->names = config->names; > chip->label = config->label ?: dev_name(config->parent); > chip->can_sleep = regmap_might_sleep(config->regmap); > + chip->request = config->request; > + chip->free = config->free; > > chip->request = gpiochip_generic_request; > chip->free = gpiochip_generic_free; You probably have a bad rebase here, as the chip->{request,free} functions are immediately overwritten by gpiochip_generic_{request,free}. Perhaps those can actually be used if you provide a pinctrl driver for the MFD. Best, Sander
On Sun Feb 16, 2025 at 2:17 PM CET, Sander Vanheule wrote: > Hi Mathieu, > > On Fri, 2025-02-14 at 12:49 +0100, Mathieu Dubois-Briand wrote: > > Allows to populate the gpio_regmap_config structure with request() and > > free() callbacks to set on the final gpio_chip structure. > > > > Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> > > --- > > drivers/gpio/gpio-regmap.c | 2 ++ > > include/linux/gpio/regmap.h | 7 +++++++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c > > index 05f8781b5204..ba72c23bcf97 100644 > > --- a/drivers/gpio/gpio-regmap.c > > +++ b/drivers/gpio/gpio-regmap.c > > @@ -261,6 +261,8 @@ struct gpio_regmap *gpio_regmap_register(const struct > > gpio_regmap_config *config > > chip->names = config->names; > > chip->label = config->label ?: dev_name(config->parent); > > chip->can_sleep = regmap_might_sleep(config->regmap); > > + chip->request = config->request; > > + chip->free = config->free; > > > > chip->request = gpiochip_generic_request; > > chip->free = gpiochip_generic_free; > > You probably have a bad rebase here, as the chip->{request,free} functions are immediately > overwritten by gpiochip_generic_{request,free}. Perhaps those can actually be used if you > provide a pinctrl driver for the MFD. > Thanks, indeed I missed this rebase issue. It's fixed now.
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c index 05f8781b5204..ba72c23bcf97 100644 --- a/drivers/gpio/gpio-regmap.c +++ b/drivers/gpio/gpio-regmap.c @@ -261,6 +261,8 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config chip->names = config->names; chip->label = config->label ?: dev_name(config->parent); chip->can_sleep = regmap_might_sleep(config->regmap); + chip->request = config->request; + chip->free = config->free; chip->request = gpiochip_generic_request; chip->free = gpiochip_generic_free; diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h index a9f7b7faf57b..16f0c33df75d 100644 --- a/include/linux/gpio/regmap.h +++ b/include/linux/gpio/regmap.h @@ -6,6 +6,7 @@ struct device; struct fwnode_handle; struct gpio_regmap; +struct gpio_chip; struct irq_domain; struct regmap; @@ -40,6 +41,10 @@ struct regmap; * @drvdata: (Optional) Pointer to driver specific data which is * not used by gpio-remap but is provided "as is" to the * driver callback(s). + * @request: (Optional) Hook for chip-specific activation, such as + * enabling module power and clock + * @free: (Optional) Hook for chip-specific deactivation, such as + * disabling module power and clock * * 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 register @@ -83,6 +88,8 @@ struct gpio_regmap_config { unsigned int *mask); void *drvdata; + int (*request)(struct gpio_chip *gc, unsigned int offset); + void (*free)(struct gpio_chip *gc, unsigned int offset); }; struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config);
Allows to populate the gpio_regmap_config structure with request() and free() callbacks to set on the final gpio_chip structure. Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> --- drivers/gpio/gpio-regmap.c | 2 ++ include/linux/gpio/regmap.h | 7 +++++++ 2 files changed, 9 insertions(+)