| Message ID | 20250409-mdb-max7360-support-v6-7-7a2535876e39@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for MAX7360 | expand |
On Wed, Apr 09, 2025 at 04:55:54PM +0200, Mathieu Dubois-Briand wrote: > GPIO controller often have support for IRQ: allow to easily allocate > both gpio-regmap and regmap-irq in one operation. > > - memcpy(d->prev_status_buf, d->status_buf, array_size(d->prev_status_buf)); > + memcpy(d->prev_status_buf, d->status_buf, > + array_size(d->chip->num_regs, sizeof(d->prev_status_buf[0]))); ... > +#ifdef CONFIG_REGMAP_IRQ > + if (config->regmap_irq_chip) { > + struct regmap_irq_chip_data *irq_chip_data; > + > + ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent), > + config->regmap, config->regmap_irq_irqno, > + config->regmap_irq_flags, 0, > + config->regmap_irq_chip, &irq_chip_data); > + if (ret) > + goto err_free_gpio; > + > + irq_domain = regmap_irq_get_domain(irq_chip_data); > + } else > +#endif > + irq_domain = config->irq_domain; > + This is blank line is not needed, but I not mind either way. > + if (irq_domain) { > + ret = gpiochip_irqchip_add_domain(chip, irq_domain); > if (ret) > goto err_remove_gpiochip; > } ... > + * @regmap_irq_chip: (Optional) Pointer on an regmap_irq_chip structure. If > + * set, a regmap-irq device will be created and the IRQ > + * domain will be set accordingly. > + * @regmap_irq_chip_data: (Optional) Pointer on an regmap_irq_chip_data > + * structure pointer. If set, it will be populated with a > + * pointer on allocated regmap_irq data. Leftover? > + * @regmap_irq_irqno (Optional) The IRQ the device uses to signal interrupts. > + * @regmap_irq_flags (Optional) The IRQF_ flags to use for the interrupt. ... > +#ifdef CONFIG_REGMAP_IRQ > + struct regmap_irq_chip *regmap_irq_chip; > + int regmap_irq_irqno; Perhaps call it line? int regmap_irq_line; > + unsigned long regmap_irq_flags; > +#endif
On Wed Apr 9, 2025 at 6:39 PM CEST, Andy Shevchenko wrote: > On Wed, Apr 09, 2025 at 04:55:54PM +0200, Mathieu Dubois-Briand wrote: >> GPIO controller often have support for IRQ: allow to easily allocate >> both gpio-regmap and regmap-irq in one operation. > >> >> - memcpy(d->prev_status_buf, d->status_buf, array_size(d->prev_status_buf)); >> + memcpy(d->prev_status_buf, d->status_buf, >> + array_size(d->chip->num_regs, sizeof(d->prev_status_buf[0]))); > > ... > >> +#ifdef CONFIG_REGMAP_IRQ >> + if (config->regmap_irq_chip) { >> + struct regmap_irq_chip_data *irq_chip_data; >> + >> + ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent), >> + config->regmap, config->regmap_irq_irqno, >> + config->regmap_irq_flags, 0, >> + config->regmap_irq_chip, &irq_chip_data); >> + if (ret) >> + goto err_free_gpio; >> + >> + irq_domain = regmap_irq_get_domain(irq_chip_data); >> + } else >> +#endif >> + irq_domain = config->irq_domain; > >> + > > This is blank line is not needed, but I not mind either way. > I can remove it, but as the line above is potentially part of the "else", I have a small preference for keeping it. >> + if (irq_domain) { >> + ret = gpiochip_irqchip_add_domain(chip, irq_domain); >> if (ret) >> goto err_remove_gpiochip; >> } > > ... > >> + * @regmap_irq_chip: (Optional) Pointer on an regmap_irq_chip structure. If >> + * set, a regmap-irq device will be created and the IRQ >> + * domain will be set accordingly. > >> + * @regmap_irq_chip_data: (Optional) Pointer on an regmap_irq_chip_data >> + * structure pointer. If set, it will be populated with a >> + * pointer on allocated regmap_irq data. > > Leftover? Yes, sorry... > >> + * @regmap_irq_irqno (Optional) The IRQ the device uses to signal interrupts. >> + * @regmap_irq_flags (Optional) The IRQF_ flags to use for the interrupt. > > ... > >> +#ifdef CONFIG_REGMAP_IRQ >> + struct regmap_irq_chip *regmap_irq_chip; >> + int regmap_irq_irqno; > > Perhaps call it line? > > int regmap_irq_line; > Makes sense, thanks. >> + unsigned long regmap_irq_flags; >> +#endif Thanks for your review. Mathieu
On Thu, Apr 10, 2025 at 11:03:46AM +0200, Mathieu Dubois-Briand wrote: > On Wed Apr 9, 2025 at 6:39 PM CEST, Andy Shevchenko wrote: > > On Wed, Apr 09, 2025 at 04:55:54PM +0200, Mathieu Dubois-Briand wrote: ... > >> +#ifdef CONFIG_REGMAP_IRQ > >> + if (config->regmap_irq_chip) { > >> + struct regmap_irq_chip_data *irq_chip_data; > >> + > >> + ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent), > >> + config->regmap, config->regmap_irq_irqno, > >> + config->regmap_irq_flags, 0, > >> + config->regmap_irq_chip, &irq_chip_data); > >> + if (ret) > >> + goto err_free_gpio; > >> + > >> + irq_domain = regmap_irq_get_domain(irq_chip_data); > >> + } else > >> +#endif > >> + irq_domain = config->irq_domain; > > > >> + > > > > This is blank line is not needed, but I not mind either way. > > I can remove it, but as the line above is potentially part of the > "else", I have a small preference for keeping it. Yes, but it's still coupled with the flow. But okay to leave as is. > >> + if (irq_domain) { > >> + ret = gpiochip_irqchip_add_domain(chip, irq_domain); > >> if (ret) > >> goto err_remove_gpiochip; > >> }
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index b4a72750ff6d..882c6516301b 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -6,12 +6,12 @@ // // Author: Mark Brown <broonie@opensource.wolfsonmicro.com> -#include <linux/array_size.h> #include <linux/device.h> #include <linux/export.h> #include <linux/interrupt.h> #include <linux/irq.h> #include <linux/irqdomain.h> +#include <linux/overflow.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/slab.h> @@ -910,7 +910,8 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, if (ret < 0) goto err_alloc; - memcpy(d->prev_status_buf, d->status_buf, array_size(d->prev_status_buf)); + memcpy(d->prev_status_buf, d->status_buf, + array_size(d->chip->num_regs, sizeof(d->prev_status_buf[0]))); } ret = regmap_irq_create_domain(fwnode, irq_base, chip, d); diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c index 87c4225784cf..c27cc78f9025 100644 --- a/drivers/gpio/gpio-regmap.c +++ b/drivers/gpio/gpio-regmap.c @@ -215,6 +215,7 @@ EXPORT_SYMBOL_GPL(gpio_regmap_get_drvdata); */ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config) { + struct irq_domain *irq_domain; struct gpio_regmap *gpio; struct gpio_chip *chip; int ret; @@ -295,8 +296,24 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config if (ret < 0) goto err_free_gpio; - if (config->irq_domain) { - ret = gpiochip_irqchip_add_domain(chip, config->irq_domain); +#ifdef CONFIG_REGMAP_IRQ + if (config->regmap_irq_chip) { + struct regmap_irq_chip_data *irq_chip_data; + + ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent), + config->regmap, config->regmap_irq_irqno, + config->regmap_irq_flags, 0, + config->regmap_irq_chip, &irq_chip_data); + if (ret) + goto err_free_gpio; + + irq_domain = regmap_irq_get_domain(irq_chip_data); + } else +#endif + irq_domain = config->irq_domain; + + if (irq_domain) { + ret = gpiochip_irqchip_add_domain(chip, irq_domain); if (ret) goto err_remove_gpiochip; } diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h index c722c67668c6..7ed7927ae2e9 100644 --- a/include/linux/gpio/regmap.h +++ b/include/linux/gpio/regmap.h @@ -40,6 +40,14 @@ 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). + * @regmap_irq_chip: (Optional) Pointer on an regmap_irq_chip structure. If + * set, a regmap-irq device will be created and the IRQ + * domain will be set accordingly. + * @regmap_irq_chip_data: (Optional) Pointer on an regmap_irq_chip_data + * structure pointer. If set, it will be populated with a + * pointer on allocated regmap_irq data. + * @regmap_irq_irqno (Optional) The IRQ the device uses to signal interrupts. + * @regmap_irq_flags (Optional) The IRQF_ flags to use for the interrupt. * * 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 @@ -78,6 +86,12 @@ struct gpio_regmap_config { int ngpio_per_reg; struct irq_domain *irq_domain; +#ifdef CONFIG_REGMAP_IRQ + struct regmap_irq_chip *regmap_irq_chip; + int regmap_irq_irqno; + unsigned long regmap_irq_flags; +#endif + int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base, unsigned int offset, unsigned int *reg, unsigned int *mask);
GPIO controller often have support for IRQ: allow to easily allocate both gpio-regmap and regmap-irq in one operation. Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> --- drivers/base/regmap/regmap-irq.c | 5 +++-- drivers/gpio/gpio-regmap.c | 21 +++++++++++++++++++-- include/linux/gpio/regmap.h | 14 ++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-)