Re: [PATCH v2 1/2] gpio: adnp: use lock guards for the I2C lock

[David Lechner <dlechner@baylibre.com>]

On 3/6/25 11:19 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Reduce the code complexity by using automatic lock guards with the I2C
> mutex.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---

...

> @@ -248,32 +232,24 @@ static irqreturn_t adnp_irq(int irq, void *data)
>  
>  	for (i = 0; i < num_regs; i++) {
>  		unsigned int base = i << adnp->reg_shift, bit;
> -		u8 changed, level, isr, ier;
> +		u8 changed, level = 0, isr = 0, ier = 0;
>  		unsigned long pending;
>  		int err;
>  
> -		mutex_lock(&adnp->i2c_lock);
> +		scoped_guard(mutex, &adnp->i2c_lock) {
> +			err = adnp_read(adnp, GPIO_PLR(adnp) + i, &level);
> +			if (err < 0)
> +				continue;

I think this unintentionally changes the flow here. scoped_guard() is a
for loop, so the continue now breaks out of the scoped_guard() rather
than the outer for loop.

>  
> -		err = adnp_read(adnp, GPIO_PLR(adnp) + i, &level);
> -		if (err < 0) {
> -			mutex_unlock(&adnp->i2c_lock);
> -			continue;
> +			err = adnp_read(adnp, GPIO_ISR(adnp) + i, &isr);
> +			if (err < 0)
> +				continue;
> +
> +			err = adnp_read(adnp, GPIO_IER(adnp) + i, &ier);
> +			if (err < 0)
> +				continue;
>  		}
>  
> -		err = adnp_read(adnp, GPIO_ISR(adnp) + i, &isr);
> -		if (err < 0) {
> -			mutex_unlock(&adnp->i2c_lock);
> -			continue;
> -		}
> -
> -		err = adnp_read(adnp, GPIO_IER(adnp) + i, &ier);
> -		if (err < 0) {
> -			mutex_unlock(&adnp->i2c_lock);
> -			continue;
> -		}
> -
> -		mutex_unlock(&adnp->i2c_lock);
> -
>  		/* determine pins that changed levels */
>  		changed = level ^ adnp->irq_level[i];
>