[PATCH v2 0/7] ROHM BU27034NUC to ROHM BU27034ANUC

[PATCH v2 0/7] ROHM BU27034NUC to ROHM BU27034ANUC

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]

As discussed here:
https://lore.kernel.org/all/ff8d6d14-6b48-4347-8525-e05eeb9721ff@gmail.com/

The ROHM BU27034NUC was cancelled before it entered mass-production. A
replacement was developed and named to BU27034ANUC. (Note the added
'A' in the model name). The BU27034ANUC has several changes that make
the old BU27034NUC driver unusable with it. I was told the old BU27034NUC
should not be encountered anywhere.

Hence, this series converts the rohm-bu27034.c to support the new
BU27034ANUC instead of the obsoleted BU27034NUC. Additionally, the
series adds a read-only entry for the HARDWAREGAIN to help understanding
what part of the scale is contributed by the gain, and what by the
integration time. This is useful when figuring out why some transitions
from one 'scale' to other are failing.

Revision history:
	v1 => v2:
		- Split the one large patch to patches 3 - 6 for easier
		  review. (Please let me know if you wish me to squash
		  them to one).
		- Introduce new compatible for the BU27034ANUC and drop
		  the old one.
		- Add styling fixes as suggested by Jonathan
		- Fix the lux calculation coefficient selection logic
	link to v1:
		https://lore.kernel.org/all/cover.1718013518.git.mazziesaccount@gmail.com/

---

Matti Vaittinen (7):
  dt-bindings: iio: BU27034 => BU27034ANUC
  dt-bindings: iio: rename bu27034 file
  bu27034: ROHM BU27034NUC to BU27034ANUC
  bu27034: ROHM BU27034NUC to BU27034ANUC drop data2
  bu27034: ROHM BU27034ANUC correct gains and times
  bu27034: ROHM BU27034ANUC correct lux calculation
  iio: bu27034: Add a read only HWARDWAREGAIN

 ...ohm,bu27034.yaml => rohm,bu27034anuc.yaml} |  11 +-
 drivers/iio/light/rohm-bu27034.c              | 343 +++++-------------
 2 files changed, 89 insertions(+), 265 deletions(-)
 rename Documentation/devicetree/bindings/iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml} (66%)


base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
-- 
2.45.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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 1/7] dt-bindings: iio: BU27034 => BU27034ANUC

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 3276 bytes --]

The BU27034NUC was cancelled before it entered mass production. It was
replaced by a new variant BU27034ANUC (note, added 'A'). The new
variant gained a few significant changes, like removal of the 3.rd data
channel and dropping some of the gain settings. This means that, from
software point of view these ICs are incompatible. Lux calculation based
on the data from the sensors needs to be done differently, and on the
BU27034ANUC the channel 3 data is missing. Also, the gain setting
differencies matter.

Unfortunately, the identification register was not changed so there is no
safe way for the software to distinguish the variants.

According to the ROHM HQ engineers, the old BU27034NUC should not be
encountered in the wild. Hence it makes sense to remove the support for
the old BU27034NUC and add support for the new BU27034ANUC. Change the
compatible in order to not load the incompatible old driver for new sensor
(or, if someone had the old sensor, the new driver for it).

Drop the compatible for old sensor which should not be in the wild and
add a new compatible for the new model with accurate model suffix
'anuc'.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
A patch renaming the file according to the new compatible will follow.
If renaming is not needed or appropriate, that patch can be dropped.

Revision history:
v2: New patch
---
 .../devicetree/bindings/iio/light/rohm,bu27034.yaml      | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml b/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
index 30a109a1bf3b..535bd18348ac 100644
--- a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
+++ b/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
@@ -4,20 +4,19 @@
 $id: http://devicetree.org/schemas/iio/light/rohm,bu27034.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: ROHM BU27034 ambient light sensor
+title: ROHM BU27034ANUC ambient light sensor
 
 maintainers:
   - Matti Vaittinen <mazziesaccount@gmail.com>
 
 description: |
-  ROHM BU27034 is an ambient light sesnor with 3 channels and 3 photo diodes
+  ROHM BU27034ANUC is an ambient light sesnor with 2 channels and 2 photo diodes
   capable of detecting a very wide range of illuminance. Typical application
   is adjusting LCD and backlight power of TVs and mobile phones.
-  https://fscdn.rohm.com/en/products/databook/datasheet/ic/sensor/light/bu27034nuc-e.pdf
 
 properties:
   compatible:
-    const: rohm,bu27034
+    const: rohm,bu27034anuc
 
   reg:
     maxItems: 1
@@ -37,7 +36,7 @@ examples:
       #size-cells = <0>;
 
       light-sensor@38 {
-        compatible = "rohm,bu27034";
+        compatible = "rohm,bu27034anuc";
         reg = <0x38>;
         vdd-supply = <&vdd>;
       };
-- 
2.45.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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 2/7] dt-bindings: iio: rename bu27034 file

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]

The BU27034NUC was cancelled before it entered mass production. It was
replaced by a new variant BU27034ANUC (note, added 'A'). The new
variant gained a few significant changes, like removal of the 3.rd data
channel and dropping some of the gain settings. This means that, from
software point of view these ICs are incompatible. Lux calculation based
on the data from the sensors needs to be done differently, and on the
BU27034ANUC the channel 3 data is missing. Also, the gain setting
differencies matter.

The old sensor should not be out there so the compatible was dropped and
a new compatible was added for the bu27034anuc. Move the yaml file so
the file name matches the binding and change the $id.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
v1 => v2:
- New patch
---
 .../iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml}      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename Documentation/devicetree/bindings/iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml} (92%)

diff --git a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml b/Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
similarity index 92%
rename from Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
rename to Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
index 535bd18348ac..fc3d826ed8ba 100644
--- a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
+++ b/Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/iio/light/rohm,bu27034.yaml#
+$id: http://devicetree.org/schemas/iio/light/rohm,bu27034anuc.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: ROHM BU27034ANUC ambient light sensor
-- 
2.45.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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 3/7] bu27034: ROHM BU27034NUC to BU27034ANUC

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]

The ROHM BU27034NUC was cancelled and BU27034ANUC is replacing this
sensor. These senors aren't compatible from the software point of view.

According to ROHM, the BU27034NUC was never mass-produced. Hence dropping
the BU27034NUC support and using this driver to support BU27034ANUC
should not be a problem to users. We however need to ensure than people
who use old kernel with the old BU27034NUC driver don't get the old
driver probed for the new sensor.

Prepare to use the BU27034NUC driver to support the new BU27034ANUC and
change the compatible to prevent probing the old driver with the new
sensor.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/iio/light/rohm-bu27034.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index bf3de853a811..f876bb21ffa5 100644
--- a/drivers/iio/light/rohm-bu27034.c
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -1,9 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * BU27034 ROHM Ambient Light Sensor
+ * BU27034ANUC ROHM Ambient Light Sensor
  *
  * Copyright (c) 2023, ROHM Semiconductor.
- * https://fscdn.rohm.com/en/products/databook/datasheet/ic/sensor/light/bu27034nuc-e.pdf
  */
 
 #include <linux/bitfield.h>
@@ -1507,7 +1506,7 @@ static int bu27034_probe(struct i2c_client *i2c)
 }
 
 static const struct of_device_id bu27034_of_match[] = {
-	{ .compatible = "rohm,bu27034" },
+	{ .compatible = "rohm,bu27034anuc" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, bu27034_of_match);
-- 
2.45.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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 4/7] bu27034: ROHM BU27034NUC to BU27034ANUC drop data2

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 10761 bytes --]

The ROHM BU27034NUC was cancelled and BU27034ANUC is replacing this
sensor. The BU27034ANUC does not have the data2 channel.

Drop the data2 channel.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
Revision history:
v1 => v2:
 - patch splitted
 - fixed empty lines in bu27034_get_gain_sel()
---
 drivers/iio/light/rohm-bu27034.c | 138 +++++++------------------------
 1 file changed, 28 insertions(+), 110 deletions(-)

diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index f876bb21ffa5..86b8771caff0 100644
--- a/drivers/iio/light/rohm-bu27034.c
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -29,17 +29,15 @@
 
 #define BU27034_REG_MODE_CONTROL2	0x42
 #define BU27034_MASK_D01_GAIN		GENMASK(7, 3)
-#define BU27034_MASK_D2_GAIN_HI		GENMASK(7, 6)
-#define BU27034_MASK_D2_GAIN_LO		GENMASK(2, 0)
 
 #define BU27034_REG_MODE_CONTROL3	0x43
 #define BU27034_REG_MODE_CONTROL4	0x44
 #define BU27034_MASK_MEAS_EN		BIT(0)
 #define BU27034_MASK_VALID		BIT(7)
+#define BU27034_NUM_HW_DATA_CHANS	2
 #define BU27034_REG_DATA0_LO		0x50
 #define BU27034_REG_DATA1_LO		0x52
-#define BU27034_REG_DATA2_LO		0x54
-#define BU27034_REG_DATA2_HI		0x55
+#define BU27034_REG_DATA1_HI		0x53
 #define BU27034_REG_MANUFACTURER_ID	0x92
 #define BU27034_REG_MAX BU27034_REG_MANUFACTURER_ID
 
@@ -87,12 +85,12 @@ enum {
 	BU27034_CHAN_ALS,
 	BU27034_CHAN_DATA0,
 	BU27034_CHAN_DATA1,
-	BU27034_CHAN_DATA2,
 	BU27034_NUM_CHANS
 };
 
 static const unsigned long bu27034_scan_masks[] = {
-	GENMASK(BU27034_CHAN_DATA2, BU27034_CHAN_ALS), 0
+	GENMASK(BU27034_CHAN_DATA1, BU27034_CHAN_DATA0),
+	GENMASK(BU27034_CHAN_DATA1, BU27034_CHAN_ALS), 0
 };
 
 /*
@@ -155,11 +153,10 @@ static const struct iio_itime_sel_mul bu27034_itimes[] = {
 	GAIN_SCALE_ITIME_US(55000, BU27034_MEAS_MODE_55MS, 1),
 };
 
-#define BU27034_CHAN_DATA(_name, _ch2)					\
+#define BU27034_CHAN_DATA(_name)					\
 {									\
 	.type = IIO_INTENSITY,						\
 	.channel = BU27034_CHAN_##_name,				\
-	.channel2 = (_ch2),						\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
 			      BIT(IIO_CHAN_INFO_SCALE),			\
 	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),	\
@@ -194,13 +191,12 @@ static const struct iio_chan_spec bu27034_channels[] = {
 	/*
 	 * The BU27034 DATA0 and DATA1 channels are both on the visible light
 	 * area (mostly). The data0 sensitivity peaks at 500nm, DATA1 at 600nm.
-	 * These wave lengths are pretty much on the border of colours making
-	 * these a poor candidates for R/G/B standardization. Hence they're both
-	 * marked as clear channels
+	 * These wave lengths are cyan(ish) and orange(ish), making these
+	 * sub-optiomal candidates for R/G/B standardization. Hence the
+	 * colour modifier is omitted.
 	 */
-	BU27034_CHAN_DATA(DATA0, IIO_MOD_LIGHT_CLEAR),
-	BU27034_CHAN_DATA(DATA1, IIO_MOD_LIGHT_CLEAR),
-	BU27034_CHAN_DATA(DATA2, IIO_MOD_LIGHT_IR),
+	BU27034_CHAN_DATA(DATA0),
+	BU27034_CHAN_DATA(DATA1),
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
@@ -214,20 +210,14 @@ struct bu27034_data {
 	struct mutex mutex;
 	struct iio_gts gts;
 	struct task_struct *task;
-	__le16 raw[3];
+	__le16 raw[BU27034_NUM_HW_DATA_CHANS];
 	struct {
 		u32 mlux;
-		__le16 channels[3];
+		__le16 channels[BU27034_NUM_HW_DATA_CHANS];
 		s64 ts __aligned(8);
 	} scan;
 };
 
-struct bu27034_result {
-	u16 ch0;
-	u16 ch1;
-	u16 ch2;
-};
-
 static const struct regmap_range bu27034_volatile_ranges[] = {
 	{
 		.range_min = BU27034_REG_SYSTEM_CONTROL,
@@ -237,7 +227,7 @@ static const struct regmap_range bu27034_volatile_ranges[] = {
 		.range_max = BU27034_REG_MODE_CONTROL4,
 	}, {
 		.range_min = BU27034_REG_DATA0_LO,
-		.range_max = BU27034_REG_DATA2_HI,
+		.range_max = BU27034_REG_DATA1_HI,
 	},
 };
 
@@ -249,7 +239,7 @@ static const struct regmap_access_table bu27034_volatile_regs = {
 static const struct regmap_range bu27034_read_only_ranges[] = {
 	{
 		.range_min = BU27034_REG_DATA0_LO,
-		.range_max = BU27034_REG_DATA2_HI,
+		.range_max = BU27034_REG_DATA1_HI,
 	}, {
 		.range_min = BU27034_REG_MANUFACTURER_ID,
 		.range_max = BU27034_REG_MANUFACTURER_ID,
@@ -278,41 +268,17 @@ struct bu27034_gain_check {
 
 static int bu27034_get_gain_sel(struct bu27034_data *data, int chan)
 {
+	int reg[] = {
+		[BU27034_CHAN_DATA0] = BU27034_REG_MODE_CONTROL2,
+		[BU27034_CHAN_DATA1] = BU27034_REG_MODE_CONTROL3,
+	};
 	int ret, val;
 
-	switch (chan) {
-	case BU27034_CHAN_DATA0:
-	case BU27034_CHAN_DATA1:
-	{
-		int reg[] = {
-			[BU27034_CHAN_DATA0] = BU27034_REG_MODE_CONTROL2,
-			[BU27034_CHAN_DATA1] = BU27034_REG_MODE_CONTROL3,
-		};
-		ret = regmap_read(data->regmap, reg[chan], &val);
-		if (ret)
-			return ret;
-
-		return FIELD_GET(BU27034_MASK_D01_GAIN, val);
-	}
-	case BU27034_CHAN_DATA2:
-	{
-		int d2_lo_bits = fls(BU27034_MASK_D2_GAIN_LO);
-
-		ret = regmap_read(data->regmap, BU27034_REG_MODE_CONTROL2, &val);
-		if (ret)
-			return ret;
+	ret = regmap_read(data->regmap, reg[chan], &val);
+	if (ret)
+		return ret;
 
-		/*
-		 * The data2 channel gain is composed by 5 non continuous bits
-		 * [7:6], [2:0]. Thus when we combine the 5-bit 'selector'
-		 * from register value we must right shift the high bits by 3.
-		 */
-		return FIELD_GET(BU27034_MASK_D2_GAIN_HI, val) << d2_lo_bits |
-		       FIELD_GET(BU27034_MASK_D2_GAIN_LO, val);
-	}
-	default:
-		return -EINVAL;
-	}
+	return FIELD_GET(BU27034_MASK_D01_GAIN, val);
 }
 
 static int bu27034_get_gain(struct bu27034_data *data, int chan, int *gain)
@@ -395,44 +361,9 @@ static int bu27034_write_gain_sel(struct bu27034_data *data, int chan, int sel)
 	};
 	int mask, val;
 
-	if (chan != BU27034_CHAN_DATA0 && chan != BU27034_CHAN_DATA1)
-		return -EINVAL;
-
 	val = FIELD_PREP(BU27034_MASK_D01_GAIN, sel);
-
 	mask = BU27034_MASK_D01_GAIN;
 
-	if (chan == BU27034_CHAN_DATA0) {
-		/*
-		 * We keep the same gain for channel 2 as we set for channel 0
-		 * We can't allow them to be individually controlled because
-		 * setting one will impact also the other. Also, if we don't
-		 * always update both gains we may result unsupported bit
-		 * combinations.
-		 *
-		 * This is not nice but this is yet another place where the
-		 * user space must be prepared to surprizes. Namely, see chan 2
-		 * gain changed when chan 0 gain is changed.
-		 *
-		 * This is not fatal for most users though. I don't expect the
-		 * channel 2 to be used in any generic cases - the intensity
-		 * values provided by the sensor for IR area are not openly
-		 * documented. Also, channel 2 is not used for visible light.
-		 *
-		 * So, if there is application which is written to utilize the
-		 * channel 2 - then it is probably specifically targeted to this
-		 * sensor and knows how to utilize those values. It is safe to
-		 * hope such user can also cope with the gain changes.
-		 */
-		mask |=  BU27034_MASK_D2_GAIN_LO;
-
-		/*
-		 * The D2 gain bits are directly the lowest bits of selector.
-		 * Just do add those bits to the value
-		 */
-		val |= sel & BU27034_MASK_D2_GAIN_LO;
-	}
-
 	return regmap_update_bits(data->regmap, reg[chan], mask, val);
 }
 
@@ -440,13 +371,6 @@ static int bu27034_set_gain(struct bu27034_data *data, int chan, int gain)
 {
 	int ret;
 
-	/*
-	 * We don't allow setting channel 2 gain as it messes up the
-	 * gain for channel 0 - which shares the high bits
-	 */
-	if (chan != BU27034_CHAN_DATA0 && chan != BU27034_CHAN_DATA1)
-		return -EINVAL;
-
 	ret = iio_gts_find_sel_by_gain(&data->gts, gain);
 	if (ret < 0)
 		return ret;
@@ -570,9 +494,6 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 	int ret, time_sel, gain_sel, i;
 	bool found = false;
 
-	if (chan == BU27034_CHAN_DATA2)
-		return -EINVAL;
-
 	if (chan == BU27034_CHAN_ALS) {
 		if (val == 0 && val2 == 1000000)
 			return 0;
@@ -597,9 +518,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 
 		/*
 		 * Populate information for the other channel which should also
-		 * maintain the scale. (Due to the HW limitations the chan2
-		 * gets the same gain as chan0, so we only need to explicitly
-		 * set the chan 0 and 1).
+		 * maintain the scale.
 		 */
 		if (chan == BU27034_CHAN_DATA0)
 			gain.chan = BU27034_CHAN_DATA1;
@@ -613,7 +532,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 		/*
 		 * Iterate through all the times to see if we find one which
 		 * can support requested scale for requested channel, while
-		 * maintaining the scale for other channels
+		 * maintaining the scale for the other channel
 		 */
 		for (i = 0; i < data->gts.num_itime; i++) {
 			new_time_sel = data->gts.itime_table[i].sel;
@@ -628,7 +547,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 			if (ret)
 				continue;
 
-			/* Can the other channel(s) maintain scale? */
+			/* Can the other channel maintain scale? */
 			ret = iio_gts_find_new_gain_sel_by_old_gain_time(
 				&data->gts, gain.old_gain, time_sel,
 				new_time_sel, &gain.new_gain);
@@ -640,7 +559,7 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 		}
 		if (!found) {
 			dev_dbg(data->dev,
-				"Can't set scale maintaining other channels\n");
+				"Can't set scale maintaining other channel\n");
 			ret = -EINVAL;
 
 			goto unlock_out;
@@ -972,7 +891,6 @@ static int bu27034_read_result(struct bu27034_data *data, int chan, int *res)
 	int reg[] = {
 		[BU27034_CHAN_DATA0] = BU27034_REG_DATA0_LO,
 		[BU27034_CHAN_DATA1] = BU27034_REG_DATA1_LO,
-		[BU27034_CHAN_DATA2] = BU27034_REG_DATA2_LO,
 	};
 	int valid, ret;
 	__le16 val;
@@ -1039,7 +957,7 @@ static int bu27034_get_single_result(struct bu27034_data *data, int chan,
 {
 	int ret;
 
-	if (chan < BU27034_CHAN_DATA0 || chan > BU27034_CHAN_DATA2)
+	if (chan < BU27034_CHAN_DATA0 || chan > BU27034_CHAN_DATA1)
 		return -EINVAL;
 
 	ret = bu27034_meas_set(data, true);
@@ -1138,7 +1056,7 @@ static int bu27034_calc_mlux(struct bu27034_data *data, __le16 *res, int *val)
 
 static int bu27034_get_mlux(struct bu27034_data *data, int chan, int *val)
 {
-	__le16 res[3];
+	__le16 res[BU27034_NUM_HW_DATA_CHANS];
 	int ret;
 
 	ret = bu27034_meas_set(data, true);
-- 
2.45.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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 5/7] bu27034: ROHM BU27034ANUC correct gains and times

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 3696 bytes --]

The ROHM BU27034NUC was cancelled and BU27034ANUC is replacing this
sensor. The BU27034ANUC does not support all the gains or all the
integration times that were supported on BU27034NUC.

Srop unsupported times and gains

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
v1 => v2:
 - splitted from a big patch, no real changes
---
 drivers/iio/light/rohm-bu27034.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index 86b8771caff0..850ec78f5405 100644
--- a/drivers/iio/light/rohm-bu27034.c
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -94,49 +94,39 @@ static const unsigned long bu27034_scan_masks[] = {
 };
 
 /*
- * Available scales with gain 1x - 4096x, timings 55, 100, 200, 400 mS
+ * Available scales with gain 1x - 1024x, timings 55, 100, 200, 400 mS
  * Time impacts to gain: 1x, 2x, 4x, 8x.
  *
- * => Max total gain is HWGAIN * gain by integration time (8 * 4096) = 32768
+ * => Max total gain is HWGAIN * gain by integration time (8 * 1024) = 8192
+ * if 1x gain is scale 1, scale for 2x gain is 0.5, 4x => 0.25,
+ * ... 8192x => 0.0001220703125 => 122070.3125 nanos
  *
- * Using NANO precision for scale we must use scale 64x corresponding gain 1x
- * to avoid precision loss. (32x would result scale 976 562.5(nanos).
+ * Using NANO precision for scale, we must use scale 16x corresponding gain 1x
+ * to avoid precision loss. (8x would result scale 976 562.5(nanos).
  */
-#define BU27034_SCALE_1X	64
+#define BU27034_SCALE_1X	16
 
 /* See the data sheet for the "Gain Setting" table */
 #define BU27034_GSEL_1X		0x00 /* 00000 */
 #define BU27034_GSEL_4X		0x08 /* 01000 */
-#define BU27034_GSEL_16X	0x0a /* 01010 */
 #define BU27034_GSEL_32X	0x0b /* 01011 */
-#define BU27034_GSEL_64X	0x0c /* 01100 */
 #define BU27034_GSEL_256X	0x18 /* 11000 */
 #define BU27034_GSEL_512X	0x19 /* 11001 */
 #define BU27034_GSEL_1024X	0x1a /* 11010 */
-#define BU27034_GSEL_2048X	0x1b /* 11011 */
-#define BU27034_GSEL_4096X	0x1c /* 11100 */
 
 /* Available gain settings */
 static const struct iio_gain_sel_pair bu27034_gains[] = {
 	GAIN_SCALE_GAIN(1, BU27034_GSEL_1X),
 	GAIN_SCALE_GAIN(4, BU27034_GSEL_4X),
-	GAIN_SCALE_GAIN(16, BU27034_GSEL_16X),
 	GAIN_SCALE_GAIN(32, BU27034_GSEL_32X),
-	GAIN_SCALE_GAIN(64, BU27034_GSEL_64X),
 	GAIN_SCALE_GAIN(256, BU27034_GSEL_256X),
 	GAIN_SCALE_GAIN(512, BU27034_GSEL_512X),
 	GAIN_SCALE_GAIN(1024, BU27034_GSEL_1024X),
-	GAIN_SCALE_GAIN(2048, BU27034_GSEL_2048X),
-	GAIN_SCALE_GAIN(4096, BU27034_GSEL_4096X),
 };
 
 /*
- * The IC has 5 modes for sampling time. 5 mS mode is exceptional as it limits
- * the data collection to data0-channel only and cuts the supported range to
- * 10 bit. It is not supported by the driver.
- *
- * "normal" modes are 55, 100, 200 and 400 mS modes - which do have direct
- * multiplying impact to the register values (similar to gain).
+ * Measurement modes are 55, 100, 200 and 400 mS modes - which do have direct
+ * multiplying impact to the data register values (similar to gain).
  *
  * This means that if meas-mode is changed for example from 400 => 200,
  * the scale is doubled. Eg, time impact to total gain is x1, x2, x4, x8.
-- 
2.45.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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 6/7] bu27034: ROHM BU27034ANUC correct lux calculation

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 8651 bytes --]

The ROHM BU27034NUC was cancelled and BU27034ANUC is replacing this
sensor. The lux computation based on the data from a BU27034ANUC is
different from the computation for the data from an old BU27034NUC.

Fix the lux computation.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
v1 => v2:
 - splitted from a big patch
 - fix maths in comments to follow the coding style
 - fix the D1 / D0 ratio logic when picking the right coefficients for
   Lux calculation
---
 drivers/iio/light/rohm-bu27034.c | 157 ++++++-------------------------
 1 file changed, 31 insertions(+), 126 deletions(-)

diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index 850ec78f5405..ec4f9bef83f8 100644
--- a/drivers/iio/light/rohm-bu27034.c
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -573,102 +573,21 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
 }
 
 /*
- * for (D1/D0 < 0.87):
- * lx = 0.004521097 * D1 - 0.002663996 * D0 +
- *	0.00012213 * D1 * D1 / D0
+ * for (D1/D0 < 1.5):
+ *    lx = (0.001193 * D0 + (-0.0000747) * D1) * ((D1/D0 – 1.5) * (0.25) + 1)
  *
- * =>	115.7400832 * ch1 / gain1 / mt -
- *	68.1982976 * ch0 / gain0 / mt +
- *	0.00012213 * 25600 * (ch1 / gain1 / mt) * 25600 *
- *	(ch1 /gain1 / mt) / (25600 * ch0 / gain0 / mt)
+ *    => -0.000745625 * D0 + 0.0002515625 * D1 + -0.000018675 * D1 * D1 / D0
  *
- * A =	0.00012213 * 25600 * (ch1 /gain1 / mt) * 25600 *
- *	(ch1 /gain1 / mt) / (25600 * ch0 / gain0 / mt)
- * =>	0.00012213 * 25600 * (ch1 /gain1 / mt) *
- *	(ch1 /gain1 / mt) / (ch0 / gain0 / mt)
- * =>	0.00012213 * 25600 * (ch1 / gain1) * (ch1 /gain1 / mt) /
- *	(ch0 / gain0)
- * =>	0.00012213 * 25600 * (ch1 / gain1) * (ch1 /gain1 / mt) *
- *	gain0 / ch0
- * =>	3.126528 * ch1 * ch1 * gain0 / gain1 / gain1 / mt /ch0
+ *    => (6.44 * ch1 / gain1 + 19.088 * ch0 / gain0 -
+ *       0.47808 * ch1 * ch1 * gain0 / gain1 / gain1 / ch0) /
+ *       mt
  *
- * lx = (115.7400832 * ch1 / gain1 - 68.1982976 * ch0 / gain0) /
- *	mt + A
- * =>	(115.7400832 * ch1 / gain1 - 68.1982976 * ch0 / gain0) /
- *	mt + 3.126528 * ch1 * ch1 * gain0 / gain1 / gain1 / mt /
- *	ch0
+ * Else
+ *    lx = 0.001193 * D0 - 0.0000747 * D1
  *
- * =>	(115.7400832 * ch1 / gain1 - 68.1982976 * ch0 / gain0 +
- *	  3.126528 * ch1 * ch1 * gain0 / gain1 / gain1 / ch0) /
- *	  mt
- *
- * For (0.87 <= D1/D0 < 1.00)
- * lx = (0.001331* D0 + 0.0000354 * D1) * ((D1/D0 – 0.87) * (0.385) + 1)
- * =>	(0.001331 * 256 * 100 * ch0 / gain0 / mt + 0.0000354 * 256 *
- *	100 * ch1 / gain1 / mt) * ((D1/D0 -  0.87) * (0.385) + 1)
- * =>	(34.0736 * ch0 / gain0 / mt + 0.90624 * ch1 / gain1 / mt) *
- *	((D1/D0 -  0.87) * (0.385) + 1)
- * =>	(34.0736 * ch0 / gain0 / mt + 0.90624 * ch1 / gain1 / mt) *
- *	(0.385 * D1/D0 - 0.66505)
- * =>	(34.0736 * ch0 / gain0 / mt + 0.90624 * ch1 / gain1 / mt) *
- *	(0.385 * 256 * 100 * ch1 / gain1 / mt / (256 * 100 * ch0 / gain0 / mt) - 0.66505)
- * =>	(34.0736 * ch0 / gain0 / mt + 0.90624 * ch1 / gain1 / mt) *
- *	(9856 * ch1 / gain1 / mt / (25600 * ch0 / gain0 / mt) + 0.66505)
- * =>	13.118336 * ch1 / (gain1 * mt)
- *	+ 22.66064768 * ch0 / (gain0 * mt)
- *	+ 8931.90144 * ch1 * ch1 * gain0 /
- *	  (25600 * ch0 * gain1 * gain1 * mt)
- *	+ 0.602694912 * ch1 / (gain1 * mt)
- *
- * =>	[0.3489024 * ch1 * ch1 * gain0 / (ch0 * gain1 * gain1)
- *	 + 22.66064768 * ch0 / gain0
- *	 + 13.721030912 * ch1 / gain1
- *	] / mt
- *
- * For (D1/D0 >= 1.00)
- *
- * lx	= (0.001331* D0 + 0.0000354 * D1) * ((D1/D0 – 2.0) * (-0.05) + 1)
- *	=> (0.001331* D0 + 0.0000354 * D1) * (-0.05D1/D0 + 1.1)
- *	=> (0.001331 * 256 * 100 * ch0 / gain0 / mt + 0.0000354 * 256 *
- *	   100 * ch1 / gain1 / mt) * (-0.05D1/D0 + 1.1)
- *	=> (34.0736 * ch0 / gain0 / mt + 0.90624 * ch1 / gain1 / mt) *
- *	   (-0.05 * 256 * 100 * ch1 / gain1 / mt / (256 * 100 * ch0 / gain0 / mt) + 1.1)
- *	=> (34.0736 * ch0 / gain0 / mt + 0.90624 * ch1 / gain1 / mt) *
- *	   (-1280 * ch1 / (gain1 * mt * 25600 * ch0 / gain0 / mt) + 1.1)
- *	=> (34.0736 * ch0 * -1280 * ch1 * gain0 * mt /( gain0 * mt * gain1 * mt * 25600 * ch0)
- *	    + 34.0736 * 1.1 * ch0 / (gain0 * mt)
- *	    + 0.90624 * ch1 * -1280 * ch1 *gain0 * mt / (gain1 * mt *gain1 * mt * 25600 * ch0)
- *	    + 1.1 * 0.90624 * ch1 / (gain1 * mt)
- *	=> -43614.208 * ch1 / (gain1 * mt * 25600)
- *	    + 37.48096  ch0 / (gain0 * mt)
- *	    - 1159.9872 * ch1 * ch1 * gain0 / (gain1 * gain1 * mt * 25600 * ch0)
- *	    + 0.996864 ch1 / (gain1 * mt)
- *	=> [
- *		- 0.045312 * ch1 * ch1 * gain0 / (gain1 * gain1 * ch0)
- *		- 0.706816 * ch1 / gain1
- *		+ 37.48096  ch0 /gain0
- *	   ] * mt
- *
- *
- * So, the first case (D1/D0 < 0.87) can be computed to a form:
- *
- * lx = (3.126528 * ch1 * ch1 * gain0 / (ch0 * gain1 * gain1) +
- *	 115.7400832 * ch1 / gain1 +
- *	-68.1982976 * ch0 / gain0
- *	 / mt
- *
- * Second case (0.87 <= D1/D0 < 1.00) goes to form:
- *
- *	=> [0.3489024 * ch1 * ch1 * gain0 / (ch0 * gain1 * gain1) +
- *	    13.721030912 * ch1 / gain1 +
- *	    22.66064768 * ch0 / gain0
- *	   ] / mt
- *
- * Third case (D1/D0 >= 1.00) goes to form:
- *	=> [-0.045312 * ch1 * ch1 * gain0 / (ch0 * gain1 * gain1) +
- *	    -0.706816 * ch1 / gain1 +
- *	    37.48096  ch0 /(gain0
- *	   ] / mt
+ *    => (1.91232 * ch1 / gain1 + 30.5408 * ch0 / gain0 +
+ *        [0 * ch1 * ch1 * gain0 / gain1 / gain1 / ch0] ) /
+ *        mt
  *
  * This can be unified to format:
  * lx = [
@@ -678,19 +597,14 @@ static int bu27034_set_scale(struct bu27034_data *data, int chan,
  *	] / mt
  *
  * For case 1:
- * A = 3.126528,
- * B = 115.7400832
- * C = -68.1982976
+ * A = -0.47808,
+ * B = 6.44,
+ * C = 19.088
  *
  * For case 2:
- * A = 0.3489024
- * B = 13.721030912
- * C = 22.66064768
- *
- * For case 3:
- * A = -0.045312
- * B = -0.706816
- * C = 37.48096
+ * A = 0
+ * B = 1.91232
+ * C = 30.5408
  */
 
 struct bu27034_lx_coeff {
@@ -795,21 +709,16 @@ static int bu27034_fixp_calc_lx(unsigned int ch0, unsigned int ch1,
 {
 	static const struct bu27034_lx_coeff coeff[] = {
 		{
-			.A = 31265280,		/* 3.126528 */
-			.B = 1157400832,	/*115.7400832 */
-			.C = 681982976,		/* -68.1982976 */
-			.is_neg = {false, false, true},
+			.A = 4780800,		/* -0.47808 */
+			.B = 64400000,		/* 6.44 */
+			.C = 190880000,		/* 19.088 */
+			.is_neg = { true, false, false },
 		}, {
-			.A = 3489024,		/* 0.3489024 */
-			.B = 137210309,		/* 13.721030912 */
-			.C = 226606476,		/* 22.66064768 */
+			.A = 0,			/* 0 */
+			.B = 19123200,		/* 1.91232 */
+			.C = 305408000,		/* 30.5408 */
 			/* All terms positive */
-		}, {
-			.A = 453120,		/* -0.045312 */
-			.B = 7068160,		/* -0.706816 */
-			.C = 374809600,		/* 37.48096 */
-			.is_neg = {true, true, false},
-		}
+		},
 	};
 	const struct bu27034_lx_coeff *c = &coeff[coeff_idx];
 	u64 res = 0, terms[3];
@@ -972,12 +881,10 @@ static int bu27034_get_single_result(struct bu27034_data *data, int chan,
  * D1 = data1/ch1_gain/meas_time_ms * 25600
  *
  * Then:
- * if (D1/D0 < 0.87)
- *	lx = (0.001331 * D0 + 0.0000354 * D1) * ((D1 / D0 - 0.87) * 3.45 + 1)
- * else if (D1/D0 < 1)
- *	lx = (0.001331 * D0 + 0.0000354 * D1) * ((D1 / D0 - 0.87) * 0.385 + 1)
- * else
- *	lx = (0.001331 * D0 + 0.0000354 * D1) * ((D1 / D0 - 2) * -0.05 + 1)
+ * If (D1/D0 < 1.5)
+ *    lx = (0.001193 * D0 + (-0.0000747) * D1) * ((D1 / D0 – 1.5) * 0.25 + 1)
+ * Else
+ *    lx = (0.001193 * D0 + (-0.0000747) * D1)
  *
  * We use it here. Users who have for example some colored lens
  * need to modify the calculation but I hope this gives a starting point for
@@ -1028,12 +935,10 @@ static int bu27034_calc_mlux(struct bu27034_data *data, __le16 *res, int *val)
 		d1_d0_ratio_scaled /= ch0 * gain1;
 	}
 
-	if (d1_d0_ratio_scaled < 87)
+	if (d1_d0_ratio_scaled < 150)
 		ret = bu27034_fixp_calc_lx(ch0, ch1, gain0, gain1, meastime, 0);
-	else if (d1_d0_ratio_scaled < 100)
-		ret = bu27034_fixp_calc_lx(ch0, ch1, gain0, gain1, meastime, 1);
 	else
-		ret = bu27034_fixp_calc_lx(ch0, ch1, gain0, gain1, meastime, 2);
+		ret = bu27034_fixp_calc_lx(ch0, ch1, gain0, gain1, meastime, 1);
 
 	if (ret < 0)
 		return ret;
-- 
2.45.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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 7/7] iio: bu27034: Add a read only HWARDWAREGAIN

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 3136 bytes --]

The ROHM BU27034 light sensor has two data channels for measuring
different frequencies of light. The result from these channels is
combined into Lux value while the raw channel values are reported via
intensity channels.

Both of the intensity channels have adjustable gain setting which
impacts the scale of the raw channels. Eg, doubling the gain will double
the values read from the raw channels, which halves the scale value. The
integration time can also be set for the sensor. This does also have an
impact to the scale of the intensity channels because increasing the
integration time will also increase the values reported via the raw
channels.

Impact of integration time to the scale and the fact that the scale value
does not start from '1', can make it hard for a human reader to compute the
gain values based on the scale.

Add read-only HARDWAREGAIN to help debugging.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
Revision history:
v1 => v2:
 - fix switch case fallthrough warning by adding explicit return
---
 drivers/iio/light/rohm-bu27034.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index ec4f9bef83f8..76711c3cdf7c 100644
--- a/drivers/iio/light/rohm-bu27034.c
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -148,7 +148,8 @@ static const struct iio_itime_sel_mul bu27034_itimes[] = {
 	.type = IIO_INTENSITY,						\
 	.channel = BU27034_CHAN_##_name,				\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
-			      BIT(IIO_CHAN_INFO_SCALE),			\
+			      BIT(IIO_CHAN_INFO_SCALE) |		\
+			      BIT(IIO_CHAN_INFO_HARDWAREGAIN),		\
 	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),	\
 	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),		\
 	.info_mask_shared_by_all_available =				\
@@ -989,6 +990,13 @@ static int bu27034_read_raw(struct iio_dev *idev,
 
 		return IIO_VAL_INT_PLUS_MICRO;
 
+	case IIO_CHAN_INFO_HARDWAREGAIN:
+		ret = bu27034_get_gain(data, chan->channel, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_SCALE:
 		return bu27034_get_scale(data, chan->channel, val, val2);
 
@@ -1033,12 +1041,17 @@ static int bu27034_write_raw_get_fmt(struct iio_dev *indio_dev,
 				     struct iio_chan_spec const *chan,
 				     long mask)
 {
+	struct bu27034_data *data = iio_priv(indio_dev);
 
 	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;
+	case IIO_CHAN_INFO_HARDWAREGAIN:
+		dev_dbg(data->dev,
+			"HARDWAREGAIN is read-only, use scale to set\n");
+		return -EINVAL;
 	default:
 		return -EINVAL;
 	}
-- 
2.45.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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/7] dt-bindings: iio: BU27034 => BU27034ANUC

[Jonathan Cameron]

On Fri, 5 Jul 2024 13:54:12 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> The BU27034NUC was cancelled before it entered mass production. It was
> replaced by a new variant BU27034ANUC (note, added 'A'). The new
> variant gained a few significant changes, like removal of the 3.rd data
> channel and dropping some of the gain settings. This means that, from
> software point of view these ICs are incompatible. Lux calculation based
> on the data from the sensors needs to be done differently, and on the
> BU27034ANUC the channel 3 data is missing. Also, the gain setting
> differencies matter.
> 
> Unfortunately, the identification register was not changed so there is no
> safe way for the software to distinguish the variants.
> 
> According to the ROHM HQ engineers, the old BU27034NUC should not be
> encountered in the wild. Hence it makes sense to remove the support for
> the old BU27034NUC and add support for the new BU27034ANUC. Change the
> compatible in order to not load the incompatible old driver for new sensor
> (or, if someone had the old sensor, the new driver for it).
> 
> Drop the compatible for old sensor which should not be in the wild and
> add a new compatible for the new model with accurate model suffix
> 'anuc'.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Rename indeed makes sense.  One minor, 'whilst you are here' comment inline.

> 
> ---
> A patch renaming the file according to the new compatible will follow.
> If renaming is not needed or appropriate, that patch can be dropped.
> 
> Revision history:
> v2: New patch
> ---
>  .../devicetree/bindings/iio/light/rohm,bu27034.yaml      | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml b/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
> index 30a109a1bf3b..535bd18348ac 100644
> --- a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
> @@ -4,20 +4,19 @@
>  $id: http://devicetree.org/schemas/iio/light/rohm,bu27034.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: ROHM BU27034 ambient light sensor
> +title: ROHM BU27034ANUC ambient light sensor
>  
>  maintainers:
>    - Matti Vaittinen <mazziesaccount@gmail.com>
>  
>  description: |
> -  ROHM BU27034 is an ambient light sesnor with 3 channels and 3 photo diodes
> +  ROHM BU27034ANUC is an ambient light sesnor with 2 channels and 2 photo diodes

 sensor

>    capable of detecting a very wide range of illuminance. Typical application
>    is adjusting LCD and backlight power of TVs and mobile phones.
> -  https://fscdn.rohm.com/en/products/databook/datasheet/ic/sensor/light/bu27034nuc-e.pdf
>  
>  properties:
>    compatible:
> -    const: rohm,bu27034
> +    const: rohm,bu27034anuc
>  
>    reg:
>      maxItems: 1
> @@ -37,7 +36,7 @@ examples:
>        #size-cells = <0>;
>  
>        light-sensor@38 {
> -        compatible = "rohm,bu27034";
> +        compatible = "rohm,bu27034anuc";
>          reg = <0x38>;
>          vdd-supply = <&vdd>;
>        };

Re: [PATCH v2 7/7] iio: bu27034: Add a read only HWARDWAREGAIN

[Jonathan Cameron]

On Fri, 5 Jul 2024 13:55:49 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

Typo in patch title. If nothing much else comes up I can fix whilst applying.

> The ROHM BU27034 light sensor has two data channels for measuring
> different frequencies of light. The result from these channels is
> combined into Lux value while the raw channel values are reported via
> intensity channels.
> 
> Both of the intensity channels have adjustable gain setting which
> impacts the scale of the raw channels. Eg, doubling the gain will double
> the values read from the raw channels, which halves the scale value. The
> integration time can also be set for the sensor. This does also have an
> impact to the scale of the intensity channels because increasing the
> integration time will also increase the values reported via the raw
> channels.
> 
> Impact of integration time to the scale and the fact that the scale value
> does not start from '1', can make it hard for a human reader to compute the
> gain values based on the scale.
> 
> Add read-only HARDWAREGAIN to help debugging.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

Rest of this series looks good to me and I'm fine making appropriate tweaks
for stuff I identified whilst applying.

Will give a bit of time for DT maintainers to look at the renames and check
we haven't missed anything subtle there.

Jonathan


> 
> ---
> Revision history:
> v1 => v2:
>  - fix switch case fallthrough warning by adding explicit return
> ---
>  drivers/iio/light/rohm-bu27034.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
> index ec4f9bef83f8..76711c3cdf7c 100644
> --- a/drivers/iio/light/rohm-bu27034.c
> +++ b/drivers/iio/light/rohm-bu27034.c
> @@ -148,7 +148,8 @@ static const struct iio_itime_sel_mul bu27034_itimes[] = {
>  	.type = IIO_INTENSITY,						\
>  	.channel = BU27034_CHAN_##_name,				\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> -			      BIT(IIO_CHAN_INFO_SCALE),			\
> +			      BIT(IIO_CHAN_INFO_SCALE) |		\
> +			      BIT(IIO_CHAN_INFO_HARDWAREGAIN),		\
>  	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),	\
>  	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),		\
>  	.info_mask_shared_by_all_available =				\
> @@ -989,6 +990,13 @@ static int bu27034_read_raw(struct iio_dev *idev,
>  
>  		return IIO_VAL_INT_PLUS_MICRO;
>  
> +	case IIO_CHAN_INFO_HARDWAREGAIN:
> +		ret = bu27034_get_gain(data, chan->channel, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +
>  	case IIO_CHAN_INFO_SCALE:
>  		return bu27034_get_scale(data, chan->channel, val, val2);
>  
> @@ -1033,12 +1041,17 @@ static int bu27034_write_raw_get_fmt(struct iio_dev *indio_dev,
>  				     struct iio_chan_spec const *chan,
>  				     long mask)
>  {
> +	struct bu27034_data *data = iio_priv(indio_dev);
>  
>  	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;
> +	case IIO_CHAN_INFO_HARDWAREGAIN:
> +		dev_dbg(data->dev,
> +			"HARDWAREGAIN is read-only, use scale to set\n");
> +		return -EINVAL;
>  	default:
>  		return -EINVAL;
>  	}

Re: [PATCH v2 1/7] dt-bindings: iio: BU27034 => BU27034ANUC

[Matti Vaittinen]

On 7/7/24 16:05, Jonathan Cameron wrote:
> On Fri, 5 Jul 2024 13:54:12 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> The BU27034NUC was cancelled before it entered mass production. It was
>> replaced by a new variant BU27034ANUC (note, added 'A'). The new
>> variant gained a few significant changes, like removal of the 3.rd data
>> channel and dropping some of the gain settings. This means that, from
>> software point of view these ICs are incompatible. Lux calculation based
>> on the data from the sensors needs to be done differently, and on the
>> BU27034ANUC the channel 3 data is missing. Also, the gain setting
>> differencies matter.
>>
>> Unfortunately, the identification register was not changed so there is no
>> safe way for the software to distinguish the variants.
>>
>> According to the ROHM HQ engineers, the old BU27034NUC should not be
>> encountered in the wild. Hence it makes sense to remove the support for
>> the old BU27034NUC and add support for the new BU27034ANUC. Change the
>> compatible in order to not load the incompatible old driver for new sensor
>> (or, if someone had the old sensor, the new driver for it).
>>
>> Drop the compatible for old sensor which should not be in the wild and
>> add a new compatible for the new model with accurate model suffix
>> 'anuc'.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> Rename indeed makes sense.  One minor, 'whilst you are here' comment inline.
> 
>>
>> ---
>> A patch renaming the file according to the new compatible will follow.
>> If renaming is not needed or appropriate, that patch can be dropped.
>>
>> Revision history:
>> v2: New patch
>> ---
>>   .../devicetree/bindings/iio/light/rohm,bu27034.yaml      | 9 ++++-----
>>   1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml b/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
>> index 30a109a1bf3b..535bd18348ac 100644
>> --- a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
>> +++ b/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
>> @@ -4,20 +4,19 @@
>>   $id: http://devicetree.org/schemas/iio/light/rohm,bu27034.yaml#
>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>>   
>> -title: ROHM BU27034 ambient light sensor
>> +title: ROHM BU27034ANUC ambient light sensor
>>   
>>   maintainers:
>>     - Matti Vaittinen <mazziesaccount@gmail.com>
>>   
>>   description: |
>> -  ROHM BU27034 is an ambient light sesnor with 3 channels and 3 photo diodes
>> +  ROHM BU27034ANUC is an ambient light sesnor with 2 channels and 2 photo diodes
> 
>   sensor

Thanks Jonathan!

I won't re-spin this unless you ask me to because you wrote you can fix 
it whilist applying... Please, let me know if you wish me to fix and 
re-spin :)

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [PATCH v2 2/7] dt-bindings: iio: rename bu27034 file

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2015 bytes --]

On Fri, Jul 05, 2024 at 01:54:26PM +0300, Matti Vaittinen wrote:
> The BU27034NUC was cancelled before it entered mass production. It was
> replaced by a new variant BU27034ANUC (note, added 'A'). The new
> variant gained a few significant changes, like removal of the 3.rd data
> channel and dropping some of the gain settings. This means that, from
> software point of view these ICs are incompatible. Lux calculation based
> on the data from the sensors needs to be done differently, and on the
> BU27034ANUC the channel 3 data is missing. Also, the gain setting
> differencies matter.
> 
> The old sensor should not be out there so the compatible was dropped and
> a new compatible was added for the bu27034anuc. Move the yaml file so
> the file name matches the binding and change the $id.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> Revision history:
> v1 => v2:
> - New patch
> ---
>  .../iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml}      | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>  rename Documentation/devicetree/bindings/iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml} (92%)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml b/Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
> similarity index 92%
> rename from Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
> rename to Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
> index 535bd18348ac..fc3d826ed8ba 100644
> --- a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>  %YAML 1.2
>  ---
> -$id: http://devicetree.org/schemas/iio/light/rohm,bu27034.yaml#
> +$id: http://devicetree.org/schemas/iio/light/rohm,bu27034anuc.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#

IMO this should be squashed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/7] dt-bindings: iio: BU27034 => BU27034ANUC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]

On Fri, Jul 05, 2024 at 01:54:12PM +0300, Matti Vaittinen wrote:
> The BU27034NUC was cancelled before it entered mass production. It was
> replaced by a new variant BU27034ANUC (note, added 'A'). The new
> variant gained a few significant changes, like removal of the 3.rd data
> channel and dropping some of the gain settings. This means that, from
> software point of view these ICs are incompatible. Lux calculation based
> on the data from the sensors needs to be done differently, and on the
> BU27034ANUC the channel 3 data is missing. Also, the gain setting
> differencies matter.
> 
> Unfortunately, the identification register was not changed so there is no
> safe way for the software to distinguish the variants.
> 
> According to the ROHM HQ engineers, the old BU27034NUC should not be
> encountered in the wild. Hence it makes sense to remove the support for
> the old BU27034NUC and add support for the new BU27034ANUC. Change the
> compatible in order to not load the incompatible old driver for new sensor
> (or, if someone had the old sensor, the new driver for it).
> 
> Drop the compatible for old sensor which should not be in the wild and
> add a new compatible for the new model with accurate model suffix
> 'anuc'.

Since you say that the "old" device should never be encountered "in the
wild", this change seems fine. If it turns out that it is in the wild,
the new compatible means we could resurrect the old driver etc.

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/7] dt-bindings: iio: rename bu27034 file

[Matti Vaittinen]

On 7/8/24 20:05, Conor Dooley wrote:
> On Fri, Jul 05, 2024 at 01:54:26PM +0300, Matti Vaittinen wrote:
>> The BU27034NUC was cancelled before it entered mass production. It was
>> replaced by a new variant BU27034ANUC (note, added 'A'). The new
>> variant gained a few significant changes, like removal of the 3.rd data
>> channel and dropping some of the gain settings. This means that, from
>> software point of view these ICs are incompatible. Lux calculation based
>> on the data from the sensors needs to be done differently, and on the
>> BU27034ANUC the channel 3 data is missing. Also, the gain setting
>> differencies matter.
>>
>> The old sensor should not be out there so the compatible was dropped and
>> a new compatible was added for the bu27034anuc. Move the yaml file so
>> the file name matches the binding and change the $id.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>> ---
>> Revision history:
>> v1 => v2:
>> - New patch
>> ---
>>   .../iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml}      | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>   rename Documentation/devicetree/bindings/iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml} (92%)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml b/Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
>> similarity index 92%
>> rename from Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
>> rename to Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
>> index 535bd18348ac..fc3d826ed8ba 100644
>> --- a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
>> +++ b/Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
>> @@ -1,7 +1,7 @@
>>   # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>   %YAML 1.2
>>   ---
>> -$id: http://devicetree.org/schemas/iio/light/rohm,bu27034.yaml#
>> +$id: http://devicetree.org/schemas/iio/light/rohm,bu27034anuc.yaml#
>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
> 
> IMO this should be squashed.

