Support ROHM BU27034 ALS sensor

Support ROHM BU27034 ALS sensor

This series adds support for ROHM BU27034 Ambient Light Sensor.

The BU27034 has configurable gain and measurement (integration) time settings. Both of these have inversely proportional relation to the sensor's intensity channel scale.

Many users only set the scale, which means that many drivers attempt to 'guess' the best gain+time combination to meet the scale. Usually this is the biggest integration time which allows setting the requested scale. Typically, increasing the integration time has better accuracy than increasing the gain, which often amplifies the noise as well as the real signal.

However, there may be cases where more responsive sensors are needed. So, in some cases the longest integration times may not be what the user prefers. The driver has no way of knowing this.

Hence, the approach taken by this series is to allow user to set both the scale and the integration time with following logic:

1. When scale is set, the existing integration time is tried to be maintained as a first priority.
1a) If the requested scale can't be met by current time, then also
other time + gain combinations are searched. If scale can be met by some other integration time, then the new time may be applied. If the time setting is common for all channels, then also other channels must be able to maintain their scale with this new time (by changing their gain). The new times are scanned in the order of preference (typically the longest times first).
1b) If the requested scale can be met using current time, then only
the gain for the channel is changed.

2. When the integration time change - scale is tried to be maintained. When integration time change is requested also gain for all impacted channels is adjusted so that the scale is not changed, or is chaned as little as possible. This is different from the RFCv1 where the request was rejected if suitable gain couldn't be found for some channel(s).

This logic is simple. When total gain (either caused by time or hw-gain) is doubled, the scale gets halved. Also, the supported times are given a 'multiplier' value which tells how much they increase the total gain.

However, when I wrote this logic in bu27034 driver, I made quite a few errors on the way - and driver got pretty big. As I am writing drivers for two other sensors (RGB C/IR + flicker BU27010 and RGB C/IR BU27008) with similar gain-time-scale logic I thought that adding common helpers for these computations might be wise. I hope this way all the bugs will be concentrated in one place and not in every individual driver ;)

Hence, this series also intriduces IIO gain-time-scale helpers (abbreviated as gts-helpers). I have also written a couple of KUnit tests for the most hairy parts but those are not part of this series as they depend on kunit_devices which are not yet supported in-tree. I'll send those tests as a separate patch (or series) when the Kunit dependencies are merged in-tree. Meanwhile the interested people can find the tests from my private playground:
https://github.com/M-Vaittinen/linux/tree/iio-gts-tests
(Tests are not maintained there so some porting may be needed if/when things change in mainline.

Finally, these added helpers do provide some value also for drivers which only:
a) allow gain change
or
b) allow changing both the time and gain while trying to maintain the scale.

For a) we provide the gain - selector (register value) table format + selector to gain look-ups, gain <-> scale conversions and the available scales helpers.

For latter case we also provide the time-tables, and actually all the APIs should be usable by setting the time multiplier to 1. (not testeted thoroughly though).

Revision history:
v6 => v7:
- Drop Kunit tests for gts-helpers. These will be sent as a follow-up when dependencies get in-tree.
- BU27034 driver:
- Prevent use of a "bogus" gain value in integration time setting.
- Minor styling
- GTS helpers:
- Let caller know whether the iio_gts_find_new_gain_by_old_gain_time() or iio_gts_find_new_gain_sel_by_old_gain_time() updated the new_gain in a case where the call fails. This can be relevant to callers like can be seen in BU27034 integration time setting.
v5 => v6:
- Just a minor fixes in iio-gts-helpers and bu27034 driver.
- Kunit device helper for a test device creation.
- IIO GTS tests use kunit device helper.

v4 => v5: Mostly fixes to review comments from Andy and Jonathan.
- more accurate change-log in individual patches
- copy code from DRM test helper instead of moving it to simplify merging
- document all exported GTS helpers.
- inline a few GTS helpers
- use again Milli lux for the bu27034 with RAW IIO_LIGHT channel and scale
- Fix bug from added in v4 bu27034 time setting.

v3 => v4: (Still mostly fixes to review comments from Andy and Jonathan)
- more accurate change-log in individual patches
- dt-binding and maintainer patches unchanged.
- dropped unused helpers and converted ones currently used only internally to static.
- extracted "dummy device" creation helpers from DRM tests.
- added tests for devm APIs
- dropped scale for PROCESSED channel in BU27034 and converted mLux values to luxes
- dropped channel 2 GAIN setting which can't be done due to HW limitations.

v2 => v3: (Mostly fixes to review comments from Andy and Jonathan)
- dt-binding and maintainer patches unchanged.
- iio-gts-helper tests: Use namespaces
- iio-gts-helpers + bu27034 plenty of changes. See more comprehensive changelog in individual patches.

RFCv1 => v2:
dt-bindings:
- Fix binding file name and id by using comma instead of a hyphen to separate the vendor and part names.
gts-helpers:
- fix include guardian
- Improve kernel doc for iio_init_iio_gts.
- Add iio_gts_scale_to_total_gain
- Add iio_gts_total_gain_to_scale
- Fix review comments from Jonathan
- add documentation to few functions
- replace 0xffffffffffffffffLLU by U64_MAX
- some styling fixes
- drop unnecessary NULL checks
- order function arguments by in / out purpose
- drop GAIN_SCALE_ITIME_MS()
- Add helpers for available scales and times
- Rename to iio-gts-helpers
gts-tests:
- add tests for available scales/times helpers
- adapt to renamed iio-gts-helpers.h header
bu27034-driver:
- (really) protect read-only registers
- fix get and set gain
- buffered mode
- Protect the whole sequences including meas_en/meas_dis to avoid messing up the enable / disable order
- typofixes / doc improvements
- change dropped GAIN_SCALE_ITIME_MS() to GAIN_SCALE_ITIME_US()
- use more accurate scale for lux channel (milli lux)
- provide available scales / integration times (using helpers).
- adapt to renamed iio-gts-helpers.h file
- bu27034 - longer lines in Kconfig
- Drop bu27034_meas_en and bu27034_meas_dis wrappers.
- Change device-name from bu27034-als to bu27034
MAINTAINERS:
- Add iio-list

---

Matti Vaittinen (5):
iio: light: Add gain-time-scale helpers
MAINTAINERS: Add IIO gain-time-scale helpers
dt-bindings: iio: light: Support ROHM BU27034
iio: light: ROHM BU27034 Ambient Light Sensor
MAINTAINERS: Add ROHM BU27034

 .../bindings/iio/light/rohm,bu27034.yaml      |   46 +
 MAINTAINERS                                   |   14 +
 drivers/iio/Kconfig                           |    3 +
 drivers/iio/Makefile                          |    1 +
 drivers/iio/industrialio-gts-helper.c         | 1077 ++++++++++++
 drivers/iio/light/Kconfig                     |   14 +
 drivers/iio/light/Makefile                    |    1 +
 drivers/iio/light/rohm-bu27034.c              | 1498 +++++++++++++++++
 include/linux/iio/iio-gts-helper.h            |  206 +++
 9 files changed, 2860 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
 create mode 100644 drivers/iio/industrialio-gts-helper.c
 create mode 100644 drivers/iio/light/rohm-bu27034.c
 create mode 100644 include/linux/iio/iio-gts-helper.h

base-commit: eeac8ede17557680855031c6f305ece2378af326

2.39.2


-- 
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 =] 

Some light sensors can adjust both the HW-gain and integration time. There are cases where adjusting the integration time has similar impact to the scale of the reported values as gain setting has.

IIO users do typically expect to handle scale by a single writable 'scale' entry. Driver should then adjust the gain/time accordingly.

It however is difficult for a driver to know whether it should change gain or integration time to meet the requested scale. Usually it is preferred to have longer integration time which usually improves accuracy, but there may be use-cases where long measurement times can be an issue. Thus it can be preferable to allow also changing the integration time - but mitigate the scale impact by also changing the gain underneath. Eg, if integration time change doubles the measured values, the driver can reduce the HW-gain to half.

The theory of the computations of gain-time-scale is simple. However, some people (undersigned) got that implemented wrong for more than once.

Add some gain-time-scale helpers in order to not dublicate errors in all drivers needing these computations.

