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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux