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

[Mudit Sharma]

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