---
Currently it is only BU27034 using these in this series. I am however working with drivers for RGB sensors BU27008 and BU27010 which have similar [gain - integration time - scale] - relation. I hope sending those follows soon after the BU27034 is done.

Changes:
v6 => v7:
- Allow caller to know whether the iio_gts_find_new_gain_sel_by_old_gain_time() and iio_gts_find_new_gain_by_old_gain_time() did manage to compute a new (unsupported) gain or if the operation failed altogether.
- Minor styling

v5 => v6:
- drop unnecessary comments about using validity tables after free

v4 => v5:
- drop DEFAULT_SYMBOL_NAMESPACE again
- spellcheck
- always build availability tables
- combine table-build and gts-init. Drop no longer needed interfaces
- check for invalid values at init
- document limitations of gain, time, selector and multiplier values
- document all exported functions
- move and inline valid gain / valid time and related functions to header.
NOTE: As a side-effect this makes the internal iteration functions public as well.
- drop unlikely() from "cold path"
- add iio_gts_find_new_gain_by_old_gain_time()

v3 => v4:
- doc styling
- use memset to zero the helper struct at init
- drop unnecessary min calculation at iio_find_closest_gain_low()
- use namespace to all exports
- many minor stylings
- make available outside iio/light (move code to drivers/iio and move the header under include
- rename to look like other files under drivers/iio (s/iio/industrialio)
- drop unused functions
- don't export only internally used functions and make them static Note, I decided to keep iio_gts_total_gain_to_scale() exported as it is currently needed by the tests outside the helpers.

v2 => v3: (mostly fixes based on review by Andy)
- Fix typos
- Styling fixes
- Use namespace for exported symbols
- Protect allocs against argument overflow
- Fix include protection name
- add types.h inclusion and struct device forward declaration

RFCv1 => v2:
- fix include guardian
- Improve kernel doc for iio_init_iio_gts.
- Add iio_gts_scale_to_total_gain
- Add iio_gts_total_gain_to_scale
- Fix review comments from Jonathan
- add documentation to few functions
- replace 0xffffffffffffffffLLU by U64_MAX
- some styling fixes
- drop unnecessary NULL checks
- order function arguments by in / out purpose
- drop GAIN_SCALE_ITIME_MS()
- Add helpers for available scales and times
- Rename to iio-gts-helpers

---
 drivers/iio/Kconfig                   |    3 +
 drivers/iio/Makefile                  |    1 +
 drivers/iio/industrialio-gts-helper.c | 1077 +++++++++++++++++++++++++
 include/linux/iio/iio-gts-helper.h    |  206 +++++
 4 files changed, 1287 insertions(+)
 create mode 100644 drivers/iio/industrialio-gts-helper.c
 create mode 100644 include/linux/iio/iio-gts-helper.h

index b190846c3dc2..52eb46ef84c1 100644
@@ -30,6 +30,9 @@ config IIO_CONFIGFS
 	  (e.g. software triggers). For more info see
 	  Documentation/iio/iio_configfs.rst.
 
+config IIO_GTS_HELPER
+	tristate
+
 config IIO_TRIGGER
 	bool "Enable triggered sampling support"
 	help

index 3be08cdadd7e..9622347a1c1b 100644
@@ -9,6 +9,7 @@ industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
 industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
 
 obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
+obj-$(CONFIG_IIO_GTS_HELPER) += industrialio-gts-helper.o
 obj-$(CONFIG_IIO_SW_DEVICE) += industrialio-sw-device.o
 obj-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
 obj-$(CONFIG_IIO_TRIGGERED_EVENT) += industrialio-triggered-event.o

2.39.2


-- 
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 =] 

Add myself as a maintainer for IIO light sensor helpers (helpers for maintaining the scale while adjusting intergration time or gain) and related Kunit tests.

---
RFCv1 =>
- No changes

---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

index ec57c42ed544..6ec9326f4ce9 100644
@@ -9938,6 +9938,14 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.yaml
 F:	drivers/iio/adc/envelope-detector.c
 
+IIO LIGHT SENSOR GAIN-TIME-SCALE HELPERS
+M:	Matti Vaittinen <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/iio/light/gain-time-scale-helper.c
+F:	drivers/iio/light/gain-time-scale-helper.h
+F:	drivers/iio/test/iio-test-gts.c
+
 IIO MULTIPLEXER
 M:	Peter Rosin <[email protected]>
 L:	[email protected]

