mbox series

[v5,0/2] gpio: mxc: Locking and direction input fix

Message ID 20220725103445.88674-1-marex@denx.de
Headers show
Series gpio: mxc: Locking and direction input fix | expand

Message

Marek Vasut July 25, 2022, 10:34 a.m. UTC
The irqchip callbacks in gpio-mxc perform register read-modify-write operations
without locking, which may lead to a race condition. Add the missing locking.

In case the GPIO is used as IRQ, make sure it is configured as input.

Marek Vasut (2):
  gpio: mxc: Protect GPIO irqchip RMW with bgpio spinlock
  gpio: mxc: Always set GPIOs used as interrupt source to INPUT mode

 drivers/gpio/gpio-mxc.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Loic Poulain <loic.poulain@linaro.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Shawn Guo <shawnguo@kernel.org>

Comments

Marc Zyngier July 25, 2022, 3:50 p.m. UTC | #1
On Mon, 25 Jul 2022 11:34:43 +0100,
Marek Vasut <marex@denx.de> wrote:
> 
> The irqchip callbacks in gpio-mxc perform register read-modify-write operations
> without locking, which may lead to a race condition. Add the missing locking.
> 
> In case the GPIO is used as IRQ, make sure it is configured as input.
> 
> Marek Vasut (2):
>   gpio: mxc: Protect GPIO irqchip RMW with bgpio spinlock
>   gpio: mxc: Always set GPIOs used as interrupt source to INPUT mode
> 
>  drivers/gpio/gpio-mxc.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Loic Poulain <loic.poulain@linaro.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.
Linus Walleij July 25, 2022, 10:36 p.m. UTC | #2
On Mon, Jul 25, 2022 at 12:35 PM Marek Vasut <marex@denx.de> wrote:

> The driver currently performs register read-modify-write without locking
> in its irqchip part, this could lead to a race condition when configuring
> interrupt mode setting. Add the missing bgpio spinlock lock/unlock around
> the register read-modify-write.
>
> Fixes: 07bd1a6cc7cbb ("MXC arch: Add gpio support for the whole platform")
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Loic Poulain <loic.poulain@linaro.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> ---
> V3: New patch
> V4: Include linux/spinlock.h
> V5: Use raw_spinlock per 3c938cc5cebcb ("gpio: use raw spinlock for gpio chip shadowed data")

OK I was a bit confused this looks good and that spinlock is indeed raw.
There is maybe a bit of over-locking with one single lock for all registers
but I'm not very picky so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Linus Walleij Aug. 26, 2022, 8:40 a.m. UTC | #3
On Mon, Jul 25, 2022 at 12:35 PM Marek Vasut <marex@denx.de> wrote:

> The irqchip callbacks in gpio-mxc perform register read-modify-write operations
> without locking, which may lead to a race condition. Add the missing locking.
>
> In case the GPIO is used as IRQ, make sure it is configured as input.

Changes look good to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Marek Vasut Sept. 29, 2022, 5:35 p.m. UTC | #4
On 8/23/22 16:20, Marc Zyngier wrote:
> On 2022-08-23 11:41, Marek Vasut wrote:
>> On 7/25/22 17:50, Marc Zyngier wrote:
>>> On Mon, 25 Jul 2022 11:34:43 +0100,
>>> Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> The irqchip callbacks in gpio-mxc perform register read-modify-write 
>>>> operations
>>>> without locking, which may lead to a race condition. Add the missing 
>>>> locking.
>>>>
>>>> In case the GPIO is used as IRQ, make sure it is configured as input.
>>>>
>>>> Marek Vasut (2):
>>>>    gpio: mxc: Protect GPIO irqchip RMW with bgpio spinlock
>>>>    gpio: mxc: Always set GPIOs used as interrupt source to INPUT mode
>>>>
>>>>   drivers/gpio/gpio-mxc.c | 17 ++++++++++++++++-
>>>>   1 file changed, 16 insertions(+), 1 deletion(-)
>>>>
>>>> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>> Cc: Loic Poulain <loic.poulain@linaro.org>
>>>> Cc: Marc Zyngier <maz@kernel.org>
>>>> Cc: NXP Linux Team <linux-imx@nxp.com>
>>>> Cc: Peng Fan <peng.fan@nxp.com>
>>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>>
>>> Reviewed-by: Marc Zyngier <maz@kernel.org>
>>
>> Can these two now be applied or is there something more to do ?
> 
> That'd be question for Linus and Bartosz, I guess. From my
> own PoV, this is good to go.

So uh ... could either of you schedule those for I guess 6.2 please ?