All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/7] Support ROHM BU27034 ALS sensor
@ 2023-03-27 11:27 Matti Vaittinen
  2023-03-27 11:27 ` [PATCH v6 1/7] dt-bindings: iio: light: Support ROHM BU27034 Matti Vaittinen
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:27 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Masahiro Yamada, Randy Dunlap, Matti Vaittinen, Shreeya Patel,
	Krzysztof Kozlowski, Jonathan Cameron, devicetree, Zhigang Shi,
	Lars-Peter Clausen, Paul Gazzillo, Rob Herring, Dmitry Osipenko,
	linux-iio, linux-kernel, kunit-dev, Stephen Boyd, Liam Beguin,
	Greg Kroah-Hartman, Andy Shevchenko, David Gow, Brendan Higgins,
	linux-kselftest

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

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) + a couple of KUnit tests for the most
hairy parts.

Speaking of which - testing the devm interfaces requires a 'dummy
device'. I've learned that there has been at least two ways of handling
this kind of a dependecy.

1) Using a root_device_[un]register() functions (with or without a
wrapper)

2) Using dummy platform_device.

Way 2) is seen as abusing platform_devices to something they should not
be used.

Way 1) is also seen sub-optimal - and after a discussion a 'kunit dummy
device' is being worked on by David Gow:
https://lore.kernel.org/linux-kselftest/20230325043104.3761770-1-davidgow@google.com/T/#mf797239a8bce11630875fdf60aab9ed627add1f0

David's work relies on not yet in-tree kunit deferring API. Schedule for
this work is - as always in case of upstream development - unkonwn. In
order to be self-contained while still easily 'fixable when David's work
is completed' this series introduces warappers named similar to what was
suggested by david - and which are intended to have similar behaviour
(automatic clean-up upon test completion). These wrappers do still use
root-device APIs underneath but this should be fixed by David's work.

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).

The patch 1/7 introduces the helpers for creating/dropping a test device
for devm-tests. It can be applied alone.

The patch 4/7 (IIO GTS tests) also depends on the patch 1/7 (and also
other patches in the series).

Rest of the series should be Ok to be applied with/without the patches
1/7 and 4/7 - although the 4/7 (which depends on 1/7) would be "nice to
have" together with the rest of the series for the testability reasons.

Revision history:
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 (7):
  dt-bindings: iio: light: Support ROHM BU27034
  iio: light: Add gain-time-scale helpers
  kunit: Add kunit wrappers for (root) device creation
  iio: test: test gain-time-scale helpers
  MAINTAINERS: Add IIO gain-time-scale helpers
  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         | 1057 ++++++++++++
 drivers/iio/light/Kconfig                     |   14 +
 drivers/iio/light/Makefile                    |    1 +
 drivers/iio/light/rohm-bu27034.c              | 1496 +++++++++++++++++
 drivers/iio/test/Kconfig                      |   14 +
 drivers/iio/test/Makefile                     |    1 +
 drivers/iio/test/iio-test-gts.c               |  517 ++++++
 include/kunit/device.h                        |   18 +
 include/linux/iio/iio-gts-helper.h            |  206 +++
 lib/kunit/Makefile                            |    3 +-
 lib/kunit/device.c                            |   36 +
 15 files changed, 3426 insertions(+), 1 deletion(-)
 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 drivers/iio/test/iio-test-gts.c
 create mode 100644 include/kunit/device.h
 create mode 100644 include/linux/iio/iio-gts-helper.h
 create mode 100644 lib/kunit/device.c


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

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 1/7] dt-bindings: iio: light: Support ROHM BU27034
  2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
@ 2023-03-27 11:27 ` Matti Vaittinen
  2023-03-27 11:28 ` [PATCH v6 2/7] iio: light: Add gain-time-scale helpers Matti Vaittinen
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:27 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
	Krzysztof Kozlowski, Matti Vaittinen, linux-iio, devicetree,
	linux-kernel

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

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.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

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

diff --git a/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml b/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
new file mode 100644
index 000000000000..30a109a1bf3b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
@@ -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 <mazziesaccount@gmail.com>
+
+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 =] 

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 2/7] iio: light: Add gain-time-scale helpers
  2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
  2023-03-27 11:27 ` [PATCH v6 1/7] dt-bindings: iio: light: Support ROHM BU27034 Matti Vaittinen