2.39.2


-- 
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 =] 

ROHM BU27034 is an ambient light sesnor with 3 channels and 3 photo diodes capable of detecting a very wide range of illuminance. Typical application is adjusting LCD and backlight power of TVs and mobile phones.

Add dt-bindings.

---
v2 =>
- No changes

Changes since RFCv1 => v2
- Fix binding file name and id by using comma instead of a hyphen to separate the vendor and part names.

---
 .../bindings/iio/light/rohm,bu27034.yaml      | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml

new file mode 100644
index 000000000000..30a109a1bf3b
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/rohm,bu27034.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BU27034 ambient light sensor
+
+maintainers:
+  - Matti Vaittinen <[email protected]>
+
+description: |
+  ROHM BU27034 is an ambient light sesnor with 3 channels and 3 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
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      light-sensor@38 {
+        compatible = "rohm,bu27034";
+        reg = <0x38>;
+        vdd-supply = <&vdd>;
+      };
+    };
+
+...

2.39.2


-- 
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 =] 

ROHM BU27034 is an ambient light sensor with 3 channels and 3 photo diodes capable of detecting a very wide range of illuminance. Typical application is adjusting LCD and backlight power of TVs and mobile phones.

Add initial support for the ROHM BU27034 ambient light sensor.

NOTE:
- Driver exposes 4 channels. One IIO_LIGHT channel providing the calculated lux values based on measured data from diodes #0 and #1. In addition, 3 IIO_INTENSITY channels are emitting the raw register data from all diodes for more intense user-space computations.
- Sensor has GAIN values that can be adjusted from 1x to 4096x.
- Sensor has adjustible measurement times of 5, 55, 100, 200 and 400 mS. Driver does not support 5 mS which has special limitations.
- Driver exposes standard 'scale' adjustment which is implemented by:
1) Trying to adjust only the GAIN
2) If GAIN adjustment alone can't provide requested scale, adjusting both the time and the gain is attempted.
- Driver exposes writable INT_TIME property that can be used for adjusting the measurement time. Time adjustment will also cause the driver to try to adjust the GAIN so that the overall scale is kept as close to the original as possible.

---
v6 => v7:
- remove extra linebreak, simplify scan_masks
- use mask define in FIELD_PREP
- do not proceed with setting integration time if 'target gain' was not successfully computed

v5 => v6:
- Use the multiplication with overflow check from overflow.h
- Use FIELD_PREP()

Changes
v4 => v5:
- spellcheck
- back to mlux again
- lux channel PROCESSED => RAW
- use new devm_init for GTS and drop explicit table building
- styling
- drop unnecessary loop in gain setting
- don't unnecessarily delay returning error
- fix integration time change compensation which was broken by v4 change allowing to use the (55 mS) in the time tables. (Rounding error when computing new gain based on times not multipliers).

v3 => v4:
- use min_t() for division by zero check
- adapt to new GTS helper header location
- calculate luxes not milli luxes
- drop scale for PROCESSED channel
- comment improvements
- do not allow changing gain (scale) for channel 2.
- 'tie' channel 2 scale to channel 0 scale
This is because channel 0 and channel 2 GAIN settings share part of the bits in the register. This means that setting one will also impact the other. The v3 of the patches attempted to work-around this by only disallowing the channel 2 gain setting to set the bits which were shared with channel 0 gain. This does not work because setting channel 0 gain (which was allowed to set also the shared bits) could result unsupported bit combinations for channel 2 gain. Thus it is safest to always set also the channel 2 gain to same value as channel 0 gain.
- Use the correct integration time (55 mS) in the gain table as the calcuations can be done based on the time multiplier.
- styling

