MIME-Version: 1.0
References: <cover.1677515341.git.william.gray@linaro.org> <bd501b4b5ff88da24d467f75e8c71b4e0e6f21e2.1677515341.git.william.gray@linaro.org>
In-Reply-To: <bd501b4b5ff88da24d467f75e8c71b4e0e6f21e2.1677515341.git.william.gray@linaro.org>
From:   Bartosz Golaszewski <brgl@bgdev.pl>
Date:   Mon, 6 Mar 2023 11:11:26 +0100
Message-ID: <CAMRc=MdkkO4DpdLJA4SkEbAFFrdDtfZBOtLFPmkTBnSMDz=gCQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/6] regmap-irq: Add no_status support
To:     broonie@kernel.org
Cc:     linus.walleij@linaro.org, andriy.shevchenko@linux.intel.com,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        William Breathitt Gray <william.gray@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Feb 27, 2023 at 5:54=E2=80=AFPM 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/regma=
p-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 =3D d->get_irq_reg(d, d->chip->status_base, i);
> -               ret =3D regmap_read(map, reg, &d->status_buf[i]);
> -               if (ret !=3D 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] =3D GENMASK(31, 0);
> +               } else {
> +                       reg =3D d->get_irq_reg(d, d->chip->status_base, i=
);
> +                       ret =3D regmap_read(map, reg, &d->status_buf[i]);
> +                       if (ret !=3D 0) {
> +                               dev_err(map->dev, "Failed to read IRQ sta=
tus: %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 int=
errupts.
>   * @runtime_pm:  Hold a runtime PM lock on the device when accessing it.
> + * @no_status: No status register: all interrupts assumed generated by d=
evice.
>   *
>   * @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