@ 2023-03-27 11:28 ` Matti Vaittinen
  2023-03-30 16:48   ` Matti Vaittinen
  2023-03-27 11:34 ` [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation Matti Vaittinen
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:28 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-kernel, linux-iio

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

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.

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

---
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:
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 | 1057 +++++++++++++++++++++++++
 include/linux/iio/iio-gts-helper.h    |  206 +++++
 4 files changed, 1267 insertions(+)
 create mode 100644 drivers/iio/industrialio-gts-helper.c
 create mode 100644 include/linux/iio/iio-gts-helper.h

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index b190846c3dc2..52eb46ef84c1 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -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
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 3be08cdadd7e..9622347a1c1b 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -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
diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
new file mode 100644
index 000000000000..812ef6875a5e
--- /dev/null
+++ b/drivers/iio/industrialio-gts-helper.c
@@ -0,0 +1,1057 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* gain-time-scale conversion helpers for IIO light sensors
+ *
+ * Copyright (c) 2023 Matti Vaittinen <mazziesaccount@gmail.com>
+ */
+
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/slab.h>
+#include <linux/sort.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio-gts-helper.h>
+#include <linux/iio/types.h>
+
+/**
+ * iio_gts_get_gain - Convert scale to total gain
+ *
+ * Internal helper for converting scale to total gain.
+ *
+ * @max:	Maximum linearized scale. As an example, when scale is created
+ *		in magnitude of NANOs and max scale is 64.1 - The linearized
+ *		scale is 64 100 000 000.
+ * @scale:	Linearized scale to compute the gain for.
+ *
+ * Return:	(floored) gain corresponding to the scale. -EINVAL if scale
+ *		is invalid.
+ */
+static int iio_gts_get_gain(const u64 max, const u64 scale)
+{
+	u64 full = max;
+	int tmp = 1;
+
+	if (scale > full || !scale)
+		return -EINVAL;
+
+	if (U64_MAX - full < scale) {
+		/* Risk of overflow */
+		if (full - scale < scale)
+			return 1;
+
+		full -= scale;
+		tmp++;
+	}
+
+	while (full > scale * (u64)tmp)
+		tmp++;
+
+	return tmp;
+}
+
+/**
+ * gain_get_scale_fraction - get the gain or time based on scale and known one
+ *
+ * @max:	Maximum linearized scale. As an example, when scale is created
+ *		in magnitude of NANOs and max scale is 64.1 - The linearized
+ *		scale is 64 100 000 000.
+ * @scale:	Linearized scale to compute the gain/time for.
+ * @known:	Either integration time or gain depending on which one is known
+ * @unknown:	Pointer to variable where the computed gain/time is stored
+ *
+ * Internal helper for computing unknown fraction of total gain.
+ * Compute either gain or time based on scale and either the gain or time
+ * depending on which one is known.
+ *
+ * Return:	0 on success.
+ */
+static int gain_get_scale_fraction(const u64 max, u64 scale, int known,
+				   int *unknown)
+{
+	int tot_gain;
+
+	tot_gain = iio_gts_get_gain(max, scale);
+	if (tot_gain < 0)
+		return tot_gain;
+
+	*unknown = tot_gain / known;
+
+	/* We require total gain to be exact multiple of known * unknown */
+	if (!*unknown || *unknown * known != tot_gain)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int iio_gts_delinearize(u64 lin_scale, unsigned long scaler,
+			       int *scale_whole, int *scale_nano)
+{
+	int frac;
+
+	if (scaler > NANO)
+		return -EOVERFLOW;
+
+	if (!scaler)
+		return -EINVAL;
+
+	frac = do_div(lin_scale, scaler);
+
+	*scale_whole = lin_scale;
+	*scale_nano = frac * (NANO / scaler);
+
+	return 0;
+}
+
+static int iio_gts_linearize(int scale_whole, int scale_nano,
+			     unsigned long scaler, u64 *lin_scale)
+{
+	/*
+	 * Expect scale to be (mostly) NANO or MICRO. Divide divider instead of
+	 * multiplication followed by division to avoid overflow.
+	 */
+	if (scaler > NANO || !scaler)
+		return -EINVAL;
+
+	*lin_scale = (u64)scale_whole * (u64)scaler +
+		     (u64)(scale_nano / (NANO / scaler));
+
+	return 0;
+}
+
+/**
+ * iio_gts_total_gain_to_scale - convert gain to scale
+ * @gts:	Gain time scale descriptor
+ * @total_gain:	the gain to be converted
+ * @scale_int:	Pointer to integral part of the scale (typically val1)
+ * @scale_nano:	Pointer to fractional part of the scale (nano or ppb)
+ *
+ * Convert the total gain value to scale. NOTE: This does not separate gain
+ * generated by HW-gain or integration time. It is up to caller to decide what
+ * part of the total gain is due to integration time and what due to HW-gain.
+ *
+ * Return: 0 on success. Negative errno on failure.
+ */
+int iio_gts_total_gain_to_scale(struct iio_gts *gts, int total_gain,
+				int *scale_int, int *scale_nano)
+{
+	u64 tmp;
+
+	tmp = gts->max_scale;
+
+	do_div(tmp, total_gain);
+
+	return iio_gts_delinearize(tmp, NANO, scale_int, scale_nano);
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_total_gain_to_scale, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_purge_avail_scale_table - free-up the available scale tables
+ * @gts:	Gain time scale descriptor
+ *
+ * Free the space reserved by iio_gts_build_avail_scale_table().
+ */
+static void iio_gts_purge_avail_scale_table(struct iio_gts *gts)
+{
+	int i;
+
+	if (gts->per_time_avail_scale_tables) {
+		for (i = 0; i < gts->num_itime; i++)
+			kfree(gts->per_time_avail_scale_tables[i]);
+
+		kfree(gts->per_time_avail_scale_tables);
+		gts->per_time_avail_scale_tables = NULL;
+	}
+
+	kfree(gts->avail_all_scales_table);
+	gts->avail_all_scales_table = NULL;
+
+	gts->num_avail_all_scales = 0;
+}
+
+static int iio_gts_gain_cmp(const void *a, const void *b)
+{
+	return *(int *)a - *(int *)b;
+}
+
+static int gain_to_scaletables(struct iio_gts *gts, int **gains, int **scales)
+{
+	int ret, i, j, new_idx, time_idx;
+	int *all_gains;
+	size_t gain_bytes;
+
+	for (i = 0; i < gts->num_itime; i++) {
+		/*
+		 * Sort the tables for nice output and for easier finding of
+		 * unique values.
+		 */
+		sort(gains[i], gts->num_hwgain, sizeof(int), iio_gts_gain_cmp,
+		     NULL);
+
+		/* Convert gains to scales */
+		for (j = 0; j < gts->num_hwgain; j++) {
+			ret = iio_gts_total_gain_to_scale(gts, gains[i][j],
+							  &scales[i][2 * j],
+							  &scales[i][2 * j + 1]);
+			if (ret)
+				return ret;
+		}
+	}
+
+	gain_bytes = array_size(gts->num_hwgain, sizeof(int));
+	all_gains = kcalloc(gts->num_itime, gain_bytes, GFP_KERNEL);
+	if (!all_gains)
+		return -ENOMEM;
+
+	/*
+	 * We assume all the gains for same integration time were unique.
+	 * It is likely the first time table had greatest time multiplier as
+	 * the times are in the order of preference and greater times are
+	 * usually preferred. Hence we start from the last table which is likely
+	 * to have the smallest total gains.
+	 */
+	time_idx = gts->num_itime - 1;
+	memcpy(all_gains, gains[time_idx], gain_bytes);
+	new_idx = gts->num_hwgain;
+
+	while (time_idx--) {
+		for (j = 0; j < gts->num_hwgain; j++) {
+			int candidate = gains[time_idx][j];
+			int chk;
+
+			if (candidate > all_gains[new_idx - 1]) {
+				all_gains[new_idx] = candidate;
+				new_idx++;
+
+				continue;
+			}
+			for (chk = 0; chk < new_idx; chk++)
+				if (candidate <= all_gains[chk])
+					break;
+
+			if (candidate == all_gains[chk])
+				continue;
+
+			memmove(&all_gains[chk + 1], &all_gains[chk],
+				(new_idx - chk) * sizeof(int));
+			all_gains[chk] = candidate;
+			new_idx++;
+		}
+	}
+
+	gts->avail_all_scales_table = kcalloc(new_idx, 2 * sizeof(int),
+					      GFP_KERNEL);
+	if (!gts->avail_all_scales_table) {
+		ret = -ENOMEM;
+		goto free_out;
+	}
+	gts->num_avail_all_scales = new_idx;
+
+	for (i = 0; i < gts->num_avail_all_scales; i++) {
+		ret = iio_gts_total_gain_to_scale(gts, all_gains[i],
+					&gts->avail_all_scales_table[i * 2],
+					&gts->avail_all_scales_table[i * 2 + 1]);
+
+		if (ret) {
+			kfree(gts->avail_all_scales_table);
+			gts->num_avail_all_scales = 0;
+			goto free_out;
+		}
+	}
+
+free_out:
+	kfree(all_gains);
+
+	return ret;
+}
+
+/**
+ * iio_gts_build_avail_scale_table - create tables of available scales
+ * @gts:	Gain time scale descriptor
+ *
+ * Build the tables which can represent the available scales based on the
+ * originally given gain and time tables. When both time and gain tables are
+ * given this results:
+ * 1. A set of tables representing available scales for each supported
+ *    integration time.
+ * 2. A single table listing all the unique scales that any combination of
+ *    supported gains and times can provide.
+ *
+ * NOTE: Space allocated for the tables must be freed using
+ * iio_gts_purge_avail_scale_table() when the tables are no longer needed.
+ *
+ * Return: 0 on success.
+ */
+static int iio_gts_build_avail_scale_table(struct iio_gts *gts)
+{
+	int **per_time_gains, **per_time_scales, i, j, ret = -ENOMEM;
+
+	per_time_gains = kcalloc(gts->num_itime, sizeof(*per_time_gains), GFP_KERNEL);
+	if (!per_time_gains)
+		return ret;
+
+	per_time_scales = kcalloc(gts->num_itime, sizeof(*per_time_scales), GFP_KERNEL);
+	if (!per_time_scales)
+		goto free_gains;
+
+	for (i = 0; i < gts->num_itime; i++) {
+		per_time_scales[i] = kcalloc(gts->num_hwgain, 2 * sizeof(int),
+					     GFP_KERNEL);
+		if (!per_time_scales[i])
+			goto err_free_out;
+
+		per_time_gains[i] = kcalloc(gts->num_hwgain, sizeof(int),
+					    GFP_KERNEL);
+		if (!per_time_gains[i]) {
+			kfree(per_time_scales[i]);
+			goto err_free_out;
+		}
+
+		for (j = 0; j < gts->num_hwgain; j++)
+			per_time_gains[i][j] = gts->hwgain_table[j].gain *
+					       gts->itime_table[i].mul;
+	}
+
+	ret = gain_to_scaletables(gts, per_time_gains, per_time_scales);
+	if (ret)
+		goto err_free_out;
+
+	kfree(per_time_gains);
+	gts->per_time_avail_scale_tables = per_time_scales;
+
+	return 0;
+
+err_free_out:
+	for (i--; i; i--) {
+		kfree(per_time_scales[i]);
+		kfree(per_time_gains[i]);
+	}
+	kfree(per_time_scales);
+free_gains:
+	kfree(per_time_gains);
+
+	return ret;
+}
+
+/**
+ * iio_gts_build_avail_time_table - build table of available integration times
+ * @gts:	Gain time scale descriptor
+ *
+ * Build the table which can represent the available times to be returned
+ * to users using the read_avail-callback.
+ *
+ * NOTE: Space allocated for the tables must be freed using
+ * iio_gts_purge_avail_time_table() when the tables are no longer needed.
+ *
+ * Return: 0 on success.
+ */
+static int iio_gts_build_avail_time_table(struct iio_gts *gts)
+{
+	int *times, i, j, idx = 0;
+
+	if (!gts->num_itime)
+		return 0;
+
+	times = kcalloc(gts->num_itime, sizeof(int), GFP_KERNEL);
+	if (!times)
+		return -ENOMEM;
+
+	/* Sort times from all tables to one and remove duplicates */
+	for (i = gts->num_itime - 1; i >= 0; i--) {
+		int new = gts->itime_table[i].time_us;
+
+		if (times[idx] < new) {
+			times[idx++] = new;
+			continue;
+		}
+
+		for (j = 0; j <= idx; j++) {
+			if (times[j] > new) {
+				memmove(&times[j + 1], &times[j],
+					(idx - j) * sizeof(int));
+				times[j] = new;
+				idx++;
+			}
+		}
+	}
+	gts->avail_time_tables = times;
+	/*
+	 * This is just to survive a unlikely corner-case where times in the
+	 * given time table were not unique. Else we could just trust the
+	 * gts->num_itime.
+	 */
+	gts->num_avail_time_tables = idx;
+
+	return 0;
+}
+
+/**
+ * iio_gts_purge_avail_time_table - free-up the available integration time table
+ * @gts:	Gain time scale descriptor
+ *
+ * Free the space reserved by iio_gts_build_avail_time_table().
+ */
+static void iio_gts_purge_avail_time_table(struct iio_gts *gts)
+{
+	if (gts->num_avail_time_tables) {
+		kfree(gts->avail_time_tables);
+		gts->avail_time_tables = NULL;
+		gts->num_avail_time_tables = 0;
+	}
+}
+
+/**
+ * iio_gts_build_avail_tables - create tables of available scales and int times
+ * @gts:	Gain time scale descriptor
+ *
+ * Build the tables which can represent the available scales and available
+ * integration times. Availability tables are built based on the originally
+ * given gain and given time tables.
+ *
+ * When both time and gain tables are
+ * given this results:
+ * 1. A set of sorted tables representing available scales for each supported
+ *    integration time.
+ * 2. A single sorted table listing all the unique scales that any combination
+ *    of supported gains and times can provide.
+ * 3. A sorted table of supported integration times
+ *
+ * After these tables are built one can use the iio_gts_all_avail_scales(),
+ * iio_gts_avail_scales_for_time() and iio_gts_avail_times() helpers to
+ * implement the read_avail operations.
+ *
+ * NOTE: Space allocated for the tables must be freed using
+ * iio_gts_purge_avail_tables() when the tables are no longer needed.
+ *
+ * Return: 0 on success.
+ */
+static int iio_gts_build_avail_tables(struct iio_gts *gts)
+{
+	int ret;
+
+	ret = iio_gts_build_avail_scale_table(gts);
+	if (ret)
+		return ret;
+
+	ret = iio_gts_build_avail_time_table(gts);
+	if (ret)
+		iio_gts_purge_avail_scale_table(gts);
+
+	return ret;
+}
+
+/**
+ * iio_gts_purge_avail_tables - free-up the availability tables
+ * @gts:	Gain time scale descriptor
+ *
+ * Free the space reserved by iio_gts_build_avail_tables(). Frees both the
+ * integration time and scale tables.
+ */
+static void iio_gts_purge_avail_tables(struct iio_gts *gts)
+{
+	iio_gts_purge_avail_time_table(gts);
+	iio_gts_purge_avail_scale_table(gts);
+}
+
+static void devm_iio_gts_avail_all_drop(void *res)
+{
+	iio_gts_purge_avail_tables(res);
+}
+
+/**
+ * devm_iio_gts_build_avail_tables - manged add availability tables
+ * @dev:	Pointer to the device whose lifetime tables are bound
+ * @gts:	Gain time scale descriptor
+ *
+ * Build the tables which can represent the available scales and available
+ * integration times. Availability tables are built based on the originally
+ * given gain and given time tables.
+ *
+ * When both time and gain tables are given this results:
+ * 1. A set of sorted tables representing available scales for each supported
+ *    integration time.
+ * 2. A single sorted table listing all the unique scales that any combination
+ *    of supported gains and times can provide.
+ * 3. A sorted table of supported integration times
+ *
+ * After these tables are built one can use the iio_gts_all_avail_scales(),
+ * iio_gts_avail_scales_for_time() and iio_gts_avail_times() helpers to
+ * implement the read_avail operations.
+ *
+ * The tables are automatically released upon device detach.
+ *
+ * Return: 0 on success.
+ */
+static int devm_iio_gts_build_avail_tables(struct device *dev,
+					   struct iio_gts *gts)
+{
+	int ret;
+
+	ret = iio_gts_build_avail_tables(gts);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(dev, devm_iio_gts_avail_all_drop, gts);
+}
+
+static int sanity_check_time(const struct iio_itime_sel_mul *t)
+{
+	if (t->sel < 0 || t->time_us < 0 || t->mul <= 0)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int sanity_check_gain(const struct iio_gain_sel_pair *g)
+{
+	if (g->sel < 0 || g->gain <= 0)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int iio_gts_sanity_check(struct iio_gts *gts)
+{
+	int g, t, ret;
+
+	if (!gts->num_hwgain && !gts->num_itime)
+		return -EINVAL;
+
+	for (t = 0; t < gts->num_itime; t++) {
+		ret = sanity_check_time(&gts->itime_table[t]);
+		if (ret)
+			return ret;
+	}
+
+	for (g = 0; g < gts->num_hwgain; g++) {
+		ret = sanity_check_gain(&gts->hwgain_table[g]);
+		if (ret)
+			return ret;
+	}
+
+	for (g = 0; g < gts->num_hwgain; g++) {
+		for (t = 0; t < gts->num_itime; t++) {
+			int gain, mul, res;
+
+			gain = gts->hwgain_table[g].gain;
+			mul = gts->itime_table[t].mul;
+
+			if (check_mul_overflow(gain, mul, &res))
+				return -EOVERFLOW;
+		}
+	}
+
+	return 0;
+}
+
+static int iio_init_iio_gts(int max_scale_int, int max_scale_nano,
+			const struct iio_gain_sel_pair *gain_tbl, int num_gain,
+			const struct iio_itime_sel_mul *tim_tbl, int num_times,
+			struct iio_gts *gts)
+{
+	int ret;
+
+	memset(gts, 0, sizeof(*gts));
+
+	ret = iio_gts_linearize(max_scale_int, max_scale_nano, NANO,
+				   &gts->max_scale);
+	if (ret)
+		return ret;
+
+	gts->hwgain_table = gain_tbl;
+	gts->num_hwgain = num_gain;
+	gts->itime_table = tim_tbl;
+	gts->num_itime = num_times;
+
+	return iio_gts_sanity_check(gts);
+}
+
+/**
+ * devm_iio_init_iio_gts - Initialize the gain-time-scale helper
+ * @dev:		Pointer to the device whose lifetime gts resources are
+ *			bound
+ * @max_scale_int:	integer part of the maximum scale value
+ * @max_scale_nano:	fraction part of the maximum scale value
+ * @gain_tbl:		table describing supported gains
+ * @num_gain:		number of gains in the gain table
+ * @tim_tbl:		table describing supported integration times. Provide
+ *			the integration time table sorted so that the preferred
+ *			integration time is in the first array index. The search
+ *			functions like the
+ *			iio_gts_find_time_and_gain_sel_for_scale() start search
+ *			from first provided time.
+ * @num_times:		number of times in the time table
+ * @gts:		pointer to the helper struct
+ *
+ * Initialize the gain-time-scale helper for use. Note, gains, times, selectors
+ * and multipliers must be positive. Negative values are reserved for error
+ * checking. The total gain (maximum gain * maximum time multiplier) must not
+ * overflow int. The allocated resources will be released upon device detach.
+ *
+ * Return: 0 on success.
+ */
+int devm_iio_init_iio_gts(struct device *dev, int max_scale_int, int max_scale_nano,
+			  const struct iio_gain_sel_pair *gain_tbl, int num_gain,
+			  const struct iio_itime_sel_mul *tim_tbl, int num_times,
+			  struct iio_gts *gts)
+{
+	int ret;
+
+	ret = iio_init_iio_gts(max_scale_int, max_scale_nano, gain_tbl,
+			       num_gain, tim_tbl, num_times, gts);
+	if (ret)
+		return ret;
+
+	return devm_iio_gts_build_avail_tables(dev, gts);
+}
+EXPORT_SYMBOL_NS_GPL(devm_iio_init_iio_gts, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_all_avail_scales - helper for listing all available scales
+ * @gts:	Gain time scale descriptor
+ * @vals:	Returned array of supported scales
+ * @type:	Type of returned scale values
+ * @length:	Amount of returned values in array
+ *
+ * Return: a value suitable to be returned from read_avail or a negative error.
+ */
+int iio_gts_all_avail_scales(struct iio_gts *gts, const int **vals, int *type,
+			     int *length)
+{
+	if (!gts->num_avail_all_scales)
+		return -EINVAL;
+
+	*vals = gts->avail_all_scales_table;
+	*type = IIO_VAL_INT_PLUS_NANO;
+	*length = gts->num_avail_all_scales * 2;
+
+	return IIO_AVAIL_LIST;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_all_avail_scales, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_avail_scales_for_time - list scales for integration time
+ * @gts:	Gain time scale descriptor
+ * @time:	Integration time for which the scales are listed
+ * @vals:	Returned array of supported scales
+ * @type:	Type of returned scale values
+ * @length:	Amount of returned values in array
+ *
+ * Drivers which do not allow scale setting to change integration time can
+ * use this helper to list only the scales which are valid for given integration
+ * time.
+ *
+ * Return: a value suitable to be returned from read_avail or a negative error.
+ */
+int iio_gts_avail_scales_for_time(struct iio_gts *gts, int time,
+				  const int **vals, int *type, int *length)
+{
+	int i;
+
+	for (i = 0; i < gts->num_itime; i++)
+		if (gts->itime_table[i].time_us == time)
+			break;
+
+	if (i == gts->num_itime)
+		return -EINVAL;
+
+	*vals = gts->per_time_avail_scale_tables[i];
+	*type = IIO_VAL_INT_PLUS_NANO;
+	*length = gts->num_hwgain * 2;
+
+	return IIO_AVAIL_LIST;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_avail_scales_for_time, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_avail_times - helper for listing available integration times
+ * @gts:	Gain time scale descriptor
+ * @vals:	Returned array of supported times
+ * @type:	Type of returned scale values
+ * @length:	Amount of returned values in array
+ *
+ * Return: a value suitable to be returned from read_avail or a negative error.
+ */
+int iio_gts_avail_times(struct iio_gts *gts,  const int **vals, int *type,
+			int *length)
+{
+	if (!gts->num_avail_time_tables)
+		return -EINVAL;
+
+	*vals = gts->avail_time_tables;
+	*type = IIO_VAL_INT;
+	*length = gts->num_avail_time_tables;
+
+	return IIO_AVAIL_LIST;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_avail_times, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_find_sel_by_gain - find selector corresponding to a HW-gain
+ * @gts:	Gain time scale descriptor
+ * @gain:	HW-gain for which matching selector is searched for
+ *
+ * Return:	a selector matching given HW-gain or -EINVAL if selector was
+ *		not found.
+ */
+int iio_gts_find_sel_by_gain(struct iio_gts *gts, int gain)
+{
+	int i;
+
+	for (i = 0; i < gts->num_hwgain; i++)
+		if (gts->hwgain_table[i].gain == gain)
+			return gts->hwgain_table[i].sel;
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_find_sel_by_gain, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_find_gain_by_sel - find HW-gain corresponding to a selector
+ * @gts:	Gain time scale descriptor
+ * @sel:	selector for which matching HW-gain is searched for
+ *
+ * Return:	a HW-gain matching given selector or -EINVAL if HW-gain was not
+ *		found.
+ */
+int iio_gts_find_gain_by_sel(struct iio_gts *gts, int sel)
+{
+	int i;
+
+	for (i = 0; i < gts->num_hwgain; i++)
+		if (gts->hwgain_table[i].sel == sel)
+			return gts->hwgain_table[i].gain;
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_find_gain_by_sel, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_get_min_gain - find smallest valid HW-gain
+ * @gts:	Gain time scale descriptor
+ *
+ * Return:	The smallest HW-gain -EINVAL if no HW-gains were in the tables.
+ */
+int iio_gts_get_min_gain(struct iio_gts *gts)
+{
+	int i, min = -EINVAL;
+
+	for (i = 0; i < gts->num_hwgain; i++) {
+		int gain = gts->hwgain_table[i].gain;
+
+		if (min == -EINVAL)
+			min = gain;
+		else
+			min = min(min, gain);
+	}
+
+	return min;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_get_min_gain, IIO_GTS_HELPER);
+
+/**
+ * iio_find_closest_gain_low - Find the closest lower matching gain
+ * @gts:	Gain time scale descriptor
+ * @gain:	HW-gain for which the closest match is searched
+ * @in_range:	indicate if the @gain was actually in the range of
+ *		supported gains.
+ *
+ * Search for closest supported gain that is lower than or equal to the
+ * gain given as a parameter. This is usable for drivers which do not require
+ * user to request exact matching gain but rather for rounding to a supported
+ * gain value which is equal or lower (setting lower gain is typical for
+ * avoiding saturation)
+ *
+ * Return:	The closest matching supported gain or -EINVAL if @gain
+ *		was smaller than the smallest supported gain.
+ */
+int iio_find_closest_gain_low(struct iio_gts *gts, int gain, bool *in_range)
+{
+	int i, diff = 0;
+	int best = -1;
+
+	*in_range = false;
+
+	for (i = 0; i < gts->num_hwgain; i++) {
+		if (gain == gts->hwgain_table[i].gain) {
+			*in_range = true;
+			return gain;
+		}
+
+		if (gain > gts->hwgain_table[i].gain) {
+			if (!diff) {
+				diff = gain - gts->hwgain_table[i].gain;
+				best = i;
+			} else {
+				int tmp = gain - gts->hwgain_table[i].gain;
+
+				if (tmp < diff) {
+					diff = tmp;
+					best = i;
+				}
+			}
+		} else {
+			/*
+			 * We found valid HW-gain which is greater than
+			 * reference. So, unless we return a failure below we
+			 * will have found an in-range gain
+			 */
+			*in_range = true;
+		}
+	}
+	/* The requested gain was smaller than anything we support */
+	if (!diff) {
+		*in_range = false;
+
+		return -EINVAL;
+	}
+
+	return gts->hwgain_table[best].gain;
+}
+EXPORT_SYMBOL_NS_GPL(iio_find_closest_gain_low, IIO_GTS_HELPER);
+
+static int iio_gts_get_int_time_gain_multiplier_by_sel(struct iio_gts *gts,
+						       int sel)
+{
+	const struct iio_itime_sel_mul *time;
+
+	time = iio_gts_find_itime_by_sel(gts, sel);
+	if (!time)
+		return -EINVAL;
+
+	return time->mul;
+}
+
+/**
+ * iio_gts_find_gain_for_scale_using_time - Find gain by time and scale
+ * @gts:	Gain time scale descriptor
+ * @time_sel:	Integration time selector corresponding to the time gain is
+ *		searched for
+ * @scale_int:	Integral part of the scale (typically val1)
+ * @scale_nano:	Fractional part of the scale (nano or ppb)
+ * @gain:	Pointer to value where gain is stored.
+ *
+ * In some cases the light sensors may want to find a gain setting which
+ * corresponds given scale and integration time. Sensors which fill the
+ * gain and time tables may use this helper to retrieve the gain.
+ *
+ * Return:	0 on success. -EINVAL if gain matching the parameters is not
+ *		found.
+ */
+static int iio_gts_find_gain_for_scale_using_time(struct iio_gts *gts, int time_sel,
+						  int scale_int, int scale_nano,
+						  int *gain)
+{
+	u64 scale_linear;
+	int ret, mul;
+
+	ret = iio_gts_linearize(scale_int, scale_nano, NANO, &scale_linear);
+	if (ret)
+		return ret;
+
+	ret = iio_gts_get_int_time_gain_multiplier_by_sel(gts, time_sel);
+	if (ret < 0)
+		return ret;
+
+	mul = ret;
+
+	ret = gain_get_scale_fraction(gts->max_scale, scale_linear, mul, gain);
+	if (ret)
+		return ret;
+
+	if (!iio_gts_valid_gain(gts, *gain))
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * iio_gts_find_gain_sel_for_scale_using_time - Fetch gain selector.
+ * @gts:	Gain time scale descriptor
+ * @time_sel:	Integration time selector corresponding to the time gain is
+ *		searched for
+ * @scale_int:	Integral part of the scale (typically val1)
+ * @scale_nano:	Fractional part of the scale (nano or ppb)
+ * @gain_sel:	Pointer to value where gain selector is stored.
+ *
+ * See iio_gts_find_gain_for_scale_using_time() for more information
+ */
+int iio_gts_find_gain_sel_for_scale_using_time(struct iio_gts *gts, int time_sel,
+					       int scale_int, int scale_nano,
+					       int *gain_sel)
+{
+	int gain, ret;
+
+	ret = iio_gts_find_gain_for_scale_using_time(gts, time_sel, scale_int,
+						     scale_nano, &gain);
+	if (ret)
+		return ret;
+
+	ret = iio_gts_find_sel_by_gain(gts, gain);
+	if (ret < 0)
+		return ret;
+
+	*gain_sel = ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_find_gain_sel_for_scale_using_time, IIO_GTS_HELPER);
+
+static int iio_gts_get_total_gain(struct iio_gts *gts, int gain, int time)
+{
+	const struct iio_itime_sel_mul *itime;
+
+	if (!iio_gts_valid_gain(gts, gain))
+		return -EINVAL;
+
+	if (!gts->num_itime)
+		return gain;
+
+	itime = iio_gts_find_itime_by_time(gts, time);
+	if (!itime)
+		return -EINVAL;
+
+	return gain * itime->mul;
+}
+
+static int iio_gts_get_scale_linear(struct iio_gts *gts, int gain, int time,
+				    u64 *scale)
+{
+	int total_gain;
+	u64 tmp;
+
+	total_gain = iio_gts_get_total_gain(gts, gain, time);
+	if (total_gain < 0)
+		return total_gain;
+
+	tmp = gts->max_scale;
+
+	do_div(tmp, total_gain);
+
+	*scale = tmp;
+
+	return 0;
+}
+
+/**
+ * iio_gts_get_scale - get scale based on integration time and HW-gain
+ * @gts:	Gain time scale descriptor
+ * @gain:	HW-gain for which the scale is computed
+ * @time:	Integration time for which the scale is computed
+ * @scale_int:	Integral part of the scale (typically val1)
+ * @scale_nano:	Fractional part of the scale (nano or ppb)
+ *
+ * Compute scale matching the integration time and HW-gain given as parameter.
+ *
+ * Return: 0 on success.
+ */
+int iio_gts_get_scale(struct iio_gts *gts, int gain, int time, int *scale_int,
+		      int *scale_nano)
+{
+	u64 lin_scale;
+	int ret;
+
+	ret = iio_gts_get_scale_linear(gts, gain, time, &lin_scale);
+	if (ret)
+		return ret;
+
+	return iio_gts_delinearize(lin_scale, NANO, scale_int, scale_nano);
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_get_scale, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_find_new_gain_sel_by_old_gain_time - compensate for time change
+ * @gts:		Gain time scale descriptor
+ * @old_gain:		Previously set gain
+ * @old_time_sel:	Selector corresponding previously set time
+ * @new_time_sel:	Selector corresponding new time to be set
+ * @new_gain:		Pointer to value where new gain is to be written
+ *
+ * We may want to mitigate the scale change caused by setting a new integration
+ * time (for a light sensor) by also updating the (HW)gain. This helper computes
+ * new gain value to maintain the scale with new integration time.
+ *
+ * Return: 0 on success. -EINVAL if gain matching the new time is not found.
+ */
+int iio_gts_find_new_gain_sel_by_old_gain_time(struct iio_gts *gts,
+					       int old_gain, int old_time_sel,
+					       int new_time_sel, int *new_gain)
+{
+	const struct iio_itime_sel_mul *itime_old, *itime_new;
+	u64 scale;
+	int ret;
+
+	itime_old = iio_gts_find_itime_by_sel(gts, old_time_sel);
+	if (!itime_old)
+		return -EINVAL;
+
+	itime_new = iio_gts_find_itime_by_sel(gts, new_time_sel);
+	if (!itime_new)
+		return -EINVAL;
+
+	ret = iio_gts_get_scale_linear(gts, old_gain, itime_old->time_us,
+				       &scale);
+	if (ret)
+		return ret;
+
+	ret = gain_get_scale_fraction(gts->max_scale, scale, itime_new->mul,
+				      new_gain);
+	if (ret)
+		return ret;
+
+	if (!iio_gts_valid_gain(gts, *new_gain))
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_find_new_gain_sel_by_old_gain_time, IIO_GTS_HELPER);
+
+/**
+ * iio_gts_find_new_gain_by_old_gain_time - compensate for time change
+ * @gts:		Gain time scale descriptor
+ * @old_gain:		Previously set gain
+ * @old_time:		Selector corresponding previously set time
+ * @new_time:		Selector corresponding new time to be set
+ * @new_gain:		Pointer to value where new gain is to be written
+ *
+ * We may want to mitigate the scale change caused by setting a new integration
+ * time (for a light sensor) by also updating the (HW)gain. This helper computes
+ * new gain value to maintain the scale with new integration time.
+ *
+ * Return: 0 on success. -EINVAL if gain matching the new time is not found.
+ */
+int iio_gts_find_new_gain_by_old_gain_time(struct iio_gts *gts, int old_gain,
+					   int old_time, int new_time,
+					   int *new_gain)
+{
+	const struct iio_itime_sel_mul *itime_new;
+	u64 scale;
+	int ret;
+
+	itime_new = iio_gts_find_itime_by_time(gts, new_time);
+	if (!itime_new)
+		return -EINVAL;
+
+	ret = iio_gts_get_scale_linear(gts, old_gain, old_time, &scale);
+	if (ret)
+		return ret;
+
+	ret = gain_get_scale_fraction(gts->max_scale, scale, itime_new->mul,
+				      new_gain);
+	if (ret)
+		return ret;
+
+	if (!iio_gts_valid_gain(gts, *new_gain))
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(iio_gts_find_new_gain_by_old_gain_time, IIO_GTS_HELPER);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Matti Vaittinen <mazziesaccount@gmail.com>");
+MODULE_DESCRIPTION("IIO light sensor gain-time-scale helpers");
diff --git a/include/linux/iio/iio-gts-helper.h b/include/linux/iio/iio-gts-helper.h
new file mode 100644
index 000000000000..dd64e544a3da
--- /dev/null
+++ b/include/linux/iio/iio-gts-helper.h
@@ -0,0 +1,206 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* gain-time-scale conversion helpers for IIO light sensors
+ *
+ * Copyright (c) 2023 Matti Vaittinen <mazziesaccount@gmail.com>
+ */
+
+#ifndef __IIO_GTS_HELPER__
+#define __IIO_GTS_HELPER__
+
+#include <linux/types.h>
+
+struct device;
+
+/**
+ * struct iio_gain_sel_pair - gain - selector values
+ *
+ * In many cases devices like light sensors allow setting signal amplification
+ * (gain) using a register interface. This structure describes amplification
+ * and corresponding selector (register value)
+ *
+ * @gain:	Gain (multiplication) value. Gain must be positive, negative
+ *		values are reserved for error handling.
+ * @sel:	Selector (usually register value) used to indicate this gain.
+ *		NOTE: Only selectors >= 0 supported.
+ */
+struct iio_gain_sel_pair {
+	int gain;
+	int sel;
+};
+
+/**
+ * struct iio_itime_sel_mul - integration time description
+ *
+ * In many cases devices like light sensors allow setting the duration of
+ * collecting data. Typically this duration has also an impact to the magnitude
+ * of measured values (gain). This structure describes the relation of
+ * integration time and amplification as well as corresponding selector
+ * (register value).
+ *
+ * An example could be a sensor allowing 50, 100, 200 and 400 mS times. The
+ * respective multiplication values could be 50 mS => 1, 100 mS => 2,
+ * 200 mS => 4 and 400 mS => 8 assuming the impact of integration time would be
+ * linear in a way that when collecting data for 50 mS caused value X, doubling
+ * the data collection time caused value 2X etc.
+ *
+ * @time_us:	Integration time in microseconds. Time values must be positive,
+ *		negative values are reserved for error handling.
+ * @sel:	Selector (usually register value) used to indicate this time
+ *		NOTE: Only selectors >= 0 supported.
+ * @mul:	Multiplication to the values caused by this time.
+ *		NOTE: Only multipliers > 0 supported.
+ */
+struct iio_itime_sel_mul {
+	int time_us;
+	int sel;
+	int mul;
+};
+
+struct iio_gts {
+	u64 max_scale;
+	const struct iio_gain_sel_pair *hwgain_table;
+	int num_hwgain;
+	const struct iio_itime_sel_mul *itime_table;
+	int num_itime;
+	int **per_time_avail_scale_tables;
+	int *avail_all_scales_table;
+	int num_avail_all_scales;
+	int *avail_time_tables;
+	int num_avail_time_tables;
+};
+
+#define GAIN_SCALE_GAIN(_gain, _sel)			\
+{							\
+	.gain = (_gain),				\
+	.sel = (_sel),					\
+}
+
+#define GAIN_SCALE_ITIME_US(_itime, _sel, _mul)		\
+{							\
+	.time_us = (_itime),				\
+	.sel = (_sel),					\
+	.mul = (_mul),					\
+}
+
+static inline const struct iio_itime_sel_mul *
+iio_gts_find_itime_by_time(struct iio_gts *gts, int time)
+{
+	int i;
+
+	if (!gts->num_itime)
+		return NULL;
+
+	for (i = 0; i < gts->num_itime; i++)
+		if (gts->itime_table[i].time_us == time)
+			return &gts->itime_table[i];
+
+	return NULL;
+}
+
+static inline const struct iio_itime_sel_mul *
+iio_gts_find_itime_by_sel(struct iio_gts *gts, int sel)
+{
+	int i;
+
+	for (i = 0; i < gts->num_itime; i++)
+		if (gts->itime_table[i].sel == sel)
+			return &gts->itime_table[i];
+
+	return NULL;
+}
+
+int devm_iio_init_iio_gts(struct device *dev, int max_scale_int, int max_scale_nano,
+			  const struct iio_gain_sel_pair *gain_tbl, int num_gain,
+			  const struct iio_itime_sel_mul *tim_tbl, int num_times,
+			  struct iio_gts *gts);
+/**
+ * iio_gts_find_int_time_by_sel - find integration time matching a selector
+ * @gts:	Gain time scale descriptor
+ * @sel:	selector for which matching integration time is searched for
+ *
+ * Return:	integration time matching given selector or -EINVAL if
+ *		integration time was not found.
+ */
+static inline int iio_gts_find_int_time_by_sel(struct iio_gts *gts, int sel)
+{
+	const struct iio_itime_sel_mul *itime;
+
+	itime = iio_gts_find_itime_by_sel(gts, sel);
+	if (!itime)
+		return -EINVAL;
+
+	return itime->time_us;
+}
+
+/**
+ * iio_gts_find_sel_by_int_time - find selector matching integration time
+ * @gts:	Gain time scale descriptor
+ * @gain:	HW-gain for which matching selector is searched for
+ *
+ * Return:	a selector matching given integration time or -EINVAL if
+ *		selector was not found.
+ */
+static inline int iio_gts_find_sel_by_int_time(struct iio_gts *gts, int time)
+{
+	const struct iio_itime_sel_mul *itime;
+
+	itime = iio_gts_find_itime_by_time(gts, time);
+	if (!itime)
+		return -EINVAL;
+
+	return itime->sel;
+}
+
+/**
+ * iio_gts_valid_time - check if given integration time is valid
+ * @gts:	Gain time scale descriptor
+ * @time_us:	Integration time to check
+ *
+ * Return:	True if given time is supported by device. False if not.
+ */
+static inline bool iio_gts_valid_time(struct iio_gts *gts, int time_us)
+{
+	return iio_gts_find_itime_by_time(gts, time_us) != NULL;
+}
+
+int iio_gts_find_sel_by_gain(struct iio_gts *gts, int gain);
+
+/**
+ * iio_gts_valid_gain - check if given HW-gain is valid
+ * @gts:	Gain time scale descriptor
+ * @gain:	HW-gain to check
+ *
+ * Return:	True if given time is supported by device. False if not.
+ */
+static inline bool iio_gts_valid_gain(struct iio_gts *gts, int gain)
+{
+	return iio_gts_find_sel_by_gain(gts, gain) >= 0;
+}
+
+int iio_find_closest_gain_low(struct iio_gts *gts, int gain, bool *in_range);
+int iio_gts_find_gain_by_sel(struct iio_gts *gts, int sel);
+int iio_gts_get_min_gain(struct iio_gts *gts);
+int iio_gts_find_int_time_by_sel(struct iio_gts *gts, int sel);
+int iio_gts_find_sel_by_int_time(struct iio_gts *gts, int time);
+
+int iio_gts_total_gain_to_scale(struct iio_gts *gts, int total_gain,
+				int *scale_int, int *scale_nano);
+int iio_gts_find_gain_sel_for_scale_using_time(struct iio_gts *gts, int time_sel,
+					       int scale_int, int scale_nano,
+					       int *gain_sel);
+int iio_gts_get_scale(struct iio_gts *gts, int gain, int time, int *scale_int,
+		      int *scale_nano);
+int iio_gts_find_new_gain_sel_by_old_gain_time(struct iio_gts *gts,
+					       int old_gain, int old_time_sel,
+					       int new_time_sel, int *new_gain);
+int iio_gts_find_new_gain_by_old_gain_time(struct iio_gts *gts, int old_gain,
+					   int old_time, int new_time,
+					   int *new_gain);
+int iio_gts_avail_times(struct iio_gts *gts,  const int **vals, int *type,
+			int *length);
+int iio_gts_all_avail_scales(struct iio_gts *gts, const int **vals, int *type,
+			     int *length);
+int iio_gts_avail_scales_for_time(struct iio_gts *gts, int time,
+				  const int **vals, int *type, int *length);
+
+#endif
-- 
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 =] 

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation
  2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
  2023-03-27 11:27 ` [PATCH v6 1/7] dt-bindings: iio: light: Support ROHM BU27034 Matti Vaittinen
  2023-03-27 11:28 ` [PATCH v6 2/7] iio: light: Add gain-time-scale helpers Matti Vaittinen
@ 2023-03-27 11:34 ` Matti Vaittinen
  2023-03-27 12:01   ` Greg Kroah-Hartman
  2023-03-27 11:34 ` [PATCH v6 4/7] iio: test: test gain-time-scale helpers Matti Vaittinen
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:34 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Brendan Higgins, David Gow, linux-kernel, linux-kselftest,
	kunit-dev, Stephen Boyd, Jonathan Cameron, Greg Kroah-Hartman,
	linux-iio

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

A few tests need to have a valid struct device. One such example is
tests which want to be testing devm-managed interfaces.

Add kunit wrapper for root_device_[un]register(), which create a root
device and also add a kunit managed clean-up routine for the device
destruction upon test exit.

Special note: In some cases the device reference-count does not reach
zero and devm-unwinding is not done if device is not sitting on a bus.
The root_device_[un]register() are dealing with such devices and thus
this interface may not be usable by all in its current form. More
information can be found from:
https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/

The use of root-devices in the kunit helpers is intended to be an
intermediate solution to allow tests which do not require device to sit
on a bus avoid directly abusing the root_device_[un]register() while
proper kunit device solution is being worked on. Related discussion can be
found from:
https://lore.kernel.org/lkml/CABVgOSmx3A4Vwos2_8xO-XQrQAw5gvY0nc5zLpLmcJ7FtA-dTQ@mail.gmail.com/

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

---
Change history:
v5 => v6:
- Kunit resource-managed root_device creation wrapper (new patch)

Please note: This patch uses root-devices (as was suggested) until there
is a proper dummy device creation mechanism added in kunit. The root
devices are embedded in kunit wrappers to simplify replacing the
root-devices with proper solution when it is available.

David Gow has sent out an RFC[1] which should implement these helpers
using not-yet-in-tree deferring API. This RFC aims to support
kunit_device which should be _the right thing to do_. I added this
implementation here because it may (or may not) take a while for the David's
RFC to make it's way in-kernel. So, in order to not delay this series I
added these helpers which use the existing kunit resource management for
clean-up while the new deferring kunit API is not yet in place.

[1] https://lore.kernel.org/linux-kselftest/20230325043104.3761770-1-davidgow@google.com/T/#mf797239a8bce11630875fdf60aab9ed627add1f0

---
 include/kunit/device.h | 18 ++++++++++++++++++
 lib/kunit/Makefile     |  3 ++-
 lib/kunit/device.c     | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 include/kunit/device.h
 create mode 100644 lib/kunit/device.c

diff --git a/include/kunit/device.h b/include/kunit/device.h
new file mode 100644
index 000000000000..f02740b7583b
--- /dev/null
+++ b/include/kunit/device.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __KUNIT_DEVICE_H__
+#define __KUNIT_DEVICE_H__
+
+#include <kunit/test.h>
+
+struct device;
+
+/* Register a new device against a KUnit test. */
+struct device *kunit_device_register(struct kunit *test, const char *name);
+/*
+ * Unregister a device created by kunit_device_register() early (i.e.,
+ * before test cleanup).
+ */
+void kunit_device_unregister(struct kunit *test, struct device *dev);
+
+#endif
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index cb417f504996..64449549b990 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -6,7 +6,8 @@ kunit-objs +=				test.o \
 					string-stream.o \
 					assert.o \
 					try-catch.o \
-					executor.o
+					executor.o \
+					device.o
 
 ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
 kunit-objs +=				debugfs.o
diff --git a/lib/kunit/device.c b/lib/kunit/device.c
new file mode 100644
index 000000000000..425f6d62ebd7
--- /dev/null
+++ b/lib/kunit/device.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <kunit/device.h>
+#include <kunit/test.h>
+
+#include <linux/device.h>
+
+static void kunit_device_drop(struct kunit_resource *res)
+{
+	root_device_unregister(res->data);
+}
+
+struct device *kunit_device_register(struct kunit *test, const char *name)
+{
+	struct device *dev;
+
+	dev = root_device_register(name);
+	if (IS_ERR_OR_NULL(dev))
+		return dev;
+
+	return kunit_alloc_resource(test, NULL, kunit_device_drop, GFP_KERNEL,
+				    dev);
+}
+EXPORT_SYMBOL_GPL(kunit_device_register);
+
+static bool kunit_device_match(struct kunit *test, struct kunit_resource *res,
+			       void *match_data)
+{
+	return res->data == match_data;
+}
+
+void kunit_device_unregister(struct kunit *test, struct device *dev)
+{
+	kunit_destroy_resource(test, kunit_device_match, dev);
+}
+EXPORT_SYMBOL_GPL(kunit_device_unregister);
-- 
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 =] 

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 4/7] iio: test: test gain-time-scale helpers
  2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
                   ` (2 preceding siblings ...)
  2023-03-27 11:34 ` [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation Matti Vaittinen
@ 2023-03-27 11:34 ` Matti Vaittinen
  2023-03-27 11:34 ` [PATCH v6 5/7] MAINTAINERS: Add IIO " Matti Vaittinen
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:34 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Jonathan Cameron, Lars-Peter Clausen, Matti Vaittinen,
	Liam Beguin, Randy Dunlap, Masahiro Yamada, linux-kernel,
	linux-iio, Stephen Boyd, David Gow, Greg Kroah-Hartman

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

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.
Hence some gain-time-scale helpers were introduced.

Add some simple tests to verify the most hairy functions.

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

---
Changes:
v5 => v6:
- Use kunit root-device registration wrapper

v4 => v5:
- remove empty lines from Kconfig
- adapt to drop of the non devm iio_init
- test also init with couple of invalid tables

v3 => v4:
- use dummy device to test devm interfaces
- adapt to the new header location
- drop tests for dropped interfaces

v2 => v3:
- Use namespace for iio-gts-helpers

RFCv1 => v2:
- add tests for available scales/times helpers
- adapt to renamed iio-gts-helpers.h header
---
 drivers/iio/test/Kconfig        |  14 +
 drivers/iio/test/Makefile       |   1 +
 drivers/iio/test/iio-test-gts.c | 517 ++++++++++++++++++++++++++++++++
 3 files changed, 532 insertions(+)
 create mode 100644 drivers/iio/test/iio-test-gts.c

diff --git a/drivers/iio/test/Kconfig b/drivers/iio/test/Kconfig
index 0b6e4e278a2f..33cca49c8058 100644
--- a/drivers/iio/test/Kconfig
+++ b/drivers/iio/test/Kconfig
@@ -4,6 +4,20 @@
 #
 
 # Keep in alphabetical order
+config IIO_GTS_KUNIT_TEST
+	tristate "Test IIO formatting functions" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	select IIO_GTS_HELPER
+	select TEST_KUNIT_DEVICE_HELPERS
+	default KUNIT_ALL_TESTS
+	help
+	  build unit tests for the IIO light sensor gain-time-scale helpers.
+
+	  For more information on KUnit and unit tests in general, please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N. Keep in alphabetical order
+
 config IIO_RESCALE_KUNIT_TEST
 	tristate "Test IIO rescale conversion functions" if !KUNIT_ALL_TESTS
 	depends on KUNIT && IIO_RESCALE
diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
index d76eaf36da82..e9a4cf1ff57f 100644
--- a/drivers/iio/test/Makefile
+++ b/drivers/iio/test/Makefile
@@ -6,4 +6,5 @@
 # Keep in alphabetical order
 obj-$(CONFIG_IIO_RESCALE_KUNIT_TEST) += iio-test-rescale.o
 obj-$(CONFIG_IIO_FORMAT_KUNIT_TEST) += iio-test-format.o
+obj-$(CONFIG_IIO_GTS_KUNIT_TEST) += iio-test-gts.o
 CFLAGS_iio-test-format.o += $(DISABLE_STRUCTLEAK_PLUGIN)
diff --git a/drivers/iio/test/iio-test-gts.c b/drivers/iio/test/iio-test-gts.c
new file mode 100644
index 000000000000..02d69d36cdbe
--- /dev/null
+++ b/drivers/iio/test/iio-test-gts.c
@@ -0,0 +1,517 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Unit tests for IIO light sensor gain-time-scale helpers
+ *
+ * Copyright (c) 2023 Matti Vaittinen <mazziesaccount@gmail.com>
+ */
+
+#include <kunit/device.h>
+#include <kunit/test.h>
+#include <linux/device.h>
+#include <linux/iio/iio-gts-helper.h>
+#include <linux/iio/types.h>
+
+/*
+ * Please, read the "rant" from the top of the lib/test_linear_ranges.c if
+ * you see a line of helper code which is not being tested.
+ *
+ * Then, please look at the line which is not being tested. Is this line
+ * somehow unusually complex? If answer is "no", then chances are that the
+ * "development inertia" caused by adding a test exceeds the benefits.
+ *
+ * If yes, then adding a test is probably a good idea but please stop for a
+ * moment and consider the effort of changing all the tests when code gets
+ * refactored. Eventually it neeeds to be.
+ */
+
+#define TEST_TSEL_50		1
+#define TEST_TSEL_X_MIN		TEST_TSEL_50
+#define TEST_TSEL_100		0
+#define TEST_TSEL_200		2
+#define TEST_TSEL_400		4
+#define TEST_TSEL_X_MAX		TEST_TSEL_400
+
+#define TEST_GSEL_1		0x00
+#define TEST_GSEL_X_MIN		TEST_GSEL_1
+#define TEST_GSEL_4		0x08
+#define TEST_GSEL_16		0x0a
+#define TEST_GSEL_32		0x0b
+#define TEST_GSEL_64		0x0c
+#define TEST_GSEL_256		0x18
+#define TEST_GSEL_512		0x19
+#define TEST_GSEL_1024		0x1a
+#define TEST_GSEL_2048		0x1b
+#define TEST_GSEL_4096		0x1c
+#define TEST_GSEL_X_MAX		TEST_GSEL_4096
+
+#define TEST_SCALE_1X		64
+#define TEST_SCALE_MIN_X	TEST_SCALE_1X
+#define TEST_SCALE_2X		32
+#define TEST_SCALE_4X		16
+#define TEST_SCALE_8X		8
+#define TEST_SCALE_16X		4
+#define TEST_SCALE_32X		2
+#define TEST_SCALE_64X		1
+
+#define TEST_SCALE_NANO_128X	500000000
+#define TEST_SCALE_NANO_256X	250000000
+#define TEST_SCALE_NANO_512X	125000000
+#define TEST_SCALE_NANO_1024X	62500000
+#define TEST_SCALE_NANO_2048X	31250000
+#define TEST_SCALE_NANO_4096X	15625000
+#define TEST_SCALE_NANO_4096X2	7812500
+#define TEST_SCALE_NANO_4096X4	3906250
+#define TEST_SCALE_NANO_4096X8	1953125
+
+#define TEST_SCALE_NANO_MAX_X TEST_SCALE_NANO_4096X8
+
+/*
+ * Can't have this allocated from stack because the kunit clean-up will
+ * happen only after the test function has already gone
+ */
+static struct iio_gts gts;
+
+static const struct iio_gain_sel_pair gts_test_gains[] = {
+	GAIN_SCALE_GAIN(1, TEST_GSEL_1),
+	GAIN_SCALE_GAIN(4, TEST_GSEL_4),
+	GAIN_SCALE_GAIN(16, TEST_GSEL_16),
+	GAIN_SCALE_GAIN(32, TEST_GSEL_32),
+	GAIN_SCALE_GAIN(64, TEST_GSEL_64),
+	GAIN_SCALE_GAIN(256, TEST_GSEL_256),
+	GAIN_SCALE_GAIN(512, TEST_GSEL_512),
+	GAIN_SCALE_GAIN(1024, TEST_GSEL_1024),
+	GAIN_SCALE_GAIN(2048, TEST_GSEL_2048),
+	GAIN_SCALE_GAIN(4096, TEST_GSEL_4096),
+#define HWGAIN_MAX 4096
+};
+
+static const struct iio_itime_sel_mul gts_test_itimes[] = {
+	GAIN_SCALE_ITIME_US(400 * 1000, TEST_TSEL_400, 8),
+	GAIN_SCALE_ITIME_US(200 * 1000, TEST_TSEL_200, 4),
+	GAIN_SCALE_ITIME_US(100 * 1000, TEST_TSEL_100, 2),
+	GAIN_SCALE_ITIME_US(50 * 1000, TEST_TSEL_50, 1),
+#define TIMEGAIN_MAX 8
+};
+#define TOTAL_GAIN_MAX	(HWGAIN_MAX * TIMEGAIN_MAX)
+#define IIO_GTS_TEST_DEV "iio-gts-test-dev"
+
+static struct device *__test_init_iio_gain_scale(struct kunit *test,
+		struct iio_gts *gts, const struct iio_gain_sel_pair *g_table,
+		int num_g, const struct iio_itime_sel_mul *i_table, int num_i)
+{
+	struct device *dev;
+	int ret;
+
+	dev = kunit_device_register(test, IIO_GTS_TEST_DEV);
+
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, dev);
+	if (IS_ERR_OR_NULL(dev))
+		return NULL;
+
+	ret = devm_iio_init_iio_gts(dev, TEST_SCALE_1X, 0, g_table, num_g,
+				    i_table, num_i, gts);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	if (ret)
+		return NULL;
+
+	return dev;
+}
+
+#define test_init_iio_gain_scale(test, gts)	\
+	__test_init_iio_gain_scale(test, gts, gts_test_gains, \
+				   ARRAY_SIZE(gts_test_gains), gts_test_itimes, \
+				   ARRAY_SIZE(gts_test_itimes))
+
+static void test_init_iio_gts_invalid(struct kunit *test)
+{
+	struct device *dev;
+	int ret;
+	const struct iio_itime_sel_mul itimes_neg[] = {
+		GAIN_SCALE_ITIME_US(-10, TEST_TSEL_400, 8),
+		GAIN_SCALE_ITIME_US(200 * 1000, TEST_TSEL_200, 4),
+	};
+	const struct iio_gain_sel_pair gains_neg[] = {
+		GAIN_SCALE_GAIN(1, TEST_GSEL_1),
+		GAIN_SCALE_GAIN(2, TEST_GSEL_4),
+		GAIN_SCALE_GAIN(-2, TEST_GSEL_16),
+	};
+	/* 55555 * 38656 = 2147534080 => overflows 32bit int */
+	const struct iio_itime_sel_mul itimes_overflow[] = {
+		GAIN_SCALE_ITIME_US(400 * 1000, TEST_TSEL_400, 55555),
+		GAIN_SCALE_ITIME_US(200 * 1000, TEST_TSEL_200, 4),
+	};
+	const struct iio_gain_sel_pair gains_overflow[] = {
+		GAIN_SCALE_GAIN(1, TEST_GSEL_1),
+		GAIN_SCALE_GAIN(2, TEST_GSEL_4),
+		GAIN_SCALE_GAIN(38656, TEST_GSEL_16),
+	};
+
+	dev = kunit_device_register(test, IIO_GTS_TEST_DEV);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, dev);
+	if (!dev)
+		return;
+
+	/* Ok gains, negative time */
+	ret = devm_iio_init_iio_gts(dev, TEST_SCALE_1X, 0, gts_test_gains,
+				    ARRAY_SIZE(gts_test_gains), itimes_neg,
+				    ARRAY_SIZE(itimes_neg), &gts);
+	KUNIT_EXPECT_EQ(test, -EINVAL, ret);
+
+	/* Ok times, negative gain */
+	ret = devm_iio_init_iio_gts(dev, TEST_SCALE_1X, 0, gains_neg,
+				    ARRAY_SIZE(gains_neg), gts_test_itimes,
+				    ARRAY_SIZE(gts_test_itimes), &gts);
+	KUNIT_EXPECT_EQ(test, -EINVAL, ret);
+
+	/* gain * time overflow int */
+	ret = devm_iio_init_iio_gts(dev, TEST_SCALE_1X, 0, gains_overflow,
+				    ARRAY_SIZE(gains_overflow), itimes_overflow,
+				    ARRAY_SIZE(itimes_overflow), &gts);
+	KUNIT_EXPECT_EQ(test, -EOVERFLOW, ret);
+}
+
+static void test_iio_gts_find_gain_for_scale_using_time(struct kunit *test)
+{
+	struct device *dev;
+	int ret, gain_sel;
+
+	dev = test_init_iio_gain_scale(test, &gts);
+	if (!dev)
+		return;
+
+	ret = iio_gts_find_gain_sel_for_scale_using_time(&gts, TEST_TSEL_100,
+						TEST_SCALE_8X, 0, &gain_sel);
+	/*
+	 * Meas time 100 => gain by time 2x
+	 * TEST_SCALE_8X matches total gain 8x
+	 * => required HWGAIN 4x
+	 */
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, TEST_GSEL_4, gain_sel);
+
+	ret = iio_gts_find_gain_sel_for_scale_using_time(&gts, TEST_TSEL_200, 0,
+						TEST_SCALE_NANO_256X, &gain_sel);
+	/*
+	 * Meas time 200 => gain by time 4x
+	 * TEST_SCALE_256X matches total gain 256x
+	 * => required HWGAIN 256/4 => 64x
+	 */
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, TEST_GSEL_64, gain_sel);
+
+	/* Min time, Min gain */
+	ret = iio_gts_find_gain_sel_for_scale_using_time(&gts, TEST_TSEL_X_MIN,
+						TEST_SCALE_MIN_X, 0, &gain_sel);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, TEST_GSEL_1, gain_sel);
+
+	/* Max time, Max gain */
+	ret = iio_gts_find_gain_sel_for_scale_using_time(&gts, TEST_TSEL_X_MAX,
+					0, TEST_SCALE_NANO_MAX_X, &gain_sel);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, TEST_GSEL_4096, gain_sel);
+
+	ret = iio_gts_find_gain_sel_for_scale_using_time(&gts, TEST_TSEL_100, 0,
+						TEST_SCALE_NANO_256X, &gain_sel);
+	/*
+	 * Meas time 100 => gain by time 2x
+	 * TEST_SCALE_256X matches total gain 256x
+	 * => required HWGAIN 256/2 => 128x (not in gain-table - unsupported)
+	 */
+	KUNIT_EXPECT_NE(test, 0, ret);
+
+	ret = iio_gts_find_gain_sel_for_scale_using_time(&gts, TEST_TSEL_200, 0,
+						TEST_SCALE_NANO_MAX_X, &gain_sel);
+	/* We can't reach the max gain with integration time smaller than MAX */
+	KUNIT_EXPECT_NE(test, 0, ret);
+
+	ret = iio_gts_find_gain_sel_for_scale_using_time(&gts, TEST_TSEL_50, 0,
+						TEST_SCALE_NANO_MAX_X, &gain_sel);
+	/* We can't reach the max gain with integration time smaller than MAX */
+	KUNIT_EXPECT_NE(test, 0, ret);
+}
+
+static void test_iio_gts_find_new_gain_sel_by_old_gain_time(struct kunit *test)
+{
+	struct device *dev;
+	int ret, old_gain, new_gain, old_time_sel, new_time_sel;
+
+	dev = test_init_iio_gain_scale(test, &gts);
+	if (!dev)
+		return;
+
+	old_gain = 32;
+	old_time_sel = TEST_TSEL_200;
+	new_time_sel = TEST_TSEL_400;
+
+	ret = iio_gts_find_new_gain_sel_by_old_gain_time(&gts, old_gain,
+					old_time_sel, new_time_sel, &new_gain);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	/*
+	 * Doubling the integration time doubles the total gain - so old
+	 * (hw)gain must be divided by two to compensate. => 32 / 2 => 16
+	 */
+	KUNIT_EXPECT_EQ(test, 16, new_gain);
+
+	old_gain = 4;
+	old_time_sel = TEST_TSEL_50;
+	new_time_sel = TEST_TSEL_200;
+	ret = iio_gts_find_new_gain_sel_by_old_gain_time(&gts, old_gain,
+					old_time_sel, new_time_sel, &new_gain);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	/*
+	 * gain by time 1x => 4x - (hw)gain 4x => 1x
+	 */
+	KUNIT_EXPECT_EQ(test, 1, new_gain);
+
+	old_gain = 512;
+	old_time_sel = TEST_TSEL_400;
+	new_time_sel = TEST_TSEL_50;
+	ret = iio_gts_find_new_gain_sel_by_old_gain_time(&gts, old_gain,
+					old_time_sel, new_time_sel, &new_gain);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	/*
+	 * gain by time 8x => 1x - (hw)gain 512x => 4096x)
+	 */
+	KUNIT_EXPECT_EQ(test, 4096, new_gain);
+
+	/* Unsupported gain 2x */
+	old_gain = 4;
+	old_time_sel = TEST_TSEL_200;
+	new_time_sel = TEST_TSEL_400;
+	ret = iio_gts_find_new_gain_sel_by_old_gain_time(&gts, old_gain,
+					old_time_sel, new_time_sel, &new_gain);
+	KUNIT_EXPECT_NE(test, 0, ret);
+
+	/* Too small gain */
+	old_gain = 4;
+	old_time_sel = TEST_TSEL_50;
+	new_time_sel = TEST_TSEL_400;
+	ret = iio_gts_find_new_gain_sel_by_old_gain_time(&gts, old_gain,
+					old_time_sel, new_time_sel, &new_gain);
+	KUNIT_EXPECT_NE(test, 0, ret);
+
+	/* Too big gain */
+	old_gain = 1024;
+	old_time_sel = TEST_TSEL_400;
+	new_time_sel = TEST_TSEL_50;
+	ret = iio_gts_find_new_gain_sel_by_old_gain_time(&gts, old_gain,
+					old_time_sel, new_time_sel, &new_gain);
+	KUNIT_EXPECT_NE(test, 0, ret);
+
+}
+
+static void test_iio_find_closest_gain_low(struct kunit *test)
+{
+	struct device *dev;
+	bool in_range;
+	int ret;
+
+	const struct iio_gain_sel_pair gts_test_gains_gain_low[] = {
+		GAIN_SCALE_GAIN(4, TEST_GSEL_4),
+		GAIN_SCALE_GAIN(16, TEST_GSEL_16),
+		GAIN_SCALE_GAIN(32, TEST_GSEL_32),
+	};
+
+	dev = test_init_iio_gain_scale(test, &gts);
+	if (!dev)
+		return;
+
+	ret = iio_find_closest_gain_low(&gts, 2, &in_range);
+	KUNIT_EXPECT_EQ(test, 1, ret);
+	KUNIT_EXPECT_EQ(test, true, in_range);
+
+	ret = iio_find_closest_gain_low(&gts, 1, &in_range);
+	KUNIT_EXPECT_EQ(test, 1, ret);
+	KUNIT_EXPECT_EQ(test, true, in_range);
+
+	ret = iio_find_closest_gain_low(&gts, 4095, &in_range);
+	KUNIT_EXPECT_EQ(test, 2048, ret);
+	KUNIT_EXPECT_EQ(test, true, in_range);
+
+	ret = iio_find_closest_gain_low(&gts, 4097, &in_range);
+	KUNIT_EXPECT_EQ(test, 4096, ret);
+	KUNIT_EXPECT_EQ(test, false, in_range);
+
+	kunit_device_unregister(test, dev);
+
+	dev = __test_init_iio_gain_scale(test, &gts, gts_test_gains_gain_low,
+				ARRAY_SIZE(gts_test_gains_gain_low),
+				gts_test_itimes, ARRAY_SIZE(gts_test_itimes));
+	if (!dev)
+		return;
+
+	ret = iio_find_closest_gain_low(&gts, 3, &in_range);
+	KUNIT_EXPECT_EQ(test, -EINVAL, ret);
+	KUNIT_EXPECT_EQ(test, false, in_range);
+
+}
+
+static void test_iio_gts_total_gain_to_scale(struct kunit *test)
+{
+	struct device *dev;
+	int ret, scale_int, scale_nano;
+
+	dev = test_init_iio_gain_scale(test, &gts);
+	if (!dev)
+		return;
+
+	ret = iio_gts_total_gain_to_scale(&gts, 1, &scale_int, &scale_nano);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, TEST_SCALE_1X, scale_int);
+	KUNIT_EXPECT_EQ(test, 0, scale_nano);
+
+	ret = iio_gts_total_gain_to_scale(&gts, 1, &scale_int, &scale_nano);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, TEST_SCALE_1X, scale_int);
+	KUNIT_EXPECT_EQ(test, 0, scale_nano);
+
+	ret = iio_gts_total_gain_to_scale(&gts, 4096 * 8, &scale_int,
+					  &scale_nano);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, 0, scale_int);
+	KUNIT_EXPECT_EQ(test, TEST_SCALE_NANO_4096X8, scale_nano);
+
+}
+
+static void test_iio_gts_chk_times(struct kunit *test, const int *vals)
+{
+	static const int expected[] = {50000, 100000, 200000, 400000};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(expected); i++)
+		KUNIT_EXPECT_EQ(test, expected[i], vals[i]);
+}
+
+static void test_iio_gts_chk_scales_all(struct kunit *test, struct iio_gts *gts,
+					const int *vals, int len)
+{
+	static const int gains[] = {1, 2, 4, 8, 16, 32, 64, 128, 256, 512,
+				    1024, 2048, 4096, 4096 * 2, 4096 * 4,
+				    4096 * 8};
+
+	int expected[ARRAY_SIZE(gains) * 2];
+	int i, ret;
+	int exp_len = ARRAY_SIZE(gains) * 2;
+
+	KUNIT_EXPECT_EQ(test, exp_len, len);
+	if (len != exp_len)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(gains); i++) {
+		ret = iio_gts_total_gain_to_scale(gts, gains[i],
+						  &expected[2 * i],
+						  &expected[2 * i + 1]);
+		KUNIT_EXPECT_EQ(test, 0, ret);
+		if (ret)
+			return;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(expected); i++)
+		KUNIT_EXPECT_EQ(test, expected[i], vals[i]);
+}
+
+static void test_iio_gts_chk_scales_t200(struct kunit *test, struct iio_gts *gts,
+					 const int *vals, int len)
+{
+	/* The gain caused by time 200 is 4x */
+	static const int gains[] = {
+		1 * 4,
+		4 * 4,
+		16 * 4,
+		32 * 4,
+		64 * 4,
+		256 * 4,
+		512 * 4,
+		1024 * 4,
+		2048 * 4,
+		4096 * 4
+	};
+	int expected[ARRAY_SIZE(gains) * 2];
+	int i, ret;
+
+	KUNIT_EXPECT_EQ(test, 2 * ARRAY_SIZE(gains), len);
+	if (len < 2 * ARRAY_SIZE(gains))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(gains); i++) {
+		ret = iio_gts_total_gain_to_scale(gts, gains[i],
+						  &expected[2 * i],
+						  &expected[2 * i + 1]);
+		KUNIT_EXPECT_EQ(test, 0, ret);
+		if (ret)
+			return;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(expected); i++)
+		KUNIT_EXPECT_EQ(test, expected[i], vals[i]);
+}
+
+static void test_iio_gts_avail_test(struct kunit *test)
+{
+	struct device *dev;
+	int ret;
+	int type, len;
+	const int *vals;
+
+	dev = test_init_iio_gain_scale(test, &gts);
+	if (!dev)
+		return;
+
+	/* test table building for times and iio_gts_avail_times() */
+	ret = iio_gts_avail_times(&gts, &vals, &type, &len);
+	KUNIT_EXPECT_EQ(test, IIO_AVAIL_LIST, ret);
+	if (ret)
+		return;
+
+	KUNIT_EXPECT_EQ(test, IIO_VAL_INT, type);
+	KUNIT_EXPECT_EQ(test, 4, len);
+	if (len < 4)
+		return;
+
+	test_iio_gts_chk_times(test, vals);
+
+	/* Test table building for all scales and iio_gts_all_avail_scales() */
+	ret = iio_gts_all_avail_scales(&gts, &vals, &type, &len);
+	KUNIT_EXPECT_EQ(test, IIO_AVAIL_LIST, ret);
+	if (ret)
+		return;
+
+	KUNIT_EXPECT_EQ(test, IIO_VAL_INT_PLUS_NANO, type);
+
+	test_iio_gts_chk_scales_all(test, &gts, vals, len);
+
+	/*
+	 * Test table building for scales/time and
+	 * iio_gts_avail_scales_for_time()
+	 */
+	ret = iio_gts_avail_scales_for_time(&gts, 200000, &vals, &type, &len);
+	KUNIT_EXPECT_EQ(test, IIO_AVAIL_LIST, ret);
+	if (ret)
+		return;
+
+	KUNIT_EXPECT_EQ(test, IIO_VAL_INT_PLUS_NANO, type);
+	test_iio_gts_chk_scales_t200(test, &gts, vals, len);
+}
+
+static struct kunit_case iio_gts_test_cases[] = {
+	KUNIT_CASE(test_init_iio_gts_invalid),
+	KUNIT_CASE(test_iio_gts_find_gain_for_scale_using_time),
+	KUNIT_CASE(test_iio_gts_find_new_gain_sel_by_old_gain_time),
+	KUNIT_CASE(test_iio_find_closest_gain_low),
+	KUNIT_CASE(test_iio_gts_total_gain_to_scale),
+	KUNIT_CASE(test_iio_gts_avail_test),
+	{}
+};
+
+static struct kunit_suite iio_gts_test_suite = {
+	.name = "iio-gain-time-scale",
+	.test_cases = iio_gts_test_cases,
+};
+
+kunit_test_suite(iio_gts_test_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Matti Vaittinen <mazziesaccount@gmail.com>");
+MODULE_DESCRIPTION("Test IIO light sensor gain-time-scale helpers");
+MODULE_IMPORT_NS(IIO_GTS_HELPER);
+
-- 
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 =] 

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 5/7] MAINTAINERS: Add IIO gain-time-scale helpers
  2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
                   ` (3 preceding siblings ...)
  2023-03-27 11:34 ` [PATCH v6 4/7] iio: test: test gain-time-scale helpers Matti Vaittinen
@ 2023-03-27 11:34 ` Matti Vaittinen
  2023-03-27 11:34 ` [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor Matti Vaittinen
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:34 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: linux-kernel, Jonathan Cameron, linux-iio

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

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.

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

---
RFCv1 =>
- No changes
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ec57c42ed544..6ec9326f4ce9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -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 <mazziesaccount@gmail.com>
+L:	linux-iio@vger.kernel.org
+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 <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
-- 
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 =] 

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor
  2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
                   ` (4 preceding siblings ...)
  2023-03-27 11:34 ` [PATCH v6 5/7] MAINTAINERS: Add IIO " Matti Vaittinen
@ 2023-03-27 11:34 ` Matti Vaittinen
  2023-03-28  5:59   ` kernel test robot
  2023-03-30 16:59   ` Matti Vaittinen
  2023-03-27 11:35 ` [PATCH v6 7/7] MAINTAINERS: Add ROHM BU27034 Matti Vaittinen
  2023-03-27 11:39 ` [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
  7 siblings, 2 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:34 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Jonathan Cameron, Lars-Peter Clausen, Matti Vaittinen,
	Andy Shevchenko, Zhigang Shi, Shreeya Patel, Dmitry Osipenko,
	Paul Gazzillo, linux-kernel, linux-iio

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

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.

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

---
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 | 1496 ++++++++++++++++++++++++++++++
 3 files changed, 1511 insertions(+)
 create mode 100644 drivers/iio/light/rohm-bu27034.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 0d4447df7200..6fa31fcd71a1 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -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
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index d74d2b5ff14c..985f6feaccd4 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -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
diff --git a/drivers/iio/light/rohm-bu27034.c b/drivers/iio/light/rohm-bu27034.c
new file mode 100644
index 000000000000..4842a9b66c97
--- /dev/null
+++ b/drivers/iio/light/rohm-bu27034.c
@@ -0,0 +1,1496 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * BU27034 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>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/units.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/iio-gts-helper.h>
+#include <linux/iio/kfifo_buf.h>
+
+
+#define BU27034_REG_SYSTEM_CONTROL	0x40
+#define BU27034_MASK_SW_RESET		BIT(7)
+#define BU27034_MASK_PART_ID		GENMASK(5, 0)
+#define BU27034_ID			0x19
+#define BU27034_REG_MODE_CONTROL1	0x41
+#define BU27034_MASK_MEAS_MODE		GENMASK(2, 0)
+
+#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_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_MANUFACTURER_ID	0x92
+#define BU27034_REG_MAX BU27034_REG_MANUFACTURER_ID
+
+/*
+ * The BU27034 does not have interrupt to trigger the data read when a
+ * measurement has finished. Hence we poll the VALID bit in a thread. We will
+ * try to wake the thread BU27034_MEAS_WAIT_PREMATURE_MS milliseconds before
+ * the expected sampling time to prevent the drifting.
+ *
+ * If we constantly wake up a bit too late we would eventually skip a sample.
+ * And because the sleep can't wake up _exactly_ at given time this would be
+ * inevitable even if the sensor clock would be perfectly phase-locked to CPU
+ * clock - which we can't say is the case.
+ *
+ * This is still fragile. No matter how big advance do we have, we will still
+ * risk of losing a sample because things can in a rainy-day scenario be
+ * delayed a lot. Yet, more we reserve the time for polling, more we also lose
+ * the performance by spending cycles polling the register. So, selecting this
+ * value is a balancing dance between severity of wasting CPU time and severity
+ * of losing samples.
+ *
+ * In most cases losing the samples is not _that_ crucial because light levels
+ * tend to change slowly.
+ *
+ * Other option that was pointed to me would be always sleeping 1/2 of the
+ * measurement time, checking the VALID bit and just sleeping again if the bit
+ * was not set. That should be pretty tolerant against missing samples due to
+ * the scheduling delays while also not wasting much of cycles for polling.
+ * Downside is that the time-stamps would be very inaccurate as the wake-up
+ * would not really be tied to the sensor toggling the valid bit. This would also
+ * result 'jumps' in the time-stamps when the delay drifted so that wake-up was
+ * performed during the consecutive wake-ups (Or, when sensor and CPU clocks
+ * were very different and scheduling the wake-ups was very close to given
+ * timeout - and when the time-outs were very close to the actual sensor
+ * sampling, Eg. once in a blue moon, two consecutive time-outs would occur
+ * without having a sample ready).
+ */
+#define BU27034_MEAS_WAIT_PREMATURE_MS	5
+#define BU27034_DATA_WAIT_TIME_US	1000
+#define BU27034_TOTAL_DATA_WAIT_TIME_US (BU27034_MEAS_WAIT_PREMATURE_MS * 1000)
+
+#define BU27034_RETRY_LIMIT 18
+
+enum {
+	BU27034_CHAN_ALS,
+	BU27034_CHAN_DATA0,
+	BU27034_CHAN_DATA1,
+	BU27034_CHAN_DATA2,
+	BU27034_NUM_CHANS
+};
+
+static const unsigned long bu27034_scan_masks[] = {
+	BIT(BU27034_CHAN_ALS) | BIT(BU27034_CHAN_DATA0) |
+	BIT(BU27034_CHAN_DATA1) | BIT(BU27034_CHAN_DATA2), 0
+};
+
+/*
+ * Available scales with gain 1x - 4096x, 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
+ *
+ * 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).
+ */
+#define BU27034_SCALE_1X	64
+
+/* 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).
+ *
+ * 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.
+ */
+#define BU27034_MEAS_MODE_100MS		0
+#define BU27034_MEAS_MODE_55MS		1
+#define BU27034_MEAS_MODE_200MS		2
+#define BU27034_MEAS_MODE_400MS		4
+
+static const struct iio_itime_sel_mul bu27034_itimes[] = {
+	GAIN_SCALE_ITIME_US(400000, BU27034_MEAS_MODE_400MS, 8),
+	GAIN_SCALE_ITIME_US(200000, BU27034_MEAS_MODE_200MS, 4),
+	GAIN_SCALE_ITIME_US(100000, BU27034_MEAS_MODE_100MS, 2),
+	GAIN_SCALE_ITIME_US(55000, BU27034_MEAS_MODE_55MS, 1),
+};
+
+#define BU27034_CHAN_DATA(_name, _ch2)					\
+{									\
+	.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),	\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),		\
+	.info_mask_shared_by_all_available =				\
+					BIT(IIO_CHAN_INFO_INT_TIME),	\
+	.address = BU27034_REG_##_name##_LO,				\
+	.scan_index = BU27034_CHAN_##_name,				\
+	.scan_type = {							\
+		.sign = 'u',						\
+		.realbits = 16,						\
+		.storagebits = 16,					\
+		.endianness = IIO_LE,					\
+	},								\
+	.indexed = 1,							\
+}
+
+static const struct iio_chan_spec bu27034_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.channel = BU27034_CHAN_ALS,
+		.scan_index = BU27034_CHAN_ALS,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
+	},
+	/*
+	 * 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
+	 */
+	BU27034_CHAN_DATA(DATA0, IIO_MOD_LIGHT_CLEAR),
+	BU27034_CHAN_DATA(DATA1, IIO_MOD_LIGHT_CLEAR),
+	BU27034_CHAN_DATA(DATA2, IIO_MOD_LIGHT_IR),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+struct bu27034_data {
+	struct regmap *regmap;
+	struct device *dev;
+	/*
+	 * Protect gain and time during scale adjustment and data reading.
+	 * Protect measurement enabling/disabling.
+	 */
+	struct mutex mutex;
+	struct iio_gts gts;
+	struct task_struct *task;
+	__le16 raw[3];
+	struct {
+		u32 mlux;
+		__le16 channels[3];
+		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_MODE_CONTROL4,
+		.range_max = BU27034_REG_MODE_CONTROL4,
+	}, {
+		.range_min = BU27034_REG_DATA0_LO,
+		.range_max = BU27034_REG_DATA2_HI,
+	},
+};
+
+static const struct regmap_access_table bu27034_volatile_regs = {
+	.yes_ranges = &bu27034_volatile_ranges[0],
+	.n_yes_ranges = ARRAY_SIZE(bu27034_volatile_ranges),
+};
+
+static const struct regmap_range bu27034_read_only_ranges[] = {
+	{
+		.range_min = BU27034_REG_DATA0_LO,
+		.range_max = BU27034_REG_DATA2_HI,
+	}, {
+		.range_min = BU27034_REG_MANUFACTURER_ID,
+		.range_max = BU27034_REG_MANUFACTURER_ID,
+	}
+};
+
+static const struct regmap_access_table bu27034_ro_regs = {
+	.no_ranges = &bu27034_read_only_ranges[0],
+	.n_no_ranges = ARRAY_SIZE(bu27034_read_only_ranges),
+};
+
+static const struct regmap_config bu27034_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = BU27034_REG_MAX,
+	.cache_type = REGCACHE_RBTREE,
+	.volatile_table = &bu27034_volatile_regs,
+	.wr_table = &bu27034_ro_regs,
+};
+
+struct bu27034_gain_check {
+	int old_gain;
+	int new_gain;
+	int chan;
+};
+
+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;
+
+		/*
+		 * 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;
+	}
+}
+
+static int bu27034_get_gain(struct bu27034_data *data, int chan, int *gain)
+{
+	int ret, sel;
+
+	ret = bu27034_get_gain_sel(data, chan);
+	if (ret < 0)
+		return ret;
+
+	sel = ret;
+
+	ret = iio_gts_find_gain_by_sel(&data->gts, sel);
+	if (ret < 0) {
+		dev_err(data->dev, "chan %u: unknown gain value 0x%x\n", chan,
+			sel);
+
+		return ret;
+	}
+
+	*gain = ret;
+
+	return 0;
+}
+
+static int bu27034_get_int_time(struct bu27034_data *data)
+{
+	int ret, sel;
+
+	ret = regmap_read(data->regmap, BU27034_REG_MODE_CONTROL1, &sel);
+	if (ret)
+		return ret;
+
+	return iio_gts_find_int_time_by_sel(&data->gts,
+					    sel & BU27034_MASK_MEAS_MODE);
+}
+
+static int _bu27034_get_scale(struct bu27034_data *data, int channel, int *val,
+			      int *val2)
+{
+	int gain, ret;
+
+	ret = bu27034_get_gain(data, channel, &gain);
+	if (ret)
+		return ret;
+
+	ret = bu27034_get_int_time(data);
+	if (ret < 0)
+		return ret;
+
+	return iio_gts_get_scale(&data->gts, gain, ret, val, val2);
+}
+
+static int bu27034_get_scale(struct bu27034_data *data, int channel, int *val,
+			      int *val2)
+{
+	int ret;
+
+	if (channel == BU27034_CHAN_ALS) {
+		*val = 0;
+		*val2 = 1000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+
+	mutex_lock(&data->mutex);
+	ret = _bu27034_get_scale(data, channel, val, val2);
+	mutex_unlock(&data->mutex);
+	if (ret)
+		return ret;
+
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+/* Caller should hold the lock to protect lux reading */
+static int bu27034_write_gain_sel(struct bu27034_data *data, int chan, int sel)
+{
+	static const int reg[] = {
+		[BU27034_CHAN_DATA0] = BU27034_REG_MODE_CONTROL2,
+		[BU27034_CHAN_DATA1] = BU27034_REG_MODE_CONTROL3,
+	};
+	int mask, val;
+
+	if (chan != BU27034_CHAN_DATA0 && chan != BU27034_CHAN_DATA1)
+		return -EINVAL;
+
+	mask = BU27034_MASK_D01_GAIN;
+	val = FIELD_PREP(mask, sel);
+
+	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);
+}
+
+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;
+
+	return bu27034_write_gain_sel(data, chan, ret);
+}
+
+/* Caller should hold the lock to protect data->int_time */
+static int bu27034_set_int_time(struct bu27034_data *data, int time)
+{
+	int ret;
+
+	ret = iio_gts_find_sel_by_int_time(&data->gts, time);
+	if (ret < 0)
+		return ret;
+
+	return regmap_update_bits(data->regmap, BU27034_REG_MODE_CONTROL1,
+				 BU27034_MASK_MEAS_MODE, ret);
+}
+
+/*
+ * We try to change the time in such way that the scale is maintained for
+ * given channels by adjusting gain so that it compensates the time change.
+ */
+static int bu27034_try_set_int_time(struct bu27034_data *data, int time_us)
+{
+	struct bu27034_gain_check gains[] = {
+		{ .chan = BU27034_CHAN_DATA0 },
+		{ .chan = BU27034_CHAN_DATA1 },
+	};
+	int numg = ARRAY_SIZE(gains);
+	int ret, int_time_old, i;
+
+	mutex_lock(&data->mutex);
+	ret = bu27034_get_int_time(data);
+	if (ret < 0)
+		goto unlock_out;
+
+	int_time_old = ret;
+
+	if (!iio_gts_valid_time(&data->gts, time_us)) {
+		dev_err(data->dev, "Unsupported integration time %u\n",
+			time_us);
+		ret = -EINVAL;
+
+		goto unlock_out;
+	}
+
+	if (time_us == int_time_old) {
+		ret = 0;
+		goto unlock_out;
+	}
+
+	for (i = 0; i < numg; i++) {
+		ret = bu27034_get_gain(data, gains[i].chan, &gains[i].old_gain);
+		if (ret)
+			goto unlock_out;
+
+		ret = iio_gts_find_new_gain_by_old_gain_time(&data->gts,
+							     gains[i].old_gain,
+							     int_time_old, time_us,
+							     &gains[i].new_gain);
+		if (ret) {
+			int scale1, scale2;
+			bool ok;
+
+			_bu27034_get_scale(data, gains[i].chan, &scale1, &scale2);
+			dev_dbg(data->dev,
+				"chan %u, can't support time %u with scale %u %u\n",
+				gains[i].chan, time_us, scale1, scale2);
+
+			/*
+			 * If caller requests for integration time change and we
+			 * can't support the scale - then the caller should be
+			 * prepared to 'pick up the pieces and deal with the
+			 * fact that the scale changed'.
+			 */
+			ret = iio_find_closest_gain_low(&data->gts,
+							gains[i].new_gain, &ok);
+
+			if (!ok) {
+				dev_dbg(data->dev,
+					"optimal gain out of range for chan %u\n",
+					gains[i].chan);
+			}
+			if (ret < 0) {
+				dev_dbg(data->dev,
+					 "Total gain increase. Risk of saturation");
+				ret = iio_gts_get_min_gain(&data->gts);
+				if (ret < 0)
+					goto unlock_out;
+			}
+			dev_dbg(data->dev, "chan %u scale changed\n",
+				 gains[i].chan);
+			gains[i].new_gain = ret;
+			dev_dbg(data->dev, "chan %u new gain %u\n",
+				gains[i].chan, gains[i].new_gain);
+		}
+	}
+
+	for (i = 0; i < numg; i++) {
+		ret = bu27034_set_gain(data, gains[i].chan, gains[i].new_gain);
+		if (ret)
+			goto unlock_out;
+	}
+
+	ret = bu27034_set_int_time(data, time_us);
+
+unlock_out:
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int bu27034_set_scale(struct bu27034_data *data, int chan,
+			    int val, int val2)
+{
+	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 == 1000)
+			return 0;
+
+		return -EINVAL;
+	}
+
+	mutex_lock(&data->mutex);
+	ret = regmap_read(data->regmap, BU27034_REG_MODE_CONTROL1, &time_sel);
+	if (ret)
+		goto unlock_out;
+
+	ret = iio_gts_find_gain_sel_for_scale_using_time(&data->gts, time_sel,
+						val, val2 * 1000, &gain_sel);
+	if (ret) {
+		/*
+		 * Could not support scale with given time. Need to change time.
+		 * We still want to maintain the scale for all channels
+		 */
+		struct bu27034_gain_check gain;
+		int new_time_sel;
+
+		/*
+		 * 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).
+		 */
+		if (chan == BU27034_CHAN_DATA0)
+			gain.chan = BU27034_CHAN_DATA1;
+		else if (chan == BU27034_CHAN_DATA1)
+			gain.chan = BU27034_CHAN_DATA0;
+
+		ret = bu27034_get_gain(data, gain.chan, &gain.old_gain);
+		if (ret)
+			goto unlock_out;
+
+		/*
+		 * 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
+		 */
+		for (i = 0; i < data->gts.num_itime; i++) {
+			new_time_sel = data->gts.itime_table[i].sel;
+
+			if (new_time_sel == time_sel)
+				continue;
+
+			/* Can we provide requested scale with this time? */
+			ret = iio_gts_find_gain_sel_for_scale_using_time(
+				&data->gts, new_time_sel, val, val2 * 1000,
+				&gain_sel);
+			if (ret)
+				continue;
+
+			/* Can the other channel(s) 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);
+			if (!ret) {
+				/* Yes - we found suitable time */
+				found = true;
+				break;
+			}
+		}
+		if (!found) {
+			dev_dbg(data->dev,
+				"Can't set scale maintaining other channels\n");
+			ret = -EINVAL;
+
+			goto unlock_out;
+		}
+
+		ret = bu27034_set_gain(data, gain.chan, gain.new_gain);
+		if (ret)
+			goto unlock_out;
+
+		ret = regmap_update_bits(data->regmap, BU27034_REG_MODE_CONTROL1,
+				  BU27034_MASK_MEAS_MODE, new_time_sel);
+		if (ret)
+			goto unlock_out;
+	}
+
+	ret = bu27034_write_gain_sel(data, chan, gain_sel);
+unlock_out:
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+/*
+ * 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)
+ *
+ * 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
+ *
+ * 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
+ *
+ * =>	(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
+ *
+ * This can be unified to format:
+ * lx = [
+ *	 A * ch1 * ch1 * gain0 / (ch0 * gain1 * gain1) +
+ *	 B * ch1 / gain1 +
+ *	 C * ch0 / gain0
+ *	] / mt
+ *
+ * For case 1:
+ * A = 3.126528,
+ * B = 115.7400832
+ * C = -68.1982976
+ *
+ * For case 2:
+ * A = 0.3489024
+ * B = 13.721030912
+ * C = 22.66064768
+ *
+ * For case 3:
+ * A = -0.045312
+ * B = -0.706816
+ * C = 37.48096
+ */
+
+struct bu27034_lx_coeff {
+	unsigned int A;
+	unsigned int B;
+	unsigned int C;
+	/* Indicate which of the coefficients above are negative */
+	bool is_neg[3];
+};
+
+static inline u64 gain_mul_div_helper(u64 val, unsigned int gain,
+				      unsigned int div)
+{
+	/*
+	 * Max gain for a channel is 4096. The max u64 (0xffffffffffffffffULL)
+	 * divided by 4096 is 0xFFFFFFFFFFFFF (GENMASK_ULL(51, 0)) (floored).
+	 * Thus, the 0xFFFFFFFFFFFFF is the largest value we can safely multiply
+	 * with the gain, no matter what gain is set.
+	 *
+	 * So, multiplication with max gain may overflow if val is greater than
+	 * 0xFFFFFFFFFFFFF (52 bits set)..
+	 *
+	 * If this is the case we divide first.
+	 */
+	if (val < GENMASK_ULL(51, 0)) {
+		val *= gain;
+		do_div(val, div);
+	} else {
+		do_div(val, div);
+		val *= gain;
+	}
+
+	return val;
+}
+
+static u64 bu27034_fixp_calc_t1_64bit(unsigned int coeff, unsigned int ch0,
+				      unsigned int ch1, unsigned int gain0,
+				      unsigned int gain1)
+{
+	unsigned int helper;
+	u64 helper64;
+
+	helper64 = (u64)coeff * (u64)ch1 * (u64)ch1;
+
+	helper = gain1 * gain1;
+	if (helper > ch0) {
+		do_div(helper64, helper);
+
+		return gain_mul_div_helper(helper64, gain0, ch0);
+	}
+
+	do_div(helper64, ch0);
+
+	return gain_mul_div_helper(helper64, gain0, helper);
+
+}
+
+static u64 bu27034_fixp_calc_t1(unsigned int coeff, unsigned int ch0,
+				unsigned int ch1, unsigned int gain0,
+				unsigned int gain1)
+{
+	unsigned int helper, tmp;
+
+	/*
+	 * Here we could overflow even the 64bit value. Hence we
+	 * multiply with gain0 only after the divisions - even though
+	 * it may result loss of accuracy
+	 */
+	helper = coeff * ch1 * ch1;
+	tmp = helper * gain0;
+
+	helper = ch1 * ch1;
+
+	if (check_mul_overflow(helper, coeff, &helper))
+		return bu27034_fixp_calc_t1_64bit(coeff, ch0, ch1, gain0, gain1);
+
+	if (check_mul_overflow(helper, gain0, &tmp))
+		return bu27034_fixp_calc_t1_64bit(coeff, ch0, ch1, gain0, gain1);
+
+	return tmp / (gain1 * gain1) / ch0;
+
+}
+
+static u64 bu27034_fixp_calc_t23(unsigned int coeff, unsigned int ch,
+				 unsigned int gain)
+{
+	unsigned int helper;
+	u64 helper64;
+
+	if (!check_mul_overflow(coeff, ch, &helper))
+		return helper / gain;
+
+	helper64 = (u64)coeff * (u64)ch;
+	do_div(helper64, gain);
+
+	return helper64;
+}
+
+static int bu27034_fixp_calc_lx(unsigned int ch0, unsigned int ch1,
+				unsigned int gain0, unsigned int gain1,
+				unsigned int meastime, int coeff_idx)
+{
+	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 = 3489024,		/* 0.3489024 */
+			.B = 137210309,		/* 13.721030912 */
+			.C = 226606476,		/* 22.66064768 */
+			/* 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];
+	int i;
+
+	if (coeff_idx >= ARRAY_SIZE(coeff))
+		return -EINVAL;
+
+	terms[0] = bu27034_fixp_calc_t1(c->A, ch0, ch1, gain0, gain1);
+	terms[1] = bu27034_fixp_calc_t23(c->B, ch1, gain1);
+	terms[2] = bu27034_fixp_calc_t23(c->C, ch0, gain0);
+
+	/* First, add positive terms */
+	for (i = 0; i < 3; i++)
+		if (!c->is_neg[i])
+			res += terms[i];
+
+	/* No positive term => zero lux */
+	if (!res)
+		return 0;
+
+	/* Then, subtract negative terms (if any) */
+	for (i = 0; i < 3; i++)
+		if (c->is_neg[i]) {
+			/*
+			 * If the negative term is greater than positive - then
+			 * the darkness has taken over and we are all doomed! Eh,
+			 * I mean, then we can just return 0 lx and go out
+			 */
+			if (terms[i] >= res)
+				return 0;
+
+			res -= terms[i];
+		}
+
+	meastime *= 10;
+	do_div(res, meastime);
+
+	return (int) res;
+}
+
+static bool bu27034_has_valid_sample(struct bu27034_data *data)
+{
+	int ret, val;
+
+	ret = regmap_read(data->regmap, BU27034_REG_MODE_CONTROL4, &val);
+	if (ret) {
+		dev_err(data->dev, "Read failed %d\n", ret);
+
+		return false;
+	}
+
+	return val & BU27034_MASK_VALID;
+}
+
+/*
+ * Reading the register where VALID bit is clears this bit. (So does changing
+ * any gain / integration time configuration registers) The bit gets
+ * set when we have acquired new data. We use this bit to indicate data
+ * validity.
+ */
+static void bu27034_invalidate_read_data(struct bu27034_data *data)
+{
+	bu27034_has_valid_sample(data);
+}
+
+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;
+
+	ret = regmap_read_poll_timeout(data->regmap, BU27034_REG_MODE_CONTROL4,
+				       valid, (valid & BU27034_MASK_VALID),
+				       BU27034_DATA_WAIT_TIME_US, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(data->regmap, reg[chan], &val, sizeof(val));
+	if (ret)
+		return ret;
+
+	*res = le16_to_cpu(val);
+
+	return 0;
+}
+
+static int bu27034_get_result_unlocked(struct bu27034_data *data, __le16 *res,
+				       int size)
+{
+	int ret = 0, retry_cnt = 0;
+
+retry:
+	/* Get new value from sensor if data is ready */
+	if (bu27034_has_valid_sample(data)) {
+		ret = regmap_bulk_read(data->regmap, BU27034_REG_DATA0_LO,
+				       res, size);
+		if (ret)
+			return ret;
+
+		bu27034_invalidate_read_data(data);
+	} else {
+		/* No new data in sensor. Wait and retry */
+		retry_cnt++;
+
+		if (retry_cnt > BU27034_RETRY_LIMIT) {
+			dev_err(data->dev, "No data from sensor\n");
+
+			return -ETIMEDOUT;
+		}
+
+		msleep(25);
+
+		goto retry;
+	}
+
+	return ret;
+}
+
+static int bu27034_meas_set(struct bu27034_data *data, bool en)
+{
+	if (en)
+		return regmap_set_bits(data->regmap, BU27034_REG_MODE_CONTROL4,
+				       BU27034_MASK_MEAS_EN);
+
+	return regmap_clear_bits(data->regmap, BU27034_REG_MODE_CONTROL4,
+				 BU27034_MASK_MEAS_EN);
+}
+
+static int bu27034_get_single_result(struct bu27034_data *data, int chan,
+				     int *val)
+{
+	int ret;
+
+	if (chan < BU27034_CHAN_DATA0 || chan > BU27034_CHAN_DATA2)
+		return -EINVAL;
+
+	ret = bu27034_meas_set(data, true);
+	if (ret)
+		return ret;
+
+	ret = bu27034_get_int_time(data);
+	if (ret < 0)
+		return ret;
+
+	msleep(ret / 1000);
+
+	return bu27034_read_result(data, chan, val);
+}
+
+/*
+ * The formula given by vendor for computing luxes out of data0 and data1
+ * (in open air) is as follows:
+ *
+ * Let's mark:
+ * D0 = data0/ch0_gain/meas_time_ms * 25600
+ * 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)
+ *
+ * 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
+ * those working with such devices.
+ */
+
+static int bu27034_calc_mlux(struct bu27034_data *data, __le16 *res, int *val)
+{
+	unsigned int gain0, gain1, meastime;
+	unsigned int d1_d0_ratio_scaled;
+	u16 ch0, ch1;
+	u64 helper64;
+	int ret;
+
+	/*
+	 * We return 0 lux if calculation fails. This should be reasonably
+	 * easy to spot from the buffers especially if raw-data channels show
+	 * valid values
+	 */
+	*val = 0;
+
+	ch0 = max_t(u16, 1, le16_to_cpu(res[0]));
+	ch1 = max_t(u16, 1, le16_to_cpu(res[1]));
+
+	ret = bu27034_get_gain(data, BU27034_CHAN_DATA0, &gain0);
+	if (ret)
+		return ret;
+
+	ret = bu27034_get_gain(data, BU27034_CHAN_DATA1, &gain1);
+	if (ret)
+		return ret;
+
+	ret = bu27034_get_int_time(data);
+	if (ret < 0)
+		return ret;
+
+	meastime = ret;
+
+	d1_d0_ratio_scaled = (unsigned int)ch1 * (unsigned int)gain0 * 100;
+	helper64 = (u64)ch1 * (u64)gain0 * 100LLU;
+
+	if (helper64 != d1_d0_ratio_scaled) {
+		unsigned int div = (unsigned int)ch0 * gain1;
+
+		do_div(helper64, div);
+		d1_d0_ratio_scaled = helper64;
+	} else {
+		d1_d0_ratio_scaled /= ch0 * gain1;
+	}
+
+	if (d1_d0_ratio_scaled < 87)
+		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);
+
+	if (ret < 0)
+		return ret;
+
+	*val = ret;
+
+	return 0;
+
+}
+
+static int bu27034_get_mlux(struct bu27034_data *data, int chan, int *val)
+{
+	__le16 res[3];
+	int ret;
+
+	ret = bu27034_meas_set(data, true);
+	if (ret)
+		return ret;
+
+	ret = bu27034_get_result_unlocked(data, &res[0], sizeof(res));
+	if (ret)
+		return ret;
+
+	ret = bu27034_calc_mlux(data, res, val);
+	if (ret)
+		return ret;
+
+	ret = bu27034_meas_set(data, false);
+	if (ret)
+		dev_err(data->dev, "failed to disable measurement\n");
+
+	return 0;
+}
+
+static int bu27034_read_raw(struct iio_dev *idev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct bu27034_data *data = iio_priv(idev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		*val = bu27034_get_int_time(data);
+		if (*val < 0)
+			return *val;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		return bu27034_get_scale(data, chan->channel, val, val2);
+
+	case IIO_CHAN_INFO_RAW:
+	{
+		int (*result_get)(struct bu27034_data *data, int chan, int *val);
+
+		if (chan->type == IIO_INTENSITY)
+			result_get = bu27034_get_single_result;
+		else if (chan->type == IIO_LIGHT)
+			result_get = bu27034_get_mlux;
+		else
+			return -EINVAL;
+
+		/* Don't mess with measurement enabling while buffering */
+		ret = iio_device_claim_direct_mode(idev);
+		if (ret)
+			return ret;
+
+		mutex_lock(&data->mutex);
+		/*
+		 * Reading one channel at a time is inefficient but we
+		 * don't care here. Buffered version should be used if
+		 * performance is an issue.
+		 */
+		ret = result_get(data, chan->channel, val);
+
+		mutex_unlock(&data->mutex);
+		iio_device_release_direct_mode(idev);
+
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+	}
+	default:
+		return -EINVAL;
+
+	}
+}
+
+static int bu27034_write_raw(struct iio_dev *idev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct bu27034_data *data = iio_priv(idev);
+	int ret;
+
+	ret = iio_device_claim_direct_mode(idev);
+	if (ret)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		ret = bu27034_set_scale(data, chan->channel, val, val2);
+		break;
+	case IIO_CHAN_INFO_INT_TIME:
+		ret = bu27034_try_set_int_time(data, val);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	iio_device_release_direct_mode(idev);
+
+	return ret;
+}
+
+static int bu27034_read_avail(struct iio_dev *idev,
+			      struct iio_chan_spec const *chan, const int **vals,
+			      int *type, int *length, long mask)
+{
+	struct bu27034_data *data = iio_priv(idev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_INT_TIME:
+		return iio_gts_avail_times(&data->gts, vals, type, length);
+	case IIO_CHAN_INFO_SCALE:
+		return iio_gts_all_avail_scales(&data->gts, vals, type, length);
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info bu27034_info = {
+	.read_raw = &bu27034_read_raw,
+	.write_raw = &bu27034_write_raw,
+	.read_avail = &bu27034_read_avail,
+};
+
+static int bu27034_chip_init(struct bu27034_data *data)
+{
+	int ret, sel;
+
+	/* Reset */
+	ret = regmap_update_bits(data->regmap, BU27034_REG_SYSTEM_CONTROL,
+			   BU27034_MASK_SW_RESET, BU27034_MASK_SW_RESET);
+	if (ret)
+		return dev_err_probe(data->dev, ret, "Sensor reset failed\n");
+
+	msleep(1);
+	/*
+	 * Read integration time here to ensure it is in regmap cache. We do
+	 * this to speed-up the int-time acquisition in the start of the buffer
+	 * handling thread where longer delays could make it more likely we end
+	 * up skipping a sample, and where the longer delays make timestamps
+	 * less accurate.
+	 */
+	ret = regmap_read(data->regmap, BU27034_REG_MODE_CONTROL1, &sel);
+	if (ret)
+		dev_err(data->dev, "reading integration time failed\n");
+
+	return 0;
+}
+
+static int bu27034_wait_for_data(struct bu27034_data *data)
+{
+	int ret, val;
+
+	ret = regmap_read_poll_timeout(data->regmap, BU27034_REG_MODE_CONTROL4,
+				       val, val & BU27034_MASK_VALID,
+				       BU27034_DATA_WAIT_TIME_US,
+				       BU27034_TOTAL_DATA_WAIT_TIME_US);
+	if (ret) {
+		dev_err(data->dev, "data polling %s\n",
+			!(val & BU27034_MASK_VALID) ? "timeout" : "fail");
+
+		return ret;
+	}
+
+	ret = regmap_bulk_read(data->regmap, BU27034_REG_DATA0_LO,
+			       &data->scan.channels[0],
+			       sizeof(data->scan.channels));
+	if (ret)
+		return ret;
+
+	bu27034_invalidate_read_data(data);
+
+	return 0;
+}
+
+static int bu27034_buffer_thread(void *arg)
+{
+	struct iio_dev *idev = arg;
+	struct bu27034_data *data;
+	int wait_ms;
+
+	data = iio_priv(idev);
+
+	wait_ms = bu27034_get_int_time(data);
+	wait_ms /= 1000;
+
+	wait_ms -= BU27034_MEAS_WAIT_PREMATURE_MS;
+
+	while (!kthread_should_stop()) {
+		int ret;
+		int64_t tstamp;
+
+		msleep(wait_ms);
+		ret = bu27034_wait_for_data(data);
+		if (ret)
+			continue;
+
+		tstamp = iio_get_time_ns(idev);
+
+		if (test_bit(BU27034_CHAN_ALS, idev->active_scan_mask)) {
+			int mlux;
+
+			ret = bu27034_calc_mlux(data, &data->scan.channels[0],
+					       &mlux);
+			if (ret)
+				dev_err(data->dev, "failed to calculate lux\n");
+
+			/*
+			 * The maximum Milli lux value we get with gain 1x time
+			 * 55mS data ch0 = 0xffff ch1 = 0xffff fits in 26 bits
+			 * so there should be no problem returning int from
+			 * computations and casting it to u32
+			 */
+			data->scan.mlux = (u32)mlux;
+		}
+		iio_push_to_buffers_with_timestamp(idev, &data->scan, tstamp);
+	}
+
+	return 0;
+}
+
+static int bu27034_buffer_enable(struct iio_dev *idev)
+{
+	struct bu27034_data *data = iio_priv(idev);
+	struct task_struct *task;
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = bu27034_meas_set(data, true);
+	if (ret)
+		goto unlock_out;
+
+	task = kthread_run(bu27034_buffer_thread, idev,
+				 "bu27034-buffering-%u",
+				 iio_device_id(idev));
+	if (IS_ERR(task)) {
+		ret = PTR_ERR(task);
+		goto unlock_out;
+	}
+
+	data->task = task;
+
+unlock_out:
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int bu27034_buffer_disable(struct iio_dev *idev)
+{
+	struct bu27034_data *data = iio_priv(idev);
+	int ret;
+
+	mutex_lock(&data->mutex);
+	if (data->task) {
+		kthread_stop(data->task);
+		data->task = NULL;
+	}
+
+	ret = bu27034_meas_set(data, false);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static const struct iio_buffer_setup_ops bu27034_buffer_ops = {
+	.postenable = &bu27034_buffer_enable,
+	.predisable = &bu27034_buffer_disable,
+};
+
+static int bu27034_probe(struct i2c_client *i2c)
+{
+	struct device *dev = &i2c->dev;
+	struct bu27034_data *data;
+	struct regmap *regmap;
+	struct iio_dev *idev;
+	unsigned int part_id, reg;
+	int ret;
+
+	regmap = devm_regmap_init_i2c(i2c, &bu27034_regmap);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap),
+				     "Failed to initialize Regmap\n");
+
+	idev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!idev)
+		return -ENOMEM;
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get regulator\n");
+
+	data = iio_priv(idev);
+
+	ret = regmap_read(regmap, BU27034_REG_SYSTEM_CONTROL, &reg);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to access sensor\n");
+
+	part_id = FIELD_GET(BU27034_MASK_PART_ID, reg);
+
+	if (part_id != BU27034_ID)
+		dev_warn(dev, "unknown device 0x%x\n", part_id);
+
+	ret = devm_iio_init_iio_gts(dev, BU27034_SCALE_1X, 0, bu27034_gains,
+				    ARRAY_SIZE(bu27034_gains), bu27034_itimes,
+				    ARRAY_SIZE(bu27034_itimes), &data->gts);
+	if (ret)
+		return ret;
+
+	mutex_init(&data->mutex);
+	data->regmap = regmap;
+	data->dev = dev;
+
+	idev->channels = bu27034_channels;
+	idev->num_channels = ARRAY_SIZE(bu27034_channels);
+	idev->name = "bu27034";
+	idev->info = &bu27034_info;
+
+	idev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+	idev->available_scan_masks = bu27034_scan_masks;
+
+	ret = bu27034_chip_init(data);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_kfifo_buffer_setup(dev, idev, &bu27034_buffer_ops);
+	if (ret)
+		return dev_err_probe(dev, ret, "buffer setup failed\n");
+
+	ret = devm_iio_device_register(dev, idev);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "Unable to register iio device\n");
+
+	return ret;
+}
+
+static const struct of_device_id bu27034_of_match[] = {
+	{ .compatible = "rohm,bu27034" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bu27034_of_match);
+
+static struct i2c_driver bu27034_i2c_driver = {
+	.driver = {
+		.name = "bu27034-als",
+		.of_match_table = bu27034_of_match,
+	},
+	.probe_new = bu27034_probe,
+};
+module_i2c_driver(bu27034_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("ROHM BU27034 ambient light sensor driver");
+MODULE_IMPORT_NS(IIO_GTS_HELPER);
-- 
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 =] 

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 7/7] MAINTAINERS: Add ROHM BU27034
  2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
                   ` (5 preceding siblings ...)
  2023-03-27 11:34 ` [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor Matti Vaittinen
@ 2023-03-27 11:35 ` Matti Vaittinen
  2023-03-27 11:39 ` [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
  7 siblings, 0 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:35 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: linux-kernel, Jonathan Cameron, linux-iio

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

Add myself as a maintainer for ROHM BU27034 ALS driver.

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

---
Changes
v2 =>
- No changes

sRFCv1 => v2:
- Add iio-list
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6ec9326f4ce9..3f13466e50fd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -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 <mazziesaccount@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+F:	drivers/iio/light/rohm-bu27034.c
+
 ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS
 M:	Marek Vasut <marek.vasut+renesas@gmail.com>
 L:	linux-kernel@vger.kernel.org
-- 
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 =] 

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 0/7] Support ROHM BU27034 ALS sensor
  2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
                   ` (6 preceding siblings ...)
  2023-03-27 11:35 ` [PATCH v6 7/7] MAINTAINERS: Add ROHM BU27034 Matti Vaittinen
@ 2023-03-27 11:39 ` Matti Vaittinen
  7 siblings, 0 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 11:39 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Masahiro Yamada, Randy Dunlap, Shreeya Patel,
	Krzysztof Kozlowski, Jonathan Cameron, devicetree, Zhigang Shi,
	Lars-Peter Clausen, Paul Gazzillo, Rob Herring, Dmitry Osipenko,
	linux-iio, linux-kernel, kunit-dev, Stephen Boyd, Liam Beguin,
	Greg Kroah-Hartman, Andy Shevchenko, David Gow, Brendan Higgins,
	linux-kselftest

On 3/27/23 14:27, Matti Vaittinen wrote:
> Support ROHM BU27034 ALS sensor
> 
> The patch 1/7 introduces the helpers for creating/dropping a test device
> for devm-tests. It can be applied alone.

Sorry folks. The wrapper is 3/7 not 1/7

> The patch 4/7 (IIO GTS tests) also depends on the patch 1/7 (and also
> other patches in the series).
> 
> Rest of the series should be Ok to be applied with/without the patches
> 1/7 and 4/7 - although the 4/7 (which depends on 1/7) would be "nice to
> have" together with the rest of the series for the testability reasons.
> 

snip.

> 
> Matti Vaittinen (7):
>    dt-bindings: iio: light: Support ROHM BU27034
>    iio: light: Add gain-time-scale helpers
>    kunit: Add kunit wrappers for (root) device creation

Here.

>    iio: test: test gain-time-scale helpers
>    MAINTAINERS: Add IIO gain-time-scale helpers
>    iio: light: ROHM BU27034 Ambient Light Sensor
>    MAINTAINERS: Add ROHM BU27034
> 
Yours,
	-- Matti

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

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation
  2023-03-27 11:34 ` [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation Matti Vaittinen
@ 2023-03-27 12:01   ` Greg Kroah-Hartman
  2023-03-27 12:20     ` Matti Vaittinen
  0 siblings, 1 reply; 20+ messages in thread
From: Greg Kroah-Hartman @ 2023-03-27 12:01 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Brendan Higgins, David Gow, linux-kernel,
	linux-kselftest, kunit-dev, Stephen Boyd, Jonathan Cameron,
	linux-iio

On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
> A few tests need to have a valid struct device. One such example is
> tests which want to be testing devm-managed interfaces.
> 
> Add kunit wrapper for root_device_[un]register(), which create a root
> device and also add a kunit managed clean-up routine for the device
> destruction upon test exit.

I really do not like this as a "root device" is a horrible hack and
should only be used if you have to hang other devices off of it and you
don't have a real device to tie those devices to.

Here you are abusing it and attempting to treat it as a real device,
which it is not at all, because:

> Special note: In some cases the device reference-count does not reach
> zero and devm-unwinding is not done if device is not sitting on a bus.
> The root_device_[un]register() are dealing with such devices and thus
> this interface may not be usable by all in its current form. More
> information can be found from:
> https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/

See, not a real device, doesn't follow normal "struct device" rules and
lifetimes, don't try to use it for a test as it will only cause problems
and you will be forced to work around that in a test.

Do the right thing here, create a fake bus and add devices to it.

Heck, I'll even write that code if you want it, what's the requirement,
something like:
	struct device *kunit_device_create(struct kunit *test, const char *name);
	void kunit_device_destroy(struct device *dev);
?

Why do you want a "match" function?  You don't provide documentation
here for it so I have no idea.

Anything else needed?

> The use of root-devices in the kunit helpers is intended to be an
> intermediate solution to allow tests which do not require device to sit
> on a bus avoid directly abusing the root_device_[un]register() while
> proper kunit device solution is being worked on. Related discussion can be
> found from:
> https://lore.kernel.org/lkml/CABVgOSmx3A4Vwos2_8xO-XQrQAw5gvY0nc5zLpLmcJ7FtA-dTQ@mail.gmail.com/

Again, no, please let's not get this wrong now and say "we will fix this
later" as that's not how kernel development should work...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation
  2023-03-27 12:01   ` Greg Kroah-Hartman