v2 => v3:
- commit message update and typofixes
- switch warning messages to dbg
- drop incorrect comment about unchanged scales
- return 'no new data' if valid bit read failed
- shorten the 'div by zero' checks
- don't use u32 pointer when int * is epected in lux calculation
- add a comment clarifying why it is safe to return int from lux calculation
- simplify read_raw() by refactoring the measurement start / stop in another function and dropping the goto based unlocking.
- Styling fixes
- select IIO_BUFFER and IIO_KFIFO_BUF
- Alphabetical order of header includes
- Split multipication w/ overflow check to own function
- Do not hang in read_raw() if sensor does not return valid sample
- Spelling fix
- Do not require fwnode
- Use namespace for gts helpers

RFCv1 => v2:
- (really) protect read-only registers
- fix get and set gain
- buffered mode
- Protect the whole sequences including meas_en/meas_dis to avoid messing up the enable / disable order
- typofixes / doc improvements
- change dropped GAIN_SCALE_ITIME_MS() to GAIN_SCALE_ITIME_US()
- use more accurate scale for lux channel (milli lux)
- provide available scales / integration times (using helpers).
- adapt to renamed iio-gts-helpers.h file
- bu27034 - longer lines in Kconfig
- Drop bu27034_meas_en and bu27034_meas_dis wrappers.
- Change device-name from bu27034-als to bu27034

---
 drivers/iio/light/Kconfig        |   14 +
 drivers/iio/light/Makefile       |    1 +
 drivers/iio/light/rohm-bu27034.c | 1498 ++++++++++++++++++++++++++++++
 3 files changed, 1513 insertions(+)
 create mode 100644 drivers/iio/light/rohm-bu27034.c

index 0d4447df7200..6fa31fcd71a1 100644
@@ -289,6 +289,20 @@ config JSA1212
 	  To compile this driver as a module, choose M here:
 	  the module will be called jsa1212.
 
+config ROHM_BU27034
+	tristate "ROHM BU27034 ambient light sensor"
+	depends on I2C
+	select REGMAP_I2C
+	select IIO_GTS_HELPER
+	select IIO_BUFFER
+	select IIO_KFIFO_BUF
+	help
+	  Enable support for the ROHM BU27034 ambient light sensor. ROHM BU27034
+	  is an ambient light sesnor with 3 channels and 3 photo diodes capable
+	  of detecting a very wide range of illuminance.
+	  Typical application is adjusting LCD and backlight power of TVs and
+	  mobile phones.
+
 config RPR0521
 	tristate "ROHM RPR0521 ALS and proximity sensor driver"
 	depends on I2C

index d74d2b5ff14c..985f6feaccd4 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_MAX44009)		+= max44009.o
 obj-$(CONFIG_NOA1305)		+= noa1305.o
 obj-$(CONFIG_OPT3001)		+= opt3001.o
 obj-$(CONFIG_PA12203001)	+= pa12203001.o
+obj-$(CONFIG_ROHM_BU27034)	+= rohm-bu27034.o
 obj-$(CONFIG_RPR0521)		+= rpr0521.o
 obj-$(CONFIG_SI1133)		+= si1133.o
 obj-$(CONFIG_SI1145)		+= si1145.o

2.39.2


-- 
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 =] 

A few trivial comments inline but nothing that stops me applying this version.

		if (chan->type == IIO_INTENSITY)
			ret = bu27034_get_single-result()...
		else
			ret = bu27034_get_mlux()

My memory is getting worse and worse over the time... It's hard for me to remember why I added a function pointer here - but it probably was to keep the if-else if-else construct out of the mutex-protected section.

If this was done, then the function-pointer had no use. If you think the check is paranoid, then I agree, we should go with the suggestion you have below.

Thanks!

Yours,
-- Matti

Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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

As I wrote in the other (sysfs) mail - this driver version is buggy as it handles INT_TIMEs in micro seconds. The correct unit would be seconds.

I'll send an incremental fix to this and the gts-helpers on top of the iio/togreg (hopefully soon).

Yours,
-- Matti

Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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

Add myself as a maintainer for ROHM BU27034 ALS driver.

---
Changes
v2 =>
- No changes

sRFCv1 => v2:
- Add iio-list

---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

index 6ec9326f4ce9..3f13466e50fd 100644
@@ -18100,6 +18100,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/light/bh1750.yaml
 F:	drivers/iio/light/bh1750.c
 
