As discussed here:
https://lore.kernel.org/all/[email protected]/
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.
---
Matti Vaittinen (2):
bu27034: ROHM BU27034NUC to BU27034ANUC
iio: bu27034: Add a read only HWARDWAREGAIN
drivers/iio/light/rohm-bu27034.c | 335 ++++++++-----------------------
1 file changed, 81 insertions(+), 254 deletions(-)
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 =]
The ROHM BU27034NUC was cancelled and BU27034ANUC is replacing this
sensor. Use the BU27034NUC driver to support the new BU27034ANUC.
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.
Signed-off-by: Matti Vaittinen <[email protected]>
Fixes: e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor")
---
drivers/iio/light/rohm-bu27034.c | 321 +++++++------------------------
1 file changed, 68 insertions(+), 253 deletions(-)
diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index bf3de853a811..51acad2cafbd 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>
@@ -30,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
@@ -88,58 +85,48 @@ 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
};
/*
- * 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.
@@ -156,11 +143,11 @@ 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), \
+ .channel2 = IIO_MOD_LIGHT_CLEAR, \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
BIT(IIO_CHAN_INFO_SCALE), \
.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
@@ -195,13 +182,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 they're
+ * both marked as clear channels.
*/
- 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),
};
@@ -215,20 +201,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,
@@ -238,7 +218,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,
},
};
@@ -250,7 +230,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,
@@ -281,39 +261,15 @@ static int bu27034_get_gain_sel(struct bu27034_data *data, int chan)
{
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;
+ 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;
- /*
- * 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)
@@ -396,44 +352,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);
}
@@ -441,13 +362,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;
@@ -571,9 +485,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;
@@ -598,9 +509,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;
@@ -614,7 +523,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;
@@ -629,7 +538,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);
@@ -641,7 +550,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;
@@ -665,102 +574,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
- *
- * => 115.7400832 * ch1 / gain1 / mt -
- * 68.1982976 * ch0 / gain0 / mt +
- * 0.00012213 * 25600 * (ch1 / gain1 / mt) * 25600 *
- * (ch1 /gain1 / mt) / (25600 * ch0 / gain0 / mt)
+ * for (D1/D0 < 1.5):
+ * lx = (0.001193 * D0 + (-0.0000747) * D1) * ((D1/D0 – 1.5) * (0.25) + 1)
*
- * 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
+ * => -0.000745625 * D0 + 0.0002515625 * D1 + -0.000018675 * D1 * D1 / D0
*
- * 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
+ * => (6.44 * ch1 / gain1 + 19.088 * ch0 / gain0 -
+ * 0.47808 * ch1 * ch1 * gain0 / gain1 / gain1 / ch0) /
+ * mt
*
- * => (115.7400832 * ch1 / gain1 - 68.1982976 * ch0 / gain0 +
- * 3.126528 * ch1 * ch1 * gain0 / gain1 / gain1 / ch0) /
- * mt
+ * Else
+ * lx = 0.001193 * D0 - 0.0000747 * D1
*
- * 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 = [
@@ -770,19 +598,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 {
@@ -887,21 +710,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];
@@ -973,7 +791,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;
@@ -1040,7 +857,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);
@@ -1065,12 +882,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
@@ -1139,7 +954,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 =]
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 <[email protected]>
---
drivers/iio/light/rohm-bu27034.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
index 51acad2cafbd..b299ff2aacce 100644
--- a/drivers/iio/light/rohm-bu27034.c
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -149,7 +149,8 @@ static const struct iio_itime_sel_mul bu27034_itimes[] = {
.channel = BU27034_CHAN_##_name, \
.channel2 = IIO_MOD_LIGHT_CLEAR, \
.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 = \
@@ -992,6 +993,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);
@@ -1036,12 +1044,16 @@ 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");
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 =]
Hi Matti,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0]
url: https://github.com/intel-lab-lkp/linux/commits/Matti-Vaittinen/bu27034-ROHM-BU27034NUC-to-BU27034ANUC/20240610-180426
base: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
patch link: https://lore.kernel.org/r/5e88c7b7b0389c6c011f15e05e065791f7561cf5.1718013518.git.mazziesaccount%40gmail.com
patch subject: [PATCH 2/2] iio: bu27034: Add a read only HWARDWAREGAIN
config: i386-buildonly-randconfig-002-20240610 (https://download.01.org/0day-ci/archive/20240610/[email protected]/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240610/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from drivers/iio/light/rohm-bu27034.c:10:
drivers/iio/light/rohm-bu27034.c: In function 'bu27034_write_raw_get_fmt':
>> include/linux/dev_printk.h:138:20: warning: this statement may fall through [-Wimplicit-fallthrough=]
137 | ({ \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
138 | if (0) \
| ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
139 | _dev_printk(level, dev, fmt, ##__VA_ARGS__); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
140 | })
| ~~
include/linux/dev_printk.h:171:9: note: in expansion of macro 'dev_no_printk'
171 | dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~
drivers/iio/light/rohm-bu27034.c:1055:17: note: in expansion of macro 'dev_dbg'
1055 | dev_dbg(data->dev,
| ^~~~~~~
drivers/iio/light/rohm-bu27034.c:1057:9: note: here
1057 | default:
| ^~~~~~~
vim +138 include/linux/dev_printk.h
af628aae8640c2 Greg Kroah-Hartman 2019-12-09 99
ad7d61f159db73 Chris Down 2021-06-15 100 /*
ad7d61f159db73 Chris Down 2021-06-15 101 * Need to take variadic arguments even though we don't use them, as dev_fmt()
ad7d61f159db73 Chris Down 2021-06-15 102 * may only just have been expanded and may result in multiple arguments.
ad7d61f159db73 Chris Down 2021-06-15 103 */
ad7d61f159db73 Chris Down 2021-06-15 104 #define dev_printk_index_emit(level, fmt, ...) \
ad7d61f159db73 Chris Down 2021-06-15 105 printk_index_subsys_emit("%s %s: ", level, fmt)
ad7d61f159db73 Chris Down 2021-06-15 106
ad7d61f159db73 Chris Down 2021-06-15 107 #define dev_printk_index_wrap(_p_func, level, dev, fmt, ...) \
ad7d61f159db73 Chris Down 2021-06-15 108 ({ \
ad7d61f159db73 Chris Down 2021-06-15 109 dev_printk_index_emit(level, fmt); \
ad7d61f159db73 Chris Down 2021-06-15 110 _p_func(dev, fmt, ##__VA_ARGS__); \
ad7d61f159db73 Chris Down 2021-06-15 111 })
ad7d61f159db73 Chris Down 2021-06-15 112
ad7d61f159db73 Chris Down 2021-06-15 113 /*
ad7d61f159db73 Chris Down 2021-06-15 114 * Some callsites directly call dev_printk rather than going through the
ad7d61f159db73 Chris Down 2021-06-15 115 * dev_<level> infrastructure, so we need to emit here as well as inside those
ad7d61f159db73 Chris Down 2021-06-15 116 * level-specific macros. Only one index entry will be produced, either way,
ad7d61f159db73 Chris Down 2021-06-15 117 * since dev_printk's `fmt` isn't known at compile time if going through the
ad7d61f159db73 Chris Down 2021-06-15 118 * dev_<level> macros.
ad7d61f159db73 Chris Down 2021-06-15 119 *
ad7d61f159db73 Chris Down 2021-06-15 120 * dev_fmt() isn't called for dev_printk when used directly, as it's used by
ad7d61f159db73 Chris Down 2021-06-15 121 * the dev_<level> macros internally which already have dev_fmt() processed.
ad7d61f159db73 Chris Down 2021-06-15 122 *
ad7d61f159db73 Chris Down 2021-06-15 123 * We also can't use dev_printk_index_wrap directly, because we have a separate
ad7d61f159db73 Chris Down 2021-06-15 124 * level to process.
ad7d61f159db73 Chris Down 2021-06-15 125 */
ad7d61f159db73 Chris Down 2021-06-15 126 #define dev_printk(level, dev, fmt, ...) \
ad7d61f159db73 Chris Down 2021-06-15 127 ({ \
ad7d61f159db73 Chris Down 2021-06-15 128 dev_printk_index_emit(level, fmt); \
ad7d61f159db73 Chris Down 2021-06-15 129 _dev_printk(level, dev, fmt, ##__VA_ARGS__); \
ad7d61f159db73 Chris Down 2021-06-15 130 })
ad7d61f159db73 Chris Down 2021-06-15 131
c26ec799042a38 Geert Uytterhoeven 2024-02-28 132 /*
c26ec799042a38 Geert Uytterhoeven 2024-02-28 133 * Dummy dev_printk for disabled debugging statements to use whilst maintaining
c26ec799042a38 Geert Uytterhoeven 2024-02-28 134 * gcc's format checking.
c26ec799042a38 Geert Uytterhoeven 2024-02-28 135 */
c26ec799042a38 Geert Uytterhoeven 2024-02-28 136 #define dev_no_printk(level, dev, fmt, ...) \
c26ec799042a38 Geert Uytterhoeven 2024-02-28 137 ({ \
c26ec799042a38 Geert Uytterhoeven 2024-02-28 @138 if (0) \
c26ec799042a38 Geert Uytterhoeven 2024-02-28 139 _dev_printk(level, dev, fmt, ##__VA_ARGS__); \
c26ec799042a38 Geert Uytterhoeven 2024-02-28 140 })
c26ec799042a38 Geert Uytterhoeven 2024-02-28 141
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Matti,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0]
url: https://github.com/intel-lab-lkp/linux/commits/Matti-Vaittinen/bu27034-ROHM-BU27034NUC-to-BU27034ANUC/20240610-180426
base: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
patch link: https://lore.kernel.org/r/5e88c7b7b0389c6c011f15e05e065791f7561cf5.1718013518.git.mazziesaccount%40gmail.com
patch subject: [PATCH 2/2] iio: bu27034: Add a read only HWARDWAREGAIN
config: i386-buildonly-randconfig-004-20240610 (https://download.01.org/0day-ci/archive/20240610/[email protected]/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240610/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/iio/light/rohm-bu27034.c:1057:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
1057 | default:
| ^
drivers/iio/light/rohm-bu27034.c:1057:2: note: insert '__attribute__((fallthrough));' to silence this warning
1057 | default:
| ^
| __attribute__((fallthrough));
drivers/iio/light/rohm-bu27034.c:1057:2: note: insert 'break;' to avoid fall-through
1057 | default:
| ^
| break;
1 warning generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for REGMAP_SPI
Depends on [n]: SPI [=n]
Selected by [y]:
- AD9739A [=y] && IIO [=y] && (SPI [=n] || COMPILE_TEST [=y])
vim +1057 drivers/iio/light/rohm-bu27034.c
e52afbd61039e2 Matti Vaittinen 2023-03-31 1042
d47b9b84292706 Matti Vaittinen 2023-06-13 1043 static int bu27034_write_raw_get_fmt(struct iio_dev *indio_dev,
d47b9b84292706 Matti Vaittinen 2023-06-13 1044 struct iio_chan_spec const *chan,
d47b9b84292706 Matti Vaittinen 2023-06-13 1045 long mask)
d47b9b84292706 Matti Vaittinen 2023-06-13 1046 {
6196c88810e86d Matti Vaittinen 2024-06-10 1047 struct bu27034_data *data = iio_priv(indio_dev);
d47b9b84292706 Matti Vaittinen 2023-06-13 1048
d47b9b84292706 Matti Vaittinen 2023-06-13 1049 switch (mask) {
d47b9b84292706 Matti Vaittinen 2023-06-13 1050 case IIO_CHAN_INFO_SCALE:
d47b9b84292706 Matti Vaittinen 2023-06-13 1051 return IIO_VAL_INT_PLUS_NANO;
d47b9b84292706 Matti Vaittinen 2023-06-13 1052 case IIO_CHAN_INFO_INT_TIME:
d47b9b84292706 Matti Vaittinen 2023-06-13 1053 return IIO_VAL_INT_PLUS_MICRO;
6196c88810e86d Matti Vaittinen 2024-06-10 1054 case IIO_CHAN_INFO_HARDWAREGAIN:
6196c88810e86d Matti Vaittinen 2024-06-10 1055 dev_dbg(data->dev,
6196c88810e86d Matti Vaittinen 2024-06-10 1056 "HARDWAREGAIN is read-only, use scale to set\n");
d47b9b84292706 Matti Vaittinen 2023-06-13 @1057 default:
d47b9b84292706 Matti Vaittinen 2023-06-13 1058 return -EINVAL;
d47b9b84292706 Matti Vaittinen 2023-06-13 1059 }
d47b9b84292706 Matti Vaittinen 2023-06-13 1060 }
d47b9b84292706 Matti Vaittinen 2023-06-13 1061
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Mon, 10 Jun 2024 13:01:23 +0300
Matti Vaittinen <[email protected]> wrote:
> The ROHM BU27034NUC was cancelled and BU27034ANUC is replacing this
> sensor. Use the BU27034NUC driver to support the new BU27034ANUC.
>
> 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.
>
> Signed-off-by: Matti Vaittinen <[email protected]>
> Fixes: e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor")
This is an odd case. I don't think a fixes tag is appropriate and I
don't think we can use the original compatible. I don't mind breaking
support for the non existent port going forwards and indeed dropping
all indication it ever existed, but the old kernel's are out there and
even getting this into stable is far from a guarantee there won't be
a kernel run on a board that has this compatible but has the old
driver. It's also too big really to be stable material.
So I think the path forwards is a new compatible and drop the old
one from the dt bindings and driver. Thus any new dts for a board
that actually has this device will use the new compatible and avoid
any risk of encountering the old driver.
Maybe we can be more relaxed - what actually happens if you use the
existing driver with the new part?
I'm trusting you copied the maths right for the computed
channels (that take too long to review!) So everything inline is
formatting type stuff.
> ---
> drivers/iio/light/rohm-bu27034.c | 321 +++++++------------------------
> 1 file changed, 68 insertions(+), 253 deletions(-)
>
> diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
> index bf3de853a811..51acad2cafbd 100644
> --- a/drivers/iio/light/rohm-bu27034.c
> +++ b/drivers/iio/light/rohm-bu27034.c
...
> /*
> - * 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
...
> -#define BU27034_CHAN_DATA(_name, _ch2) \
> +#define BU27034_CHAN_DATA(_name) \
> { \
> .type = IIO_INTENSITY, \
> .channel = BU27034_CHAN_##_name, \
> - .channel2 = (_ch2), \
> + .channel2 = IIO_MOD_LIGHT_CLEAR, \
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> BIT(IIO_CHAN_INFO_SCALE), \
> .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \
> @@ -195,13 +182,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 they're
> + * both marked as clear channels.
I think just indexing them and not giving a modifier is probably better than
claiming they are clear. Leave it more vague basically.
> */
> - 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),
> };
> @@ -281,39 +261,15 @@ static int bu27034_get_gain_sel(struct bu27034_data *data, int chan)
> {
> int ret, val;
>
Probably no blank line here.
> - 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;
> + int reg[] = {
> + [BU27034_CHAN_DATA0] = BU27034_REG_MODE_CONTROL2,
> + [BU27034_CHAN_DATA1] = BU27034_REG_MODE_CONTROL3,
> + };
blank line here
> + ret = regmap_read(data->regmap, reg[chan], &val);
> + if (ret)
> + return ret;
...
>
> struct bu27034_lx_coeff {
> @@ -887,21 +710,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 */
/* 6.44 */
It was odd before but might as well tidy that up!
> + .C = 190880000, /* 19.088 */
> + .is_neg = {true, false, false},
{ true, false, false }
Tidy that up as well.
> }, {
> - .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},
> - }
> + },
> };
...
> @@ -1065,12 +882,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)
Brackets around 0.25 odd. The negative one might be justified if the code
is such that you can't just do D0 - 0.0000747 instead.
> + * Else
> + * lx = (0.001193* D0 + (-0.0000747) * D1)
Space between 3 and *
> *
> * 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
> @@ -1139,7 +954,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);
On Mon, 10 Jun 2024 13:01:40 +0300
Matti Vaittinen <[email protected]> wrote:
> 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 <[email protected]>
Other than the thing the bot found with fallthrough on the switch statement
not being marked LGTM.
> ---
> drivers/iio/light/rohm-bu27034.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
> index 51acad2cafbd..b299ff2aacce 100644
> --- a/drivers/iio/light/rohm-bu27034.c
> +++ b/drivers/iio/light/rohm-bu27034.c
> @@ -149,7 +149,8 @@ static const struct iio_itime_sel_mul bu27034_itimes[] = {
> .channel = BU27034_CHAN_##_name, \
> .channel2 = IIO_MOD_LIGHT_CLEAR, \
> .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 = \
> @@ -992,6 +993,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);
>
> @@ -1036,12 +1044,16 @@ 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 here. You could use a fall through marking but it gains
little so I wouldn't bother.
> default:
> return -EINVAL;
> }