I've no objections to squashing this. The main motivation of having it 
as a separate patch was to point out the file rename for reviewers and 
ask if it is Ok. Furthermore, if there was a reason not to do the 
rename, then this patch could've been just dropped while the rest of the 
series could've been applied.

Thanks for the review!

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [PATCH v2 2/7] dt-bindings: iio: rename bu27034 file

[Jonathan Cameron]

On Tue, 9 Jul 2024 21:33:02 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 7/8/24 20:05, Conor Dooley wrote:
> > On Fri, Jul 05, 2024 at 01:54:26PM +0300, Matti Vaittinen wrote:  
> >> The BU27034NUC was cancelled before it entered mass production. It was
> >> replaced by a new variant BU27034ANUC (note, added 'A'). The new
> >> variant gained a few significant changes, like removal of the 3.rd data
> >> channel and dropping some of the gain settings. This means that, from
> >> software point of view these ICs are incompatible. Lux calculation based
> >> on the data from the sensors needs to be done differently, and on the
> >> BU27034ANUC the channel 3 data is missing. Also, the gain setting
> >> differencies matter.
> >>
> >> The old sensor should not be out there so the compatible was dropped and
> >> a new compatible was added for the bu27034anuc. Move the yaml file so
> >> the file name matches the binding and change the $id.
> >>
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> >> ---
> >> Revision history:
> >> v1 => v2:
> >> - New patch
> >> ---
> >>   .../iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml}      | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>   rename Documentation/devicetree/bindings/iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml} (92%)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml b/Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
> >> similarity index 92%
> >> rename from Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
> >> rename to Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
> >> index 535bd18348ac..fc3d826ed8ba 100644
> >> --- a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
> >> +++ b/Documentation/devicetree/bindings/iio/light/rohm,bu27034anuc.yaml
> >> @@ -1,7 +1,7 @@
> >>   # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>   %YAML 1.2
> >>   ---
> >> -$id: http://devicetree.org/schemas/iio/light/rohm,bu27034.yaml#
> >> +$id: http://devicetree.org/schemas/iio/light/rohm,bu27034anuc.yaml#
> >>   $schema: http://devicetree.org/meta-schemas/core.yaml#  
> > 
> > IMO this should be squashed.  
> 
> I've no objections to squashing this. The main motivation of having it 
> as a separate patch was to point out the file rename for reviewers and 
> ask if it is Ok. Furthermore, if there was a reason not to do the 
> rename, then this patch could've been just dropped while the rest of the 
> series could've been applied.
> 
> Thanks for the review!
I squashed into previous patch whilst applying.

