Skip to contentSkip to conversation

ROHM BUxxx light sensor fixes

PATCH Applied Matti Vaittinen 2 participants 6 messages Jun 13, 2023 – Jun 17, 2023

Subsystems (2)
lore
Share & tools
lore

Matti Vaittinen

author maintainer
#0 lore raw

cover ROHM BUxxx light sensor fixes

A couple of fixes to newly introduced light sensor drivers.

The patch 1/N "iio: light: bu27034: Fix scale format" should be applied as a fix because the bu27034 is already merged upstream. Please note that there are already a few fixes to bu27034 in fixes-togreg.

The patches 2/N and 3/N can be taken in as "new features" because the bu27008 has not yet been merged upstream.

Please, let me know if you want me to split the series and rebase the bu27034 fix on top of the fixes-togreg (or some other base).

Matti Vaittinen (3):
iio: light: bu27034: Fix scale format
iio: light: bu27008: Fix scale format
iio: light: bu27008: Fix intensity data type

 drivers/iio/light/rohm-bu27008.c | 22 +++++++++++++++++++---
 drivers/iio/light/rohm-bu27034.c | 22 +++++++++++++++++++---
 2 files changed, 38 insertions(+), 6 deletions(-)

base-commit: 0cf9a77e80fa48e4392e408f8660e93c080cb78d

Signature · 14 lines
2.40.1


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
author maintainer
#1 lore raw

PATCH 1/3 iio: light: bu27034: Fix scale format

The driver is expecting accuracy of NANOs for intensity scale in raw_write. The IIO core is however defaulting to MICROs. This leads the raw-write of smallest scales to never succeed as correct selector(s) are not found.

Fix this by implementing the .write_raw_get_fmt callback to use NANO accuracy for writes of IIO_CHAN_INFO_SCALE.

Fixes e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor")
---
 drivers/iio/light/rohm-bu27034.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)
drivers/iio/light/rohm-bu27034.c +19 -3
index 846c3d43aa34..f737d62bdf92 100644
@@ -572,7 +572,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 		return -EINVAL;
 
 	if (chan == BU27034_CHAN_ALS) {
-		if (val == 0 && val2 == 1000)
+		if (val == 0 && val2 == 1000000)
 			return 0;
 
 		return -EINVAL;
@@ -584,7 +584,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 		goto unlock_out;
 
 	ret = iio_gts_find_gain_sel_for_scale_using_time(&data->gts, time_sel,
-						val, val2 * 1000, &gain_sel);
+						val, val2, &gain_sel);
 	if (ret) {
 		/*
 		 * Could not support scale with given time. Need to change time.
@@ -621,7 +621,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 
 			/* Can we provide requested scale with this time? */
 			ret = iio_gts_find_gain_sel_for_scale_using_time(
-				&data->gts, new_time_sel, val, val2 * 1000,
+				&data->gts, new_time_sel, val, val2,
 				&gain_sel);
 			if (ret)
 				continue;
@@ -1213,6 +1213,21 @@ static int bu27034_read_raw(struct iio_dev *idev,
 	}
 }
 
