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

Mudit Sharma
2024-06-25T19:25:04Z

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
>