@ 2023-03-27 12:20     ` Matti Vaittinen
  2023-03-27 12:38       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-27 12:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Matti Vaittinen, Brendan Higgins, David Gow, linux-kernel,
	linux-kselftest, kunit-dev, Stephen Boyd, Jonathan Cameron,
	linux-iio

On 3/27/23 15:01, Greg Kroah-Hartman wrote:
> On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
>> A few tests need to have a valid struct device. One such example is
>> tests which want to be testing devm-managed interfaces.
>>
>> Add kunit wrapper for root_device_[un]register(), which create a root
>> device and also add a kunit managed clean-up routine for the device
>> destruction upon test exit.
> 
> I really do not like this as a "root device" is a horrible hack and
> should only be used if you have to hang other devices off of it and you
> don't have a real device to tie those devices to.
> 
> Here you are abusing it and attempting to treat it as a real device,
> which it is not at all, because:
> 
>> Special note: In some cases the device reference-count does not reach
>> zero and devm-unwinding is not done if device is not sitting on a bus.
>> The root_device_[un]register() are dealing with such devices and thus
>> this interface may not be usable by all in its current form. More
>> information can be found from:
>> https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/
> 
> See, not a real device, doesn't follow normal "struct device" rules and
> lifetimes, don't try to use it for a test as it will only cause problems
> and you will be forced to work around that in a test.

Ok. I understood using the root-device has been a work-around in some 
other tests. Thus continuing use it for tests where we don't need the 
bus until we have a proper alternative was suggested by David.

> Do the right thing here, create a fake bus and add devices to it.
> 
> Heck, I'll even write that code if you want it, what's the requirement,
> something like:
> 	struct device *kunit_device_create(struct kunit *test, const char *name);
> 	void kunit_device_destroy(struct device *dev);

Thanks for the offer Greg. This, however, is being already worked on by 
David. I don't want to step on his toes by writing the same thing, nor 
do I think I should be pushing him to rush on his work.

> Why do you want a "match" function?  You don't provide documentation
> here for it so I have no idea.
> 
> Anything else needed?
> 
>> The use of root-devices in the kunit helpers is intended to be an
>> intermediate solution to allow tests which do not require device to sit
>> on a bus avoid directly abusing the root_device_[un]register() while
>> proper kunit device solution is being worked on. Related discussion can be
>> found from:
>> https://lore.kernel.org/lkml/CABVgOSmx3A4Vwos2_8xO-XQrQAw5gvY0nc5zLpLmcJ7FtA-dTQ@mail.gmail.com/
> 
> Again, no, please let's not get this wrong now and say "we will fix this
> later" as that's not how kernel development should work...

Ok. In that case I need to drop the tests from the series until we get 
the new APIs in place. It really sucks but I guess I understand the 
rationale for not wanting to "intermediate" solutions merged. Yes, I 
hoped it'd be Ok as David is already working on it - but I was still 
kind of expecting your response. This is why I made it very clear in the 
cover-letter and this commit message what is suggested here.

Jonathan, should I re-spin the series without patches 3/7 and 5/7 or can 
you please review this and I'll just drop those for the next version?

Thanks for the review Greg, I think this case is now "closed".

Yours,
	-- Matti

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

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation
  2023-03-27 12:20     ` Matti Vaittinen
@ 2023-03-27 12:38       ` Greg Kroah-Hartman
  2023-03-28 12:45         ` David Gow
  0 siblings, 1 reply; 20+ messages in thread
From: Greg Kroah-Hartman @ 2023-03-27 12:38 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Brendan Higgins, David Gow, linux-kernel,
	linux-kselftest, kunit-dev, Stephen Boyd, Jonathan Cameron,
	linux-iio

On Mon, Mar 27, 2023 at 03:20:06PM +0300, Matti Vaittinen wrote:
> On 3/27/23 15:01, Greg Kroah-Hartman wrote:
> > On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
> > > A few tests need to have a valid struct device. One such example is
> > > tests which want to be testing devm-managed interfaces.
> > > 
> > > Add kunit wrapper for root_device_[un]register(), which create a root
> > > device and also add a kunit managed clean-up routine for the device
> > > destruction upon test exit.
> > 
> > I really do not like this as a "root device" is a horrible hack and
> > should only be used if you have to hang other devices off of it and you
> > don't have a real device to tie those devices to.
> > 
> > Here you are abusing it and attempting to treat it as a real device,
> > which it is not at all, because:
> > 
> > > Special note: In some cases the device reference-count does not reach
> > > zero and devm-unwinding is not done if device is not sitting on a bus.
> > > The root_device_[un]register() are dealing with such devices and thus
> > > this interface may not be usable by all in its current form. More
> > > information can be found from:
> > > https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/
> > 
> > See, not a real device, doesn't follow normal "struct device" rules and
> > lifetimes, don't try to use it for a test as it will only cause problems
> > and you will be forced to work around that in a test.
> 
> Ok. I understood using the root-device has been a work-around in some other
> tests. Thus continuing use it for tests where we don't need the bus until we
> have a proper alternative was suggested by David.
> 
> > Do the right thing here, create a fake bus and add devices to it.
> > 
> > Heck, I'll even write that code if you want it, what's the requirement,
> > something like:
> > 	struct device *kunit_device_create(struct kunit *test, const char *name);
> > 	void kunit_device_destroy(struct device *dev);
> 
> Thanks for the offer Greg. This, however, is being already worked on by
> David. I don't want to step on his toes by writing the same thing, nor do I
> think I should be pushing him to rush on his work.

Ok, David, my offer stands, if you want me to write this I will be glad
to do so.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor
  2023-03-27 11:34 ` [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor Matti Vaittinen
@ 2023-03-28  5:59   ` kernel test robot
  2023-03-30 16:59   ` Matti Vaittinen
  1 sibling, 0 replies; 20+ messages in thread
From: kernel test robot @ 2023-03-28  5:59 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: llvm, oe-kbuild-all, Jonathan Cameron, Lars-Peter Clausen,
	Andy Shevchenko, Zhigang Shi, Shreeya Patel, Dmitry Osipenko,
	Paul Gazzillo, linux-kernel, linux-iio

Hi Matti,

I love your patch! Perhaps something to improve:

[auto build test WARNING on eeac8ede17557680855031c6f305ece2378af326]

url:    https://github.com/intel-lab-lkp/linux/commits/Matti-Vaittinen/dt-bindings-iio-light-Support-ROHM-BU27034/20230327-193728
base:   eeac8ede17557680855031c6f305ece2378af326
patch link:    https://lore.kernel.org/r/1b7d3725bf5ae829b12eaf96362204edd29c6966.1679915278.git.mazziesaccount%40gmail.com
patch subject: [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20230328/202303281338.X45nIbaD-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/b824935db7c7734628f11c55cd64dc819be8798f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Matti-Vaittinen/dt-bindings-iio-light-Support-ROHM-BU27034/20230327-193728
        git checkout b824935db7c7734628f11c55cd64dc819be8798f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/iio/light/ drivers/media/i2c/ drivers/media/pci/intel/ drivers/net/usb/ net/netfilter/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303281338.X45nIbaD-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/iio/light/rohm-bu27034.c:402:8: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((mask), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (mask)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
           val = FIELD_PREP(mask, sel);
                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/bitfield.h:114:3: note: expanded from macro 'FIELD_PREP'
                   __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bitfield.h:71:53: note: expanded from macro '__BF_FIELD_CHECK'
                   BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >     \
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
   include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
   #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                       ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/linux/compiler_types.h:397:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:385:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:377:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
   1 warning generated.


vim +402 drivers/iio/light/rohm-bu27034.c

   388	
   389	/* Caller should hold the lock to protect lux reading */
   390	static int bu27034_write_gain_sel(struct bu27034_data *data, int chan, int sel)
   391	{
   392		static const int reg[] = {
   393			[BU27034_CHAN_DATA0] = BU27034_REG_MODE_CONTROL2,
   394			[BU27034_CHAN_DATA1] = BU27034_REG_MODE_CONTROL3,
   395		};
   396		int mask, val;
   397	
   398		if (chan != BU27034_CHAN_DATA0 && chan != BU27034_CHAN_DATA1)
   399			return -EINVAL;
   400	
   401		mask = BU27034_MASK_D01_GAIN;
 > 402		val = FIELD_PREP(mask, sel);
   403	
   404		if (chan == BU27034_CHAN_DATA0) {
   405			/*
   406			 * We keep the same gain for channel 2 as we set for channel 0
   407			 * We can't allow them to be individually controlled because
   408			 * setting one will impact also the other. Also, if we don't
   409			 * always update both gains we may result unsupported bit
   410			 * combinations.
   411			 *
   412			 * This is not nice but this is yet another place where the
   413			 * user space must be prepared to surprizes. Namely, see chan 2
   414			 * gain changed when chan 0 gain is changed.
   415			 *
   416			 * This is not fatal for most users though. I don't expect the
   417			 * channel 2 to be used in any generic cases - the intensity
   418			 * values provided by the sensor for IR area are not openly
   419			 * documented. Also, channel 2 is not used for visible light.
   420			 *
   421			 * So, if there is application which is written to utilize the
   422			 * channel 2 - then it is probably specifically targeted to this
   423			 * sensor and knows how to utilize those values. It is safe to
   424			 * hope such user can also cope with the gain changes.
   425			 */
   426			mask |=  BU27034_MASK_D2_GAIN_LO;
   427	
   428			/*
   429			 * The D2 gain bits are directly the lowest bits of selector.
   430			 * Just do add those bits to the value
   431			 */
   432			val |= sel & BU27034_MASK_D2_GAIN_LO;
   433		}
   434	
   435		return regmap_update_bits(data->regmap, reg[chan], mask, val);
   436	}
   437	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation
  2023-03-27 12:38       ` Greg Kroah-Hartman
@ 2023-03-28 12:45         ` David Gow
  2023-03-28 13:22           ` Matti Vaittinen
  2023-03-30 16:53           ` Maxime Ripard
  0 siblings, 2 replies; 20+ messages in thread
From: David Gow @ 2023-03-28 12:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Matti Vaittinen, Matti Vaittinen, Brendan Higgins, linux-kernel,
	linux-kselftest, kunit-dev, Stephen Boyd, Jonathan Cameron,
	linux-iio, Maxime Ripard

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

Thanks, Gred and Matti.

On Mon, 27 Mar 2023 at 20:38, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Mar 27, 2023 at 03:20:06PM +0300, Matti Vaittinen wrote:
> > On 3/27/23 15:01, Greg Kroah-Hartman wrote:
> > > On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
> > > > A few tests need to have a valid struct device. One such example is
> > > > tests which want to be testing devm-managed interfaces.
> > > >
> > > > Add kunit wrapper for root_device_[un]register(), which create a root
> > > > device and also add a kunit managed clean-up routine for the device
> > > > destruction upon test exit.
> > >
> > > I really do not like this as a "root device" is a horrible hack and
> > > should only be used if you have to hang other devices off of it and you
> > > don't have a real device to tie those devices to.
> > >
> > > Here you are abusing it and attempting to treat it as a real device,
> > > which it is not at all, because:
> > >

There's a tradeoff here in that we want to pull in as little code (and
complexity) as possible into unit tests, both to make them as easy as
possible to write, and to make them as targeted as possible. This
leads to a lot of tests manually filling out structures with the bare
minimum to get the code being tested to run, and creating "root
devices" seems to have been a convenient way of doing that while only
registering _one_ thing in a big global list per test. So having a
"real device" is not something I'd consider a _necessity_ in designing
these sorts of helpers: a convincing enough fake is sometimes better.

That being said, now that I've got a bit of a better understanding of
the device model, I agree that "root devices" are not an ideal
solution, even if they are a convenient one. I'm still not thrilled by
the prospect of having to register extra things like drivers to get
these simple tests to work, but when wrapped behind helpers, it's a
nice solution in practice.

> > > > Special note: In some cases the device reference-count does not reach
> > > > zero and devm-unwinding is not done if device is not sitting on a bus.
> > > > The root_device_[un]register() are dealing with such devices and thus
> > > > this interface may not be usable by all in its current form. More
> > > > information can be found from:
> > > > https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/
> > >
> > > See, not a real device, doesn't follow normal "struct device" rules and
> > > lifetimes, don't try to use it for a test as it will only cause problems
> > > and you will be forced to work around that in a test.

I think there's still some confusion around exactly what these issues
are, and if they're indeed bugs or expected behaviour. I think it
hangs off the question of what uses of a device with no driver
attached are valid. My initial reading through of the various bits of
the devres implementation seemed to imply that using it with such an
unattached device was supported, but I'm less certain now. In any
case, Maxime's tests in the other thread are a good starting point to
clarify this behaviour, and if we use the bus-based KUnit helpers, it
won't matter either way.

> >
> > Ok. I understood using the root-device has been a work-around in some other
> > tests. Thus continuing use it for tests where we don't need the bus until we
> > have a proper alternative was suggested by David.
> >
> > > Do the right thing here, create a fake bus and add devices to it.
> > >
> > > Heck, I'll even write that code if you want it, what's the requirement,
> > > something like:
> > >     struct device *kunit_device_create(struct kunit *test, const char *name);
> > >     void kunit_device_destroy(struct device *dev);
> >
> > Thanks for the offer Greg. This, however, is being already worked on by
> > David. I don't want to step on his toes by writing the same thing, nor do I
> > think I should be pushing him to rush on his work.
>
> Ok, David, my offer stands, if you want me to write this I will be glad
> to do so.

I'm happy to keep working on this, but would definitely appreciate
your feedback.

I've put my work-in-progress code here:
https://kunit.googlesource.com/linux/+/refs/heads/kunit/device-helpers%5E%21/#F0

It creates a "kunit" bus, and adds a few helpers to create both
devices and drivers on that bus, and clean them up when the test
exits. It seems to work on all of the tests which used
root_device_register so far (except those -- only
test_iio_find_closest_gain_low so far -- which create multiple devices
with the same name, as the driver name won't be unique), and the drm
tests work fine when ported to it as well.

There's still a lot of cleanup to do and questions which need
answering, including:
- Working out how best to provide an owning module (it's currently
just kunit, but probably should be the module which contains the
actual tests)
- Improving the API around drivers: probably exposing the helper to
create a driver, and add a way of cleaning it up early.
- Properly testing it with modules, not just with kunit.py (including
looking at what actually appears in sysfs)
- Experimenting with using probe, etc, callbacks.
- Cleaning up lots of ugly, still experimental code, documenting, testing, etc.

The thought of trying to expand the match function to support, e.g.,
devicetree occurred to me, but I _think_ that devicetree-based tests
are probably still better off using a platform_device. Regardless, it
can probably wait to a follow-up

In any case, does this seem like the right way forward?

Cheers,
-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation
  2023-03-28 12:45         ` David Gow
@ 2023-03-28 13:22           ` Matti Vaittinen
  2023-03-28 13:38             ` David Gow
  2023-03-30 16:53           ` Maxime Ripard
  1 sibling, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-28 13:22 UTC (permalink / raw)
  To: David Gow, Greg Kroah-Hartman
  Cc: Matti Vaittinen, Brendan Higgins, linux-kernel, linux-kselftest,
	kunit-dev, Stephen Boyd, Jonathan Cameron, linux-iio,
	Maxime Ripard

Hi David & Greg and thanks for working with this!

On 3/28/23 15:45, David Gow wrote:
> Thanks, Gred and Matti.
> 
> On Mon, 27 Mar 2023 at 20:38, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>>
>> On Mon, Mar 27, 2023 at 03:20:06PM +0300, Matti Vaittinen wrote:
>>> On 3/27/23 15:01, Greg Kroah-Hartman wrote:
>>>> On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
> 
> I'm happy to keep working on this, but would definitely appreciate
> your feedback.
> 
> I've put my work-in-progress code here:
> https://kunit.googlesource.com/linux/+/refs/heads/kunit/device-helpers%5E%21/#F0
> 
> It creates a "kunit" bus, and adds a few helpers to create both
> devices and drivers on that bus, and clean them up when the test
> exits. It seems to work on all of the tests which used
> root_device_register so far (except those -- only
> test_iio_find_closest_gain_low so far -- which create multiple devices
> with the same name, as the driver name won't be unique),

I wouldn't worry about it for as long as it's just because an iio-gts 
test does something silly. Those tests are currently only in my personal 
playground and changing those tests should be pretty trivial.

And right after saying that - the test_iio_find_closest_gain_low test does

a) register a 'test' device
b) perform test on devm_ API
c) unregister the 'test' device

d) register a 'test' device (same name as at step a)
e) perform test on devm_ API
f) unregister the 'test' device

My assumption is that the test device would be gone after step c) 
because there should be no references to it anywhere. Hence, I wonder 
why registering at step d) fails? (Or did I misunderstand something?)

> and the drm
> tests work fine when ported to it as well.
> 
> There's still a lot of cleanup to do and questions which need
> answering, including:
> - Working out how best to provide an owning module (it's currently
> just kunit, but probably should be the module which contains the
> actual tests)

Maybe there is something I am not seeing but how about wrapping the 
kunit_device_register() in a macro and getting the THIS_MODULE in 
caller's context?

> In any case, does this seem like the right way forward?

I am by no means an expert on this but this does look good to me. I 
would keep this as clean, lean and simple as possible in order to keep 
understanding / debugging the problems exposed by the tests as simple as 
possible. At some point someone is wondering why a test fails, and ends 
up looking through these helpers to ensure problem is no lurking 
there... Hence, I'd kept the code there in minimum - meaning, I might 
not add kunit class or even a driver until tests require that. (Even if 
it would not look as good in the sysfs - as far as I understand the 
kunit sysfs entries are a 'test feature' which should not be present in 
'production systems'. This is not an excuse to make things bad - but (in 
my opinion) this is a good reason to prioritize simplicity.

Anyways, thanks for the work!

Yours,
	-- Matti

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

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation
  2023-03-28 13:22           ` Matti Vaittinen
