Re: [PATCH 3/3] iio: light: tcs3472: Use guard(mutex)() family over manual locking

On Wed, May 06, 2026 at 11:43:11AM +0200, Aldo Conte wrote:
> Convert tcs3472_read_event_config, tcs3472_write_event_config,
> tcs3472_write_event, tcs3472_powerdown and tcs3472_resume to use
> automatico cleanup with guard(mutex)() instead of the old manual
> locking method.

> Found by code inspection.

Huh?! Is it a bug? What is the inspection assumed here?

> Tested on a Raspberry Pi 3B with a TCS34725 at 0x29 address.
> The following fields were read and written without any issues:
> sampling_frequency, integration_time, calibscale and the
> threshold event interface.
> The unload of the driver works cleanly.

The comments should be in the cover letter and not in each of the commit
message, we do not want that noise here!

> Signed-off-by: Aldo Conte <[email protected]>

So, do not do ping-pong programming: when the code introduced in the series is
immediately being changed by another patch in the same series. As explained
this patch should be first in the series to solve that.

...

>  {
>  	struct tcs3472_data *data = iio_priv(indio_dev);
> -	int ret;
>  
> -	mutex_lock(&data->lock);
> -	ret = !!(data->enable & TCS3472_ENABLE_AIEN);
> -	mutex_unlock(&data->lock);
> -
> -	return ret;
> +	guard(mutex)(&data->lock);

Leave a blank line here.

> +	return !!(data->enable & TCS3472_ENABLE_AIEN);
>  }

...

> static int tcs3472_resume(struct device *dev)

>  	u8 enable_mask = TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON |
>  			TCS3472_ENABLE_WEN;

Taking into account the comment against patch 1, add a new patch to make this
follow the patter suggested there.

> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);
>  
>  	ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE,
>  		data->enable | enable_mask);
>  	if (!ret)
>  		data->enable |= enable_mask;
>  
> -	mutex_unlock(&data->lock);
> -
>  	return ret;
>  }

-- 
With Best Regards,
Andy Shevchenko