+static int bu27034_write_raw_get_fmt(struct iio_dev *indio_dev,
+				     struct iio_chan_spec const *chan,
+				     long mask)
+{
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_INT_TIME:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int bu27034_write_raw(struct iio_dev *idev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
@@ -1260,6 +1275,7 @@ static int bu27034_read_avail(struct iio_dev *idev,
 static const struct iio_info bu27034_info = {
 	.read_raw = &bu27034_read_raw,
 	.write_raw = &bu27034_write_raw,
+	.write_raw_get_fmt = &bu27034_write_raw_get_fmt,
 	.read_avail = &bu27034_read_avail,
 };
 
Signature · 14 lines
2.40.1


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
maintainer Applied
#2 lore raw

Re: [PATCH 1/3] iio: light: bu27034: Fix scale format

Quoted 10 lines · On Tue, 13 Jun 2023 12:34:36 +0300 Matti Vaittinen <[email protected]>…

The driver is expecting accuracy of NANOs for intensity scale in raw_write. The IIO core is however defaulting to MICROs. This leads the raw-write of smallest scales to never succeed as correct selector(s) are not found.

Fix this by implementing the .write_raw_get_fmt callback to use NANO accuracy for writes of IIO_CHAN_INFO_SCALE.

Fixes e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor")

Applied to the fixes-togreg branch of iio.git

Note however that the fixes-togreg branch has moved on to a 'post merge window' state as the last lot of fixes are in char-misc-next (not char-misc-linus) and hence targetting the merge window.

As such I will be holding these locally for a while as if I push them out it will make a mess of the ordering of branches merged into linux-next

Jonathan

Quoted 65 lines
---
 drivers/iio/light/rohm-bu27034.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)
drivers/iio/light/rohm-bu27034.c +19 -3
index 846c3d43aa34..f737d62bdf92 100644
@@ -572,7 +572,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 		return -EINVAL;
 
 	if (chan == BU27034_CHAN_ALS) {
-		if (val == 0 && val2 == 1000)
+		if (val == 0 && val2 == 1000000)
 			return 0;
 
 		return -EINVAL;
@@ -584,7 +584,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 		goto unlock_out;
 
 	ret = iio_gts_find_gain_sel_for_scale_using_time(&data->gts, time_sel,
-						val, val2 * 1000, &gain_sel);
+						val, val2, &gain_sel);
 	if (ret) {
 		/*
 		 * Could not support scale with given time. Need to change time.
@@ -621,7 +621,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 
 			/* Can we provide requested scale with this time? */
 			ret = iio_gts_find_gain_sel_for_scale_using_time(
-				&data->gts, new_time_sel, val, val2 * 1000,
+				&data->gts, new_time_sel, val, val2,
 				&gain_sel);
 			if (ret)
 				continue;
@@ -1213,6 +1213,21 @@ static int bu27034_read_raw(struct iio_dev *idev,
 	}
 }
 
+static int bu27034_write_raw_get_fmt(struct iio_dev *indio_dev,
+				     struct iio_chan_spec const *chan,
+				     long mask)
+{
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_INT_TIME:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int bu27034_write_raw(struct iio_dev *idev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
@@ -1260,6 +1275,7 @@ static int bu27034_read_avail(struct iio_dev *idev,
 static const struct iio_info bu27034_info = {
 	.read_raw = &bu27034_read_raw,
 	.write_raw = &bu27034_write_raw,
+	.write_raw_get_fmt = &bu27034_write_raw_get_fmt,
 	.read_avail = &bu27034_read_avail,
 };
 
author maintainer
#3 lore raw

PATCH 2/3 iio: light: bu27008: Fix scale format

In reply to Matti Vaittinen

The driver is expecting accuracy of NANOs for intensity scale in raw_write. The IIO core is however defaulting to MICROs. This leads the raw-write of smallest scales to never succeed as correct selector(s) are not found.

Fix this by implementing the .write_raw_get_fmt callback to use NANO accuracy for writes of IIO_CHAN_INFO_SCALE.

---
Fixes tag not added because AFACS the bu27008 is not yet in any releases.

---
 drivers/iio/light/rohm-bu27008.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
drivers/iio/light/rohm-bu27008.c +18 -2
index 489902bed7f0..80eb14ea8193 100644
@@ -633,7 +633,7 @@ static int bu27008_try_find_new_time_gain(struct bu27008_data *data, int val,
 	for (i = 0; i < data->gts.num_itime; i++) {
 		new_time_sel = data->gts.itime_table[i].sel;
 		ret = iio_gts_find_gain_sel_for_scale_using_time(&data->gts,
-					new_time_sel, val, val2 * 1000, gain_sel);
+					new_time_sel, val, val2, gain_sel);
 		if (!ret)
 			break;
 	}
@@ -662,7 +662,7 @@ static int bu27008_set_scale(struct bu27008_data *data,
 		goto unlock_out;
 
 	ret = iio_gts_find_gain_sel_for_scale_using_time(&data->gts, time_sel,
-						val, val2 * 1000, &gain_sel);
+						val, val2, &gain_sel);
 	if (ret) {
 		ret = bu27008_try_find_new_time_gain(data, val, val2, &gain_sel);
 		if (ret)
@@ -677,6 +677,21 @@ static int bu27008_set_scale(struct bu27008_data *data,
 	return ret;
 }
 
+static int bu27008_write_raw_get_fmt(struct iio_dev *indio_dev,
+				     struct iio_chan_spec const *chan,
+				     long mask)
+{
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_INT_TIME:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int bu27008_write_raw(struct iio_dev *idev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
@@ -756,6 +771,7 @@ static int bu27008_update_scan_mode(struct iio_dev *idev,
 static const struct iio_info bu27008_info = {
 	.read_raw = &bu27008_read_raw,
 	.write_raw = &bu27008_write_raw,
+	.write_raw_get_fmt = &bu27008_write_raw_get_fmt,
 	.read_avail = &bu27008_read_avail,
 	.update_scan_mode = bu27008_update_scan_mode,
 	.validate_trigger = iio_validate_own_trigger,
Signature · 14 lines
2.40.1


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
author maintainer
#4 lore raw

PATCH 3/3 iio: light: bu27008: Fix intensity data type

In reply to Matti Vaittinen

The intensity data from bu27008 is unsigned. The type of the scan data was incorrectly marked as signed resulting large intensity values to be interpreted as negative ones.

Fix the scan data type.

---
Fixes tag not added because AFACS the bu27008 is not yet in any releases.

---
 drivers/iio/light/rohm-bu27008.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/light/rohm-bu27008.c +1 -1
index 80eb14ea8193..b50bf8973d9a 100644
@@ -190,7 +190,7 @@ static const struct iio_itime_sel_mul bu27008_itimes[] = {
 	.address = BU27008_REG_##data##_LO,					\
 	.scan_index = BU27008_##color,						\
 	.scan_type = {								\
-		.sign = 's',							\
+		.sign = 'u',							\
 		.realbits = 16,							\
 		.storagebits = 16,						\
 		.endianness = IIO_LE,						\
Signature · 14 lines
2.40.1


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 
maintainer Applied
#5 lore raw

Re: [PATCH 3/3] iio: light: bu27008: Fix intensity data type

Quoted 7 lines · On Tue, 13 Jun 2023 12:35:12 +0300 Matti Vaittinen <[email protected]>…

The intensity data from bu27008 is unsigned. The type of the scan data was incorrectly marked as signed resulting large intensity values to be interpreted as negative ones.

Fix the scan data type.

Applied series to the fixes-togreg branch of iio.git (which now includes this as it's upstream in char-misc-next) Added fixes tags where needed given the ids are now stable

Thanks,
Jonathan

Quoted 20 lines

---
Fixes tag not added because AFACS the bu27008 is not yet in any releases.

---
 drivers/iio/light/rohm-bu27008.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/light/rohm-bu27008.c +1 -1
index 80eb14ea8193..b50bf8973d9a 100644
@@ -190,7 +190,7 @@ static const struct iio_itime_sel_mul bu27008_itimes[] = {
 	.address = BU27008_REG_##data##_LO,					\
 	.scan_index = BU27008_##color,						\
 	.scan_type = {								\
-		.sign = 's',							\
+		.sign = 'u',							\
 		.realbits = 16,							\
 		.storagebits = 16,						\
 		.endianness = IIO_LE,						\