Thanks,

Jonathan

> 
> Yours,
> 	-- Matti
> 

Re: [PATCH v2 0/7] ROHM BU27034NUC to ROHM BU27034ANUC

[Jonathan Cameron]

On Fri, 5 Jul 2024 13:53:35 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> As discussed here:
> https://lore.kernel.org/all/ff8d6d14-6b48-4347-8525-e05eeb9721ff@gmail.com/
> 
> The ROHM BU27034NUC was cancelled before it entered mass-production. A
> replacement was developed and named to BU27034ANUC. (Note the added
> 'A' in the model name). The BU27034ANUC has several changes that make
> the old BU27034NUC driver unusable with it. I was told the old BU27034NUC
> should not be encountered anywhere.
> 
> Hence, this series converts the rohm-bu27034.c to support the new
> BU27034ANUC instead of the obsoleted BU27034NUC. Additionally, the
> series adds a read-only entry for the HARDWAREGAIN to help understanding
> what part of the scale is contributed by the gain, and what by the
> integration time. This is useful when figuring out why some transitions
> from one 'scale' to other are failing.
Series applied with that tweak to fix the typo that is getting moved in patch 1
and to squash the first 2 patches.

Applied for now only to the testing branch of iio.git which will become
togreg after I can rebase on rc1.

