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

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Mon, Feb 27, 2023 at 5:54 PM William Breathitt Gray
<william.gray@xxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: William Breathitt Gray <william.gray@xxxxxxxxxx>
> ---
> 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