Re: [PATCH 6.12.y-cip v2 03/10] pinctrl: renesas: rzt2h: Allow .get_direction() for IRQ function GPIOs

Hi!

> Hold the spinlock to ensure atomicity between reading the PMC register
> (which determines whether the pin is in GPIO mode or not) and reading
> the function of the pin when it is not in GPIO mode.

Ok, so according to this, two reads need to be protected.

> index 981a34dcc814e..f056e28f6a17e 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzt2h.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzt2h.c
> @@ -501,8 +503,25 @@ static int rzt2h_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
>  	if (ret)
>  		return ret;
>  
> -	if (rzt2h_pinctrl_readb(pctrl, port, PMC(port)) & BIT(bit))
> +	guard(spinlock_irqsave)(&pctrl->lock);
> +
> +	if (rzt2h_pinctrl_readb(pctrl, port, PMC(port)) & BIT(bit)) {
> +		/*
> +		 * When a GPIO is being requested as an IRQ, the pinctrl
> +		 * framework expects to be able to read the GPIO's direction.
> +		 * IRQ function is separate from GPIO, and enabling it takes the
> +		 * pin out of GPIO mode.
> +		 * At this point, .child_to_parent_hwirq() has already been
> +		 * called to enable the IRQ function.
> +		 * Default to input direction for IRQ function.
> +		 */
> +		reg64 = rzt2h_pinctrl_readq(pctrl, port, PFC(port));
> +		reg64 = (reg64 >> (bit * 8)) & PFC_MASK;
> +		if (reg64 == PFC_FUNC_INTERRUPT)
> +			return GPIO_LINE_DIRECTION_IN;
> +
>  		return -EINVAL;
> +	}
>  
>  	reg = rzt2h_pinctrl_readw(pctrl, port, PM(port));
>  	reg = (reg >> (bit * 2)) & PM_MASK;

But AFAICT here, whole rest of the function is running with spinlock
held/interrupts disabled. Is that neccessary?

Thanks and best regards,
								Pavel

-----BEGIN PGP SIGNATURE-----

iF0EABECAB0WIQRPfPO7r0eAhk010v0w5/Bqldv68gUCak1e2AAKCRAw5/Bqldv6
8tlCAJ98t4NIDUvErt88n7HkK8J04TVtCACgufGM4uwrg45eB6jQ2R7jA+cryng=
=t2Wl
-----END PGP SIGNATURE-----