diff mbox series

[v2,1/6] regmap-irq: Add no_status support

Message ID bd501b4b5ff88da24d467f75e8c71b4e0e6f21e2.1677515341.git.william.gray@linaro.org
State Accepted
Commit 4d60cac951fd2dfbf261b83abb805748abc8b995
Headers show
Series Migrate IDIO-16 GPIO drivers to regmap API | expand

Commit Message

William Breathitt Gray Feb. 27, 2023, 4:45 p.m. UTC
Some devices lack status registers, yet expect to handle interrupts.
Introduce a no_status flag to indicate such a configuration, where
rather than read a status register to verify, all interrupts received
are assumed to be active.

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
Changes in v2:
 - Utilize memset32() to set status_buf for no_status case
 - Utilize GENMASK() to generate status_buf mask
 - Move no_status kernel doc line under runtime_pm line

 drivers/base/regmap/regmap-irq.c | 22 +++++++++++++++-------
 include/linux/regmap.h           |  2 ++
 2 files changed, 17 insertions(+), 7 deletions(-)

Comments

Bartosz Golaszewski March 6, 2023, 10:11 a.m. UTC | #1
On Mon, Feb 27, 2023 at 5:54 PM William Breathitt Gray
<william.gray@linaro.org> wrote:
>
> Some devices lack status registers, yet expect to handle interrupts.
> Introduce a no_status flag to indicate such a configuration, where
> rather than read a status register to verify, all interrupts received
> are assumed to be active.
>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
> ---
> Changes in v2:
>  - Utilize memset32() to set status_buf for no_status case
>  - Utilize GENMASK() to generate status_buf mask
>  - Move no_status kernel doc line under runtime_pm line
>
>  drivers/base/regmap/regmap-irq.c | 22 +++++++++++++++-------
>  include/linux/regmap.h           |  2 ++
>  2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
> index a8f185430a07..290e26664a21 100644
> --- a/drivers/base/regmap/regmap-irq.c
> +++ b/drivers/base/regmap/regmap-irq.c
> @@ -437,7 +437,10 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
>          * possible in order to reduce the I/O overheads.
>          */
>
> -       if (chip->num_main_regs) {
> +       if (chip->no_status) {
> +               /* no status register so default to all active */
> +               memset32(data->status_buf, GENMASK(31, 0), chip->num_regs);
> +       } else if (chip->num_main_regs) {
>                 unsigned int max_main_bits;
>                 unsigned long size;
>
> @@ -967,12 +970,17 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
>                         continue;
>
>                 /* Ack masked but set interrupts */
> -               reg = d->get_irq_reg(d, d->chip->status_base, i);
> -               ret = regmap_read(map, reg, &d->status_buf[i]);
> -               if (ret != 0) {
> -                       dev_err(map->dev, "Failed to read IRQ status: %d\n",
> -                               ret);
> -                       goto err_alloc;
> +               if (d->chip->no_status) {
> +                       /* no status register so default to all active */
> +                       d->status_buf[i] = GENMASK(31, 0);
> +               } else {
> +                       reg = d->get_irq_reg(d, d->chip->status_base, i);
> +                       ret = regmap_read(map, reg, &d->status_buf[i]);
> +                       if (ret != 0) {
> +                               dev_err(map->dev, "Failed to read IRQ status: %d\n",
> +                                       ret);
> +                               goto err_alloc;
> +                       }
>                 }
>
>                 if (chip->status_invert)
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index a3bc695bcca0..c6d80d4e73de 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -1564,6 +1564,7 @@ struct regmap_irq_chip_data;
>   *                   the need for a @sub_reg_offsets table.
>   * @status_invert: Inverted status register: cleared bits are active interrupts.
>   * @runtime_pm:  Hold a runtime PM lock on the device when accessing it.
> + * @no_status: No status register: all interrupts assumed generated by device.
>   *
>   * @num_regs:    Number of registers in each control bank.
>   * @irqs:        Descriptors for individual IRQs.  Interrupt numbers are
> @@ -1630,6 +1631,7 @@ struct regmap_irq_chip {
>         unsigned int clear_on_unmask:1;
>         unsigned int not_fixed_stride:1;
>         unsigned int status_invert:1;
> +       unsigned int no_status:1;
>
>         int num_regs;
>
> --
> 2.39.2
>

Mark,

If this looks good to you, could you Ack it so that I can take it
through the GPIO tree?

Bart
William Breathitt Gray March 10, 2023, 12:01 p.m. UTC | #2
On Mon, Mar 06, 2023 at 11:11:26AM +0100, Bartosz Golaszewski wrote:
> On Mon, Feb 27, 2023 at 5:54 PM William Breathitt Gray
> <william.gray@linaro.org> wrote:
> >
> > Some devices lack status registers, yet expect to handle interrupts.
> > Introduce a no_status flag to indicate such a configuration, where
> > rather than read a status register to verify, all interrupts received
> > are assumed to be active.
> >
> > Cc: Mark Brown <broonie@kernel.org>
> > Signed-off-by: William Breathitt Gray <william.gray@linaro.org>

[...]

> Mark,
> 
> If this looks good to you, could you Ack it so that I can take it
> through the GPIO tree?
> 
> Bart

Bart,

There's a change to the regmap-irq handle_mask_sync() callback I
submitted that will affect these patches [0]. If you haven't already
picked up this series, I suggest waiting until the handle_mask_sync()
change is resolved, after which I can rebase this series on top of it
and we'll end up with a cleaner git history.

[0] https://lore.kernel.org/all/20230309232009.2938-1-william.gray@linaro.org/

William Breathitt Gray
Mark Brown March 15, 2023, 1:28 p.m. UTC | #3
On Mon, Feb 27, 2023 at 11:45:22AM -0500, William Breathitt Gray wrote:
> Some devices lack status registers, yet expect to handle interrupts.
> Introduce a no_status flag to indicate such a configuration, where
> rather than read a status register to verify, all interrupts received
> are assumed to be active.

The following changes since commit fe15c26ee26efa11741a7b632e9f23b01aca4cc6:

  Linux 6.3-rc1 (2023-03-05 14:52:03 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/regmap-no-status

for you to fetch changes up to 4d60cac951fd2dfbf261b83abb805748abc8b995:

  regmap-irq: Add no_status support (2023-03-05 23:38:50 +0000)

----------------------------------------------------------------
regmap: Add no_status support

This patch adds support for devices which don't support readback of
individual interrupt statuses, we report all interrupts as firing and
hope the consumers do the right thing.

----------------------------------------------------------------
William Breathitt Gray (1):
      regmap-irq: Add no_status support

 drivers/base/regmap/regmap-irq.c | 22 +++++++++++++++-------
 include/linux/regmap.h           |  2 ++
 2 files changed, 17 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index a8f185430a07..290e26664a21 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -437,7 +437,10 @@  static irqreturn_t regmap_irq_thread(int irq, void *d)
 	 * possible in order to reduce the I/O overheads.
 	 */
 
-	if (chip->num_main_regs) {
+	if (chip->no_status) {
+		/* no status register so default to all active */
+		memset32(data->status_buf, GENMASK(31, 0), chip->num_regs);
+	} else if (chip->num_main_regs) {
 		unsigned int max_main_bits;
 		unsigned long size;
 
@@ -967,12 +970,17 @@  int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 			continue;
 
 		/* Ack masked but set interrupts */
-		reg = d->get_irq_reg(d, d->chip->status_base, i);
-		ret = regmap_read(map, reg, &d->status_buf[i]);
-		if (ret != 0) {
-			dev_err(map->dev, "Failed to read IRQ status: %d\n",
-				ret);
-			goto err_alloc;
+		if (d->chip->no_status) {
+			/* no status register so default to all active */
+			d->status_buf[i] = GENMASK(31, 0);
+		} else {
+			reg = d->get_irq_reg(d, d->chip->status_base, i);
+			ret = regmap_read(map, reg, &d->status_buf[i]);
+			if (ret != 0) {
+				dev_err(map->dev, "Failed to read IRQ status: %d\n",
+					ret);
+				goto err_alloc;
+			}
 		}
 
 		if (chip->status_invert)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index a3bc695bcca0..c6d80d4e73de 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1564,6 +1564,7 @@  struct regmap_irq_chip_data;
  *		      the need for a @sub_reg_offsets table.
  * @status_invert: Inverted status register: cleared bits are active interrupts.
  * @runtime_pm:  Hold a runtime PM lock on the device when accessing it.
+ * @no_status: No status register: all interrupts assumed generated by device.
  *
  * @num_regs:    Number of registers in each control bank.
  * @irqs:        Descriptors for individual IRQs.  Interrupt numbers are
@@ -1630,6 +1631,7 @@  struct regmap_irq_chip {
 	unsigned int clear_on_unmask:1;
 	unsigned int not_fixed_stride:1;
 	unsigned int status_invert:1;
+	unsigned int no_status:1;
 
 	int num_regs;