Received-SPF: pass (google.com: domain of linux-kernel+bounces-8194-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20231219101620.4617-1-xiongxin@kylinos.cn> <20231220022901.29158-1-xiongxin@kylinos.cn>
In-Reply-To: <20231220022901.29158-1-xiongxin@kylinos.cn>
From: Bartosz Golaszewski <brgl@bgdev.pl>
Date: Thu, 21 Dec 2023 11:19:55 +0100
Message-ID: <CAMRc=Md7ODHUSe2fa37NazgJoV5aGXRh1Pp24LcFc49=UXgnwA@mail.gmail.com>
Subject: Re: [PATCH v5] gpio: dwapb: mask/unmask IRQ when disable/enale it
To: xiongxin <xiongxin@kylinos.cn>
Cc: fancer.lancer@gmail.com, hoan@os.amperecomputing.com, 
	linus.walleij@linaro.org, andy@kernel.org, linux-gpio@vger.kernel.org, 
	linux-kernel@vger.kernel.org, stable@kernel.org, 
	Riwen Lu <luriwen@kylinos.cn>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Dec 20, 2023 at 3:29=E2=80=AFAM xiongxin <xiongxin@kylinos.cn> wrot=
e:
>
> In the hardware implementation of the I2C HID driver based on DesignWare
> GPIO IRQ chip, when the user continues to use the I2C HID device in the
> suspend process, the I2C HID interrupt will be masked after the resume
> process is finished.
>
> This is because the disable_irq()/enable_irq() of the DesignWare GPIO
> driver does not synchronize the IRQ mask register state. In normal use
> of the I2C HID procedure, the GPIO IRQ irq_mask()/irq_unmask() functions
> are called in pairs. In case of an exception, i2c_hid_core_suspend()
> calls disable_irq() to disable the GPIO IRQ. With low probability, this
> causes irq_unmask() to not be called, which causes the GPIO IRQ to be
> masked and not unmasked in enable_irq(), raising an exception.
>
> Add synchronization to the masked register state in the
> dwapb_irq_enable()/dwapb_irq_disable() function. mask the GPIO IRQ
> before disabling it. After enabling the GPIO IRQ, unmask the IRQ.
>
> Fixes: 7779b3455697 ("gpio: add a driver for the Synopsys DesignWare APB =
GPIO block")
> Cc: stable@kernel.org
> Co-developed-by: Riwen Lu <luriwen@kylinos.cn>
> Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
> Signed-off-by: xiongxin <xiongxin@kylinos.cn>
> Acked-by: Serge Semin <fancer.lancer@gmail.com>
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> ---
> v5:
>         * fix typo in patch description
> v4:
>         * Add patch tag information
> v3:
>         * Modify the submitter's information
> v2:
>         * Resubmit the patch to fix this exception from the DesignWare
>           GPIO driver side
> v1:
>         * Resolve the exception from the IRQ core layer. (key point not
>           found correctly)
> ---
>  drivers/gpio/gpio-dwapb.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 4a4f61bf6c58..8c59332429c2 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -282,13 +282,15 @@ static void dwapb_irq_enable(struct irq_data *d)
>  {
>         struct gpio_chip *gc =3D irq_data_get_irq_chip_data(d);
>         struct dwapb_gpio *gpio =3D to_dwapb_gpio(gc);
> +       irq_hw_number_t hwirq =3D irqd_to_hwirq(d);
>         unsigned long flags;
>         u32 val;
>
>         raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
> -       val =3D dwapb_read(gpio, GPIO_INTEN);
> -       val |=3D BIT(irqd_to_hwirq(d));
> +       val =3D dwapb_read(gpio, GPIO_INTEN) | BIT(hwirq);
>         dwapb_write(gpio, GPIO_INTEN, val);
> +       val =3D dwapb_read(gpio, GPIO_INTMASK) & ~BIT(hwirq);
> +       dwapb_write(gpio, GPIO_INTMASK, val);
>         raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>  }
>
> @@ -296,12 +298,14 @@ static void dwapb_irq_disable(struct irq_data *d)
>  {
>         struct gpio_chip *gc =3D irq_data_get_irq_chip_data(d);
>         struct dwapb_gpio *gpio =3D to_dwapb_gpio(gc);
> +       irq_hw_number_t hwirq =3D irqd_to_hwirq(d);
>         unsigned long flags;
>         u32 val;
>
>         raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
> -       val =3D dwapb_read(gpio, GPIO_INTEN);
> -       val &=3D ~BIT(irqd_to_hwirq(d));
> +       val =3D dwapb_read(gpio, GPIO_INTMASK) | BIT(hwirq);
> +       dwapb_write(gpio, GPIO_INTMASK, val);
> +       val =3D dwapb_read(gpio, GPIO_INTEN) & ~BIT(hwirq);
>         dwapb_write(gpio, GPIO_INTEN, val);
>         raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>  }
> --
> 2.34.1
>

Queued for fixes, thanks!

Bartosz