@ 2023-03-28 13:38             ` David Gow
  0 siblings, 0 replies; 20+ messages in thread
From: David Gow @ 2023-03-28 13:38 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Greg Kroah-Hartman, Matti Vaittinen, Brendan Higgins,
	linux-kernel, linux-kselftest, kunit-dev, Stephen Boyd,
	Jonathan Cameron, linux-iio, Maxime Ripard

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

On Tue, 28 Mar 2023 at 21:22, Matti Vaittinen <mazziesaccount@gmail.com> wrote:
>
> Hi David & Greg and thanks for working with this!
>
> On 3/28/23 15:45, David Gow wrote:
> > Thanks, Gred and Matti.
> >
> > On Mon, 27 Mar 2023 at 20:38, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Mon, Mar 27, 2023 at 03:20:06PM +0300, Matti Vaittinen wrote:
> >>> On 3/27/23 15:01, Greg Kroah-Hartman wrote:
> >>>> On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
> >
> > I'm happy to keep working on this, but would definitely appreciate
> > your feedback.
> >
> > I've put my work-in-progress code here:
> > https://kunit.googlesource.com/linux/+/refs/heads/kunit/device-helpers%5E%21/#F0
> >
> > It creates a "kunit" bus, and adds a few helpers to create both
> > devices and drivers on that bus, and clean them up when the test
> > exits. It seems to work on all of the tests which used
> > root_device_register so far (except those -- only
> > test_iio_find_closest_gain_low so far -- which create multiple devices
> > with the same name, as the driver name won't be unique),
>
> I wouldn't worry about it for as long as it's just because an iio-gts
> test does something silly. Those tests are currently only in my personal
> playground and changing those tests should be pretty trivial.
>

Yeah: this isn't anything to worry about. It's as much my note as to
"what works with this code as-is", rather than indicative of a more
fundamental flaw.

> And right after saying that - the test_iio_find_closest_gain_low test does
>
> a) register a 'test' device
> b) perform test on devm_ API
> c) unregister the 'test' device
>
> d) register a 'test' device (same name as at step a)
> e) perform test on devm_ API
> f) unregister the 'test' device
>
> My assumption is that the test device would be gone after step c)
> because there should be no references to it anywhere. Hence, I wonder
> why registering at step d) fails? (Or did I misunderstand something?)
>

This is because I'm now creating a struct device_driver as well as a
device, and it's not being cleaned up until the end of the test.

The two solutions here would be to either:
- Add a way to clean up the driver earlier. (Shouldn't be too hard, I
just haven't got around to it yet), or:
- Use the same struct device_driver for both tests (there's a new
kunit_device_register_with_driver which allows you to pass a custom
driver in, rather than creating your own)

I think the latter's probably better, but I'll probably implement both
as I clean up the API further.

> > and the drm
> > tests work fine when ported to it as well.
> >
> > There's still a lot of cleanup to do and questions which need
> > answering, including:
> > - Working out how best to provide an owning module (it's currently
> > just kunit, but probably should be the module which contains the
> > actual tests)
>
> Maybe there is something I am not seeing but how about wrapping the
> kunit_device_register() in a macro and getting the THIS_MODULE in
> caller's context?
>

Yeah: that's probably what I'll do. The other possibility is to store
the module pointer in the struct kunit context.

> > In any case, does this seem like the right way forward?
>
> I am by no means an expert on this but this does look good to me. I
> would keep this as clean, lean and simple as possible in order to keep
> understanding / debugging the problems exposed by the tests as simple as
> possible. At some point someone is wondering why a test fails, and ends
> up looking through these helpers to ensure problem is no lurking
> there... Hence, I'd kept the code there in minimum - meaning, I might
> not add kunit class or even a driver until tests require that. (Even if
> it would not look as good in the sysfs - as far as I understand the
> kunit sysfs entries are a 'test feature' which should not be present in
> 'production systems'. This is not an excuse to make things bad - but (in
> my opinion) this is a good reason to prioritize simplicity.

I agree with you that it's best to avoid complexity for as long as we
reasonably can. I think that there are enough things which would
benefit from having the driver stuff to make it worth having
_something_ there, particularly since it makes this a more "normal"
device, so hopefully will be less surprising.

For sysfs, it's one of those things which shows up pretty rarely in
day-to-day KUnit use, as most people are using the kunit.py script
which has all of the tests built-in, and no userspace to look at sysfs
from. That being said, that doesn't cover all use cases, and I
definitely would rather not make sysfs unusably ugly for everyone
else, so I'm happy to tidy it up. (I'm not planning to do a kunit
class at the moment, though.)

I _think_ this approach (once the details of the API and
implementation are tidied up a bit) should sit in about the sweet spot
for complexity, assuming there's nothing horribly wrong with it I
haven't noticed. I suspect we're better off leaving some of the more
advanced use-cases to implement their own way of instantiating
devices, at least for now, and focus on getting these common cases
right.

Cheers,
-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 2/7] iio: light: Add gain-time-scale helpers
  2023-03-27 11:28 ` [PATCH v6 2/7] iio: light: Add gain-time-scale helpers Matti Vaittinen
@ 2023-03-30 16:48   ` Matti Vaittinen
  2023-03-31  7:39     ` Matti Vaittinen
  0 siblings, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-30 16:48 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-kernel, linux-iio

On 3/27/23 14:28, Matti Vaittinen wrote:
> 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.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
> 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.
> 

> +/**
> + * iio_gts_find_new_gain_sel_by_old_gain_time - compensate for time change
> + * @gts:		Gain time scale descriptor
> + * @old_gain:		Previously set gain
> + * @old_time_sel:	Selector corresponding previously set time
> + * @new_time_sel:	Selector corresponding new time to be set
> + * @new_gain:		Pointer to value where new gain is to be written
> + *
> + * We may want to mitigate the scale change caused by setting a new integration
> + * time (for a light sensor) by also updating the (HW)gain. This helper computes
> + * new gain value to maintain the scale with new integration time.
> + *
> + * Return: 0 on success. -EINVAL if gain matching the new time is not found.

Here we need to document another return value denote whether the 
@new_gain was updated.

> + */
> +int iio_gts_find_new_gain_sel_by_old_gain_time(struct iio_gts *gts,
> +					       int old_gain, int old_time_sel,
> +					       int new_time_sel, int *new_gain)
> +{
> +	const struct iio_itime_sel_mul *itime_old, *itime_new;
> +	u64 scale;
> +	int ret;
> +
> +	itime_old = iio_gts_find_itime_by_sel(gts, old_time_sel);
> +	if (!itime_old)
> +		return -EINVAL;
> +
> +	itime_new = iio_gts_find_itime_by_sel(gts, new_time_sel);
> +	if (!itime_new)
> +		return -EINVAL;
> +
> +	ret = iio_gts_get_scale_linear(gts, old_gain, itime_old->time_us,
> +				       &scale);
> +	if (ret)
> +		return ret;
> +
> +	ret = gain_get_scale_fraction(gts->max_scale, scale, itime_new->mul,
> +				      new_gain);
> +	if (ret)
> +		return ret;
> +
> +	if (!iio_gts_valid_gain(gts, *new_gain))
> +		return -EINVAL;

I would change this to -ERANGE to differentiate the case where the new 
gain was computed but was not valid. The bu27034 (and 
not-yet-fully-finished bu27008) driver uses the computed gain to find 
closest matching gain the hardware supports. I am not super happy with 
the -ERANGE, as it is also possible the gain is in the "range" of 
supported gains but not _exactly_ supported one. In a sense -EINVAL 
would be more correct. The invalid time could in a sense be interpreted 
as an "time selector not found" - so maybe the -ENOENT could be somehow 
tolerated. Still, in my opinion the invalid integration time is very 
much more an -EINVAL than anything else...

> +
> +	return 0;
> +}

I will fix this for v7.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation
  2023-03-28 12:45         ` David Gow
  2023-03-28 13:22           ` Matti Vaittinen
@ 2023-03-30 16:53           ` Maxime Ripard
  1 sibling, 0 replies; 20+ messages in thread
From: Maxime Ripard @ 2023-03-30 16:53 UTC (permalink / raw)
  To: David Gow
  Cc: Greg Kroah-Hartman, Matti Vaittinen, Matti Vaittinen,
	Brendan Higgins, linux-kernel, linux-kselftest, kunit-dev,
	Stephen Boyd, Jonathan Cameron, linux-iio

Hi,

On Tue, Mar 28, 2023 at 08:45:09PM +0800, David Gow wrote:
> > > Ok. I understood using the root-device has been a work-around in some other
> > > tests. Thus continuing use it for tests where we don't need the bus until we
> > > have a proper alternative was suggested by David.
> > >
> > > > Do the right thing here, create a fake bus and add devices to it.
> > > >
> > > > Heck, I'll even write that code if you want it, what's the requirement,
> > > > something like:
> > > >     struct device *kunit_device_create(struct kunit *test, const char *name);
> > > >     void kunit_device_destroy(struct device *dev);
> > >
> > > Thanks for the offer Greg. This, however, is being already worked on by
> > > David. I don't want to step on his toes by writing the same thing, nor do I
> > > think I should be pushing him to rush on his work.
> >
> > Ok, David, my offer stands, if you want me to write this I will be glad
> > to do so.
> 
> I'm happy to keep working on this, but would definitely appreciate
> your feedback.
> 
> I've put my work-in-progress code here:
> https://kunit.googlesource.com/linux/+/refs/heads/kunit/device-helpers%5E%21/#F0
> 
> It creates a "kunit" bus, and adds a few helpers to create both
> devices and drivers on that bus, and clean them up when the test
> exits. It seems to work on all of the tests which used
> root_device_register so far (except those -- only
> test_iio_find_closest_gain_low so far -- which create multiple devices
> with the same name, as the driver name won't be unique), and the drm
> tests work fine when ported to it as well.
> 
> There's still a lot of cleanup to do and questions which need
> answering, including:
> - Working out how best to provide an owning module (it's currently
> just kunit, but probably should be the module which contains the
> actual tests)
> - Improving the API around drivers: probably exposing the helper to
> create a driver, and add a way of cleaning it up early.

I'm not sure we need to give the ability for a test to pass a driver.
I'd expect there's two main use-cases: either we want to test a function
that uses a device as an argument, or we want to probe the whole driver
and test it.

The former is covered by kunit_device_register(), and I'd expect the
latter to be covered by the work Stephen is doing, where we will load an
entire overlay, which will in turn probe the driver.

> - Properly testing it with modules, not just with kunit.py (including
> looking at what actually appears in sysfs)
> - Experimenting with using probe, etc, callbacks.
> - Cleaning up lots of ugly, still experimental code, documenting, testing, etc.
>
> The thought of trying to expand the match function to support, e.g.,
> devicetree occurred to me, but I _think_ that devicetree-based tests
> are probably still better off using a platform_device. Regardless, it
> can probably wait to a follow-up

Yeah, probing the entire driver will require to instantiate the device
the driver expects, hence why relying on the overlays also makes a lot
of sense.

Maxime

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor
  2023-03-27 11:34 ` [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor Matti Vaittinen
  2023-03-28  5:59   ` kernel test robot
@ 2023-03-30 16:59   ` Matti Vaittinen
  1 sibling, 0 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-30 16:59 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko,
	Zhigang Shi, Shreeya Patel, Dmitry Osipenko, Paul Gazzillo,
	linux-kernel, linux-iio

On 3/27/23 14:34, Matti Vaittinen wrote:
> 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.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---

...

> +/*
> + * We try to change the time in such way that the scale is maintained for
> + * given channels by adjusting gain so that it compensates the time change.
> + */
> +static int bu27034_try_set_int_time(struct bu27034_data *data, int time_us)
> +{
> +	struct bu27034_gain_check gains[] = {
> +		{ .chan = BU27034_CHAN_DATA0 },
> +		{ .chan = BU27034_CHAN_DATA1 },
> +	};
> +	int numg = ARRAY_SIZE(gains);
> +	int ret, int_time_old, i;
> +
> +	mutex_lock(&data->mutex);
> +	ret = bu27034_get_int_time(data);
> +	if (ret < 0)
> +		goto unlock_out;
> +
> +	int_time_old = ret;
> +
> +	if (!iio_gts_valid_time(&data->gts, time_us)) {
> +		dev_err(data->dev, "Unsupported integration time %u\n",
> +			time_us);
> +		ret = -EINVAL;
> +
> +		goto unlock_out;
> +	}
> +
> +	if (time_us == int_time_old) {
> +		ret = 0;
> +		goto unlock_out;
> +	}
> +
> +	for (i = 0; i < numg; i++) {
> +		ret = bu27034_get_gain(data, gains[i].chan, &gains[i].old_gain);
> +		if (ret)
> +			goto unlock_out;
> +
> +		ret = iio_gts_find_new_gain_by_old_gain_time(&data->gts,
> +							     gains[i].old_gain,
> +							     int_time_old, time_us,
> +							     &gains[i].new_gain);
> +		if (ret) {

As mentioned in my comment to the helper patch, we should have different 
returnvalue here depending on if the given times were invalid (and new 
gain was not computed) or if the new_gain was computed but not 
supported. I plan to use -ERANGE to denote "new gain computed but not 
supported" and add...

> +			int scale1, scale2;
> +			bool ok;
> +
> +			_bu27034_get_scale(data, gains[i].chan, &scale1, &scale2);
> +			dev_dbg(data->dev,
> +				"chan %u, can't support time %u with scale %u %u\n",
> +				gains[i].chan, time_us, scale1, scale2);

+                       if (ret != -ERANGE)
+                               goto unlock_out;

... here at v7.

> +
> +			/*
> +			 * If caller requests for integration time change and we
> +			 * can't support the scale - then the caller should be
> +			 * prepared to 'pick up the pieces and deal with the
> +			 * fact that the scale changed'.
> +			 */
> +			ret = iio_find_closest_gain_low(&data->gts,
> +							gains[i].new_gain, &ok);
> +
> +			if (!ok) {
> +				dev_dbg(data->dev,
> +					"optimal gain out of range for chan %u\n",
> +					gains[i].chan);
> +			}
> +			if (ret < 0) {
> +				dev_dbg(data->dev,
> +					 "Total gain increase. Risk of saturation");
> +				ret = iio_gts_get_min_gain(&data->gts);
> +				if (ret < 0)
> +					goto unlock_out;
> +			}
> +			dev_dbg(data->dev, "chan %u scale changed\n",
> +				 gains[i].chan);
> +			gains[i].new_gain = ret;
> +			dev_dbg(data->dev, "chan %u new gain %u\n",
> +				gains[i].chan, gains[i].new_gain);
> +		}
> +	}
> +
> +	for (i = 0; i < numg; i++) {
> +		ret = bu27034_set_gain(data, gains[i].chan, gains[i].new_gain);
> +		if (ret)
> +			goto unlock_out;
> +	}
> +
> +	ret = bu27034_set_int_time(data, time_us);
> +
> +unlock_out:
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +


Yours,
	-- Matti

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

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6 2/7] iio: light: Add gain-time-scale helpers
  2023-03-30 16:48   ` Matti Vaittinen
@ 2023-03-31  7:39     ` Matti Vaittinen
  0 siblings, 0 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-03-31  7:39 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Jonathan Cameron, Lars-Peter Clausen, linux-kernel, linux-iio

On 3/30/23 19:48, Matti Vaittinen wrote:
> On 3/27/23 14:28, Matti Vaittinen wrote:
>> 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.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>> 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.
>>
> 
>> +/**
>> + * iio_gts_find_new_gain_sel_by_old_gain_time - compensate for time 
>> change
>> + * @gts:        Gain time scale descriptor
>> + * @old_gain:        Previously set gain
>> + * @old_time_sel:    Selector corresponding previously set time
>> + * @new_time_sel:    Selector corresponding new time to be set
>> + * @new_gain:        Pointer to value where new gain is to be written
>> + *
>> + * We may want to mitigate the scale change caused by setting a new 
>> integration
>> + * time (for a light sensor) by also updating the (HW)gain. This 
>> helper computes
>> + * new gain value to maintain the scale with new integration time.
>> + *
>> + * Return: 0 on success. -EINVAL if gain matching the new time is not 
>> found.
> 
> Here we need to document another return value denote whether the 
> @new_gain was updated.
> 
>> + */
>> +int iio_gts_find_new_gain_sel_by_old_gain_time(struct iio_gts *gts,
>> +                           int old_gain, int old_time_sel,
>> +                           int new_time_sel, int *new_gain)
>> +{
>> +    const struct iio_itime_sel_mul *itime_old, *itime_new;
>> +    u64 scale;
>> +    int ret;
>> +
>> +    itime_old = iio_gts_find_itime_by_sel(gts, old_time_sel);
>> +    if (!itime_old)
>> +        return -EINVAL;
>> +
>> +    itime_new = iio_gts_find_itime_by_sel(gts, new_time_sel);
>> +    if (!itime_new)
>> +        return -EINVAL;
>> +
>> +    ret = iio_gts_get_scale_linear(gts, old_gain, itime_old->time_us,
>> +                       &scale);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = gain_get_scale_fraction(gts->max_scale, scale, itime_new->mul,
>> +                      new_gain);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (!iio_gts_valid_gain(gts, *new_gain))
>> +        return -EINVAL;
> 
> I would change this to -ERANGE to differentiate the case where the new 
> gain was computed but was not valid. The bu27034 (and 
> not-yet-fully-finished bu27008) driver uses the computed gain to find 
> closest matching gain the hardware supports. I am not super happy with 
> the -ERANGE, as it is also possible the gain is in the "range" of 
> supported gains but not _exactly_ supported one. In a sense -EINVAL 
> would be more correct. The invalid time could in a sense be interpreted 
> as an "time selector not found" - so maybe the -ENOENT could be somehow 
> tolerated. Still, in my opinion the invalid integration time is very 
> much more an -EINVAL than anything else...

Looks like I keep discussing with myself. This however was not a good 
solution as we might detect non integer gain to be required in the 
gain_get_scale_fraction(). And deciding if that function should return 
-ERANGE or -EINVAL got things even worse.

So, the take N (where N is a positive integer, much greater than 1) is 
that I'll do:

int iio_gts_find_new_gain_sel_by_old_gain_time(struct iio_gts *gts,
                            int old_gain, int old_time_sel,
                            int new_time_sel, int *new_gain)
{
     const struct iio_itime_sel_mul *itime_old, *itime_new;
     u64 scale;
     int ret;

     *new_gain = -1;

and add return value doc like:

  * Return: 0 if an exactly matching supported new gain was found. When 
a
  * non-zero value is returned, the @new_gain will be set to a negative or
  * positive value. The negative value means that no gain could be computed.
  * Positive value will be the "best possible new gain there could be". 
There
  * can be two reasons why finding the "best possible" new gain is not 
deemed
  * successful. 1) This new value cannot be supported by the hardware. 
2) The new
  * gain required to maintain the scale would not be an integer. In this 
case,
  * the "best possible" new gain will be a floored optimal gain, which 
may or
  * may not be supported by the hardware.

> 
> I will fix this for v7.
> 

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

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-03-31  7:39 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-27 11:27 [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen
2023-03-27 11:27 ` [PATCH v6 1/7] dt-bindings: iio: light: Support ROHM BU27034 Matti Vaittinen
2023-03-27 11:28 ` [PATCH v6 2/7] iio: light: Add gain-time-scale helpers Matti Vaittinen
2023-03-30 16:48   ` Matti Vaittinen
2023-03-31  7:39     ` Matti Vaittinen
2023-03-27 11:34 ` [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation Matti Vaittinen
2023-03-27 12:01   ` Greg Kroah-Hartman
2023-03-27 12:20     ` Matti Vaittinen
2023-03-27 12:38       ` Greg Kroah-Hartman
2023-03-28 12:45         ` David Gow
2023-03-28 13:22           ` Matti Vaittinen
2023-03-28 13:38             ` David Gow
2023-03-30 16:53           ` Maxime Ripard
2023-03-27 11:34 ` [PATCH v6 4/7] iio: test: test gain-time-scale helpers Matti Vaittinen
2023-03-27 11:34 ` [PATCH v6 5/7] MAINTAINERS: Add IIO " Matti Vaittinen
2023-03-27 11:34 ` [PATCH v6 6/7] iio: light: ROHM BU27034 Ambient Light Sensor Matti Vaittinen
2023-03-28  5:59   ` kernel test robot
2023-03-30 16:59   ` Matti Vaittinen
2023-03-27 11:35 ` [PATCH v6 7/7] MAINTAINERS: Add ROHM BU27034 Matti Vaittinen
2023-03-27 11:39 ` [PATCH v6 0/7] Support ROHM BU27034 ALS sensor Matti Vaittinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.