Re: [PATCH v5 2/2] iio: light: ROHM BH1745 colour sensor

[Mudit Sharma <muditsharma.info@gmail.com>]

On 24/06/2024 23:27, Javier Carrasco wrote:
> 
>> +static int bh1745_set_trigger_state(struct iio_trigger *trig, bool state)
>> +{
>> +	int ret;
> 
> Why is value initialized here? If regmap returns an error, you will not
> use value anyway. I caught my eye because it is initialized here, and
> not in the other functions where you use the same pattern.

Hi Javier,

Thank you for the review on this.

'value' is initialized here for case when we un-set the trigger. In that 
case, 'state' will be false and 'value' of 0 (default value for 
BH1745_INTR register) will be written.

Best regards,
Mudit Sharma
> 
>> +	int value = 0;
>> +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>> +	struct bh1745_data *data = iio_priv(indio_dev);
>> +
>> +	guard(mutex)(&data->lock);
>> +	if (state) {
>> +		ret = regmap_read(data->regmap, BH1745_INTR, &value);
>> +		if (ret)
>> +			return ret;
>> +		// Latch is always set when enabling interrupt
>> +		value |= BH1745_INT_ENABLE |
>> +			FIELD_PREP(BH1745_INT_SIGNAL_LATCHED, 1) |
>> +			FIELD_PREP(BH1745_INT_SOURCE_MASK, data->int_src);
>> +		return regmap_write(data->regmap, BH1745_INTR, value);
>> +	}
>> +
>> +	return regmap_write(data->regmap, BH1745_INTR, value);
>> +}
> 
> 
> Best regards,
> Javier Carrasco
>