Thanks,

Jonathan

> 
> Revision history:
> 	v1 => v2:
> 		- Split the one large patch to patches 3 - 6 for easier
> 		  review. (Please let me know if you wish me to squash
> 		  them to one).
> 		- Introduce new compatible for the BU27034ANUC and drop
> 		  the old one.
> 		- Add styling fixes as suggested by Jonathan
> 		- Fix the lux calculation coefficient selection logic
> 	link to v1:
> 		https://lore.kernel.org/all/cover.1718013518.git.mazziesaccount@gmail.com/
> 
> ---
> 
> Matti Vaittinen (7):
>   dt-bindings: iio: BU27034 => BU27034ANUC
>   dt-bindings: iio: rename bu27034 file
>   bu27034: ROHM BU27034NUC to BU27034ANUC
>   bu27034: ROHM BU27034NUC to BU27034ANUC drop data2
>   bu27034: ROHM BU27034ANUC correct gains and times
>   bu27034: ROHM BU27034ANUC correct lux calculation
>   iio: bu27034: Add a read only HWARDWAREGAIN
> 
>  ...ohm,bu27034.yaml => rohm,bu27034anuc.yaml} |  11 +-
>  drivers/iio/light/rohm-bu27034.c              | 343 +++++-------------
>  2 files changed, 89 insertions(+), 265 deletions(-)
>  rename Documentation/devicetree/bindings/iio/light/{rohm,bu27034.yaml => rohm,bu27034anuc.yaml} (66%)
> 
> 
> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0