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(-)
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);
--
2.39.5On 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
--
With Best Regards,
Andy Shevchenko
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
--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
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;
> >> }
--
With Best Regards,
Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.