Some GPIO chips allow to rise an IRQ on GPIO level changes but do not
provide an IRQ status for each separate line: only the current gpio
level can be retrieved.
Add support for these chips, emulating IRQ status by comparing GPIO
levels with the levels during the previous interrupt.
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
drivers/base/regmap/regmap-irq.c | 97 +++++++++++++++++++++++++++-------------
include/linux/regmap.h | 3 ++
2 files changed, 69 insertions(+), 31 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 0bcd81389a29..0e53b64d028d 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -33,6 +33,7 @@ struct regmap_irq_chip_data {
void *status_reg_buf;
unsigned int *main_status_buf;
unsigned int *status_buf;
+ unsigned int *prev_status_buf;
unsigned int *mask_buf;
unsigned int *mask_buf_def;
unsigned int *wake_buf;
@@ -332,27 +333,13 @@ static inline int read_sub_irq_data(struct regmap_irq_chip_data *data,
return ret;
}
-static irqreturn_t regmap_irq_thread(int irq, void *d)
+static int read_irq_data(struct regmap_irq_chip_data *data)
{
- struct regmap_irq_chip_data *data = d;
const struct regmap_irq_chip *chip = data->chip;
struct regmap *map = data->map;
int ret, i;
- bool handled = false;
u32 reg;
- if (chip->handle_pre_irq)
- chip->handle_pre_irq(chip->irq_drv_data);
-
- if (chip->runtime_pm) {
- ret = pm_runtime_get_sync(map->dev);
- if (ret < 0) {
- dev_err(map->dev, "IRQ thread failed to resume: %d\n",
- ret);
- goto exit;
- }
- }
-
/*
* Read only registers with active IRQs if the chip has 'main status
* register'. Else read in the statuses, using a single bulk read if
@@ -379,10 +366,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
reg = data->get_irq_reg(data, chip->main_status, i);
ret = regmap_read(map, reg, &data->main_status_buf[i]);
if (ret) {
- dev_err(map->dev,
- "Failed to read IRQ status %d\n",
- ret);
- goto exit;
+ dev_err(map->dev, "Failed to read IRQ status %d\n", ret);
+ return ret;
}
}
@@ -398,10 +383,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
ret = read_sub_irq_data(data, b);
if (ret != 0) {
- dev_err(map->dev,
- "Failed to read IRQ status %d\n",
- ret);
- goto exit;
+ dev_err(map->dev, "Failed to read IRQ status %d\n", ret);
+ return ret;
}
}
@@ -418,9 +401,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
data->status_reg_buf,
chip->num_regs);
if (ret != 0) {
- dev_err(map->dev, "Failed to read IRQ status: %d\n",
- ret);
- goto exit;
+ dev_err(map->dev, "Failed to read IRQ status: %d\n", ret);
+ return ret;
}
for (i = 0; i < data->chip->num_regs; i++) {
@@ -436,7 +418,7 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
break;
default:
BUG();
- goto exit;
+ return ret;
}
}
@@ -447,10 +429,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
ret = regmap_read(map, reg, &data->status_buf[i]);
if (ret != 0) {
- dev_err(map->dev,
- "Failed to read IRQ status: %d\n",
- ret);
- goto exit;
+ dev_err(map->dev, "Failed to read IRQ status: %d\n", ret);
+ return ret;
}
}
}
@@ -459,6 +439,42 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
for (i = 0; i < data->chip->num_regs; i++)
data->status_buf[i] = ~data->status_buf[i];
+ return 0;
+}
+
+static irqreturn_t regmap_irq_thread(int irq, void *d)
+{
+ struct regmap_irq_chip_data *data = d;
+ const struct regmap_irq_chip *chip = data->chip;
+ struct regmap *map = data->map;
+ int ret, i;
+ bool handled = false;
+ u32 reg;
+
+ if (chip->handle_pre_irq)
+ chip->handle_pre_irq(chip->irq_drv_data);
+
+ if (chip->runtime_pm) {
+ ret = pm_runtime_get_sync(map->dev);
+ if (ret < 0) {
+ dev_err(map->dev, "IRQ thread failed to resume: %d\n", ret);
+ goto exit;
+ }
+ }
+
+ ret = read_irq_data(data);
+ if (ret < 0)
+ goto exit;
+
+ if (chip->status_is_level) {
+ for (i = 0; i < data->chip->num_regs; i++) {
+ unsigned int val = data->status_buf[i];
+
+ data->status_buf[i] ^= data->prev_status_buf[i];
+ data->prev_status_buf[i] = val;
+ }
+ }
+
/*
* Ignore masked IRQs and ack if we need to; we ack early so
* there is no race between handling and acknowledging the
@@ -705,6 +721,13 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
if (!d->status_buf)
goto err_alloc;
+ if (chip->status_is_level) {
+ d->prev_status_buf = kcalloc(chip->num_regs, sizeof(*d->prev_status_buf),
+ GFP_KERNEL);
+ if (!d->prev_status_buf)
+ goto err_alloc;
+ }
+
d->mask_buf = kcalloc(chip->num_regs, sizeof(*d->mask_buf),
GFP_KERNEL);
if (!d->mask_buf)
@@ -881,6 +904,16 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
}
}
+ /* Store current levels */
+ if (chip->status_is_level) {
+ ret = read_irq_data(d);
+ if (ret < 0)
+ goto err_alloc;
+
+ memcpy(d->prev_status_buf, d->status_buf,
+ d->chip->num_regs * sizeof(d->prev_status_buf[0]));
+ }
+
ret = regmap_irq_create_domain(fwnode, irq_base, chip, d);
if (ret)
goto err_alloc;
@@ -907,6 +940,7 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
kfree(d->mask_buf_def);
kfree(d->mask_buf);
kfree(d->status_buf);
+ kfree(d->prev_status_buf);
kfree(d->status_reg_buf);
if (d->config_buf) {
for (i = 0; i < chip->num_config_bases; i++)
@@ -983,6 +1017,7 @@ void regmap_del_irq_chip(int irq, struct regmap_irq_chip_data *d)
kfree(d->mask_buf);
kfree(d->status_reg_buf);
kfree(d->status_buf);
+ kfree(d->prev_status_buf);
if (d->config_buf) {
for (i = 0; i < d->chip->num_config_bases; i++)
kfree(d->config_buf[i]);
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 3a96d068915f..159527e97f00 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1640,6 +1640,8 @@ struct regmap_irq_chip_data;
* @ack_invert: Inverted ack register: cleared bits for ack.
* @clear_ack: Use this to set 1 and 0 or vice-versa to clear interrupts.
* @status_invert: Inverted status register: cleared bits are active interrupts.
+ * @status_is_level: Status register is actuall signal level: Xor status
+ * register with previous value to get active interrupts.
* @wake_invert: Inverted wake register: cleared bits are wake enabled.
* @type_in_mask: Use the mask registers for controlling irq type. Use this if
* the hardware provides separate bits for rising/falling edge
@@ -1703,6 +1705,7 @@ struct regmap_irq_chip {
unsigned int ack_invert:1;
unsigned int clear_ack:1;
unsigned int status_invert:1;
+ unsigned int status_is_level:1;
unsigned int wake_invert:1;
unsigned int type_in_mask:1;
unsigned int clear_on_unmask:1;
--
2.39.5On Tue, Mar 18, 2025 at 05:26:21PM +0100, Mathieu Dubois-Briand wrote:
> Some GPIO chips allow to rise an IRQ on GPIO level changes but do not
> provide an IRQ status for each separate line: only the current gpio
> level can be retrieved.
>
> Add support for these chips, emulating IRQ status by comparing GPIO
> levels with the levels during the previous interrupt.
Some nit-picks below, but either way
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
...
> default:
> BUG();
> - goto exit;
> + return ret;
Hmm... BUG() implies unreachable, perhaps just a precursor patch to drop this
goto completely?
...
> + /* Store current levels */
> + if (chip->status_is_level) {
> + ret = read_irq_data(d);
> + if (ret < 0)
> + goto err_alloc;
> +
> + memcpy(d->prev_status_buf, d->status_buf,
> + d->chip->num_regs * sizeof(d->prev_status_buf[0]));
Perhaps array_size()?
> + }
--
With Best Regards,
Andy Shevchenko
On Tue Mar 18, 2025 at 5:39 PM CET, Andy Shevchenko wrote:
> On Tue, Mar 18, 2025 at 05:26:21PM +0100, Mathieu Dubois-Briand wrote:
> > Some GPIO chips allow to rise an IRQ on GPIO level changes but do not
> > provide an IRQ status for each separate line: only the current gpio
> > level can be retrieved.
> >
> > Add support for these chips, emulating IRQ status by comparing GPIO
> > levels with the levels during the previous interrupt.
>
>
> Some nit-picks below, but either way
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> ...
>
> > default:
> > BUG();
> > - goto exit;
> > + return ret;
>
> Hmm... BUG() implies unreachable, perhaps just a precursor patch to drop this
> goto completely?
>
Ok, I will add a separate patch to remove the goto.
> ...
>
> > + /* Store current levels */
> > + if (chip->status_is_level) {
> > + ret = read_irq_data(d);
> > + if (ret < 0)
> > + goto err_alloc;
> > +
> > + memcpy(d->prev_status_buf, d->status_buf,
> > + d->chip->num_regs * sizeof(d->prev_status_buf[0]));
>
> Perhaps array_size()?
>
OK
> > + }
Thanks for your review, and thanks for the tag.
Mathieu
--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Thu, Mar 20, 2025 at 09:45:28AM +0100, Mathieu Dubois-Briand wrote: > On Tue Mar 18, 2025 at 5:39 PM CET, Andy Shevchenko wrote: > > On Tue, Mar 18, 2025 at 05:26:21PM +0100, Mathieu Dubois-Briand wrote: ... > > > default: > > > BUG(); > > > - goto exit; > > > + return ret; > > > > Hmm... BUG() implies unreachable, perhaps just a precursor patch to drop this > > goto completely? > > Ok, I will add a separate patch to remove the goto. I just browsed the code for the similar and there are handful that do this. At least one commit (from 2011) refers to GCC 4.3.3 that complains, but our minimum requirement AFAIR is 5.1 nowadays. In any case it's up to you. I am totally fine if you leave this as is. -- With Best Regards, Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.