+ROHM BU27034 AMBIENT LIGHT SENSOR DRIVER
+M:	Matti Vaittinen <[email protected]>
+L:	[email protected]
+S:	Supported
+F:	drivers/iio/light/rohm-bu27034.c
+
 ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS
 M:	Marek Vasut <[email protected]>
 L:	[email protected]

2.39.2


-- 
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 deee Ho peeps!

I have one word for all of you who worked to get the ROHM BU27034NUC driver working in upstream.

Meh.

I just found out that the BU27034 sensor which was developed when I wrote this driver had some "manufacturing issues"... The full model number was BU27034NUC. The has been cancelled, and, as far as I know, no significant number of those were manufactured.

The issues of BU27034NUC were solved, and new model BU27034ANUC was developed and is available in the ROHM catalog.

I did also learn that this new model BU27034ANUC is _not_ functionally equivalent to the BU27034NUC. I am currently clarifying all the differences, and I have requested the HQ to send me a sample for driver development and verification work.

This far I've come to know at least following differences:

- The DATA2 (IR) channel is removed. So is the gain setting for it. This should very much simplify the gain logic.
- Some of the gains were removed.
- The 5ms integration time was removed. (The support of 5ms was severely limited on original BU27034NUC too so driver did not support that anyways).
- The light sensitivity curves had changed so the lux calculation will be changed.

One thing that has _not_ changed though is the part-id :rolleyes:

My preferred approach would be to convert the in-tree bu27034 driver to support this new variant. I think it makes sense because:
- (I expect) the amount of code to review will be much smaller this way
than it would be if current driver was completely removed, and new one
submitted.
- This change will not break existing users as there should not be such
(judging the statement that the original BU27034NUC was cancelled
before it was sold "en masse").

It sure is possible to drop the existing driver and submit a new one too, but I think it will be quite a bit more work with no strong benefits.

I expect the rest of the information to be shared to me during the next couple of days, and I hope I can start testing the driver immediately when I get the HW.

My question is, do you prefer the changes to be sent as one "support BU27034ANUC patch, of would you rather see changes splitted to pieces like: "adapt lux calculation to support BU27034ANUC", "remove obsolete DATA2 channel", "remove unsupported gains"...? Furthermore, the DT compatible was just rohm,bu27034 and did not include the ending "nuc". Should that compatible be removed and a new one with "anuc"-suffix be added to denote the new sensor?

I am truly sorry for all the unnecessary reviewing and maintenance work you guys did. I can assure you I didn't go through it for fun either - even if the coding was fun :) I guess even the "upstream early" process has it's weaknesses...

Yours,
-- Matti

Matti Vaittinen
Linux kernel developer at ROHM Semiconductor
Oulu Finland

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

ouch. We all have some cancelled products in our history! When that happens I usually go eat cake and moan at anyone standing near by. At least this seems like there will be some direct use of the work done (sometimes you just have to list the things learnt along the way).

*sigh* Not even a version number? Even unreleased / prototype parts should have different IDs if anything in the interface changed.

Agreed, modify the existing driver. Just needs a clear statement in patch descriptions that the original part is not expected to be in the wild.

Separate patches preferred for each feature / type of change. Mostly they'll hopefully be trivial to review.

Yes. The binding patch in particular will need a really clear statement that we believe there are none in products in the wild.

True enough. It's always 'interesting' to not know if / when a product you've upstreamed code for will launch.

Jonathan

I like that approach :) Luckily, this was just a sensor. It was much more painful back in the Nokia when some of the BTS variants were cancelled. It flushed 'man years' of work instead of 'man months' :)

Yes! It wasn't all wasted effort!

No.

...tell me about it... Well, I tried to send feedback - but I am not convinced this is not happening again. I think I could fill a book with feedback which has had not been listened in the past - but who knows, occasionally feedback also works. So, we can keep trying. :)

ack.

I've drafted most of the changes and it seems they are more or less trivial. I've not yet received the hardware so the changes are 100% untested though.

ack.

Indeed. Almost as interesting as having patches for a new product in your "to be sent" - folder for 3 years waiting for the product to launch to get permission to send the patches... Don't we all love maintaining off-tree patches when we have that creeping feeling the patches will never be allowed to be sent...? Asking for a friend :rolleyes:

Yours,
-- Matti

Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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