dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/8] Support ROHM BU27034 ALS sensor
@ 2023-03-17 14:40 Matti Vaittinen
  2023-03-17 14:41 ` [PATCH v4 1/8] drm/tests: helpers: rename generic helpers Matti Vaittinen
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Matti Vaittinen @ 2023-03-17 14:40 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki, linux-iio,
	dri-devel, Krzysztof Kozlowski, Zhigang Shi, Masahiro Yamada,
	Maíra Canal, Javier Martinez Canillas, Dmitry Osipenko,
	devicetree, Matti Vaittinen, Paul Gazzillo, Liam Beguin,
	Rob Herring, Andy Shevchenko, Stephen Boyd, Greg Kroah-Hartman,
	Randy Dunlap, linux-kernel, Noralf Trønnes,
	Thomas Zimmermann, Shreeya Patel, Jonathan Cameron

[-- Attachment #1: Type: text/plain, Size: 8764 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'. There were neat helpers in DRM tests for creating and freeing
such a device. This series moves those helpers to more generic location.
What is worth noting is that there is something similar ongoing in the
CCF territory:
https://lore.kernel.org/all/20230302013822.1808711-1-sboyd@kernel.org/
These efforts should be somehow coordinated in order to avoid any ajor
conflicts.

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

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

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

Revision history:
v3 => v4: (Stil ostly 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 (8):
  drm/tests: helpers: rename generic helpers
  kunit: drm/tests: move generic helpers
  dt-bindings: iio: light: Support ROHM BU27034
  iio: light: Add gain-time-scale helpers
  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/base/test/Kconfig                     |    5 +
 drivers/base/test/Makefile                    |    2 +
 drivers/base/test/test_kunit_device.c         |   83 +
 drivers/gpu/drm/Kconfig                       |    2 +
 .../gpu/drm/tests/drm_client_modeset_test.c   |    5 +-
 drivers/gpu/drm/tests/drm_kunit_helpers.c     |   69 -
 drivers/gpu/drm/tests/drm_managed_test.c      |    5 +-
 drivers/gpu/drm/tests/drm_modes_test.c        |    5 +-
 drivers/gpu/drm/tests/drm_probe_helper_test.c |    5 +-
 drivers/gpu/drm/vc4/Kconfig                   |    1 +
 drivers/gpu/drm/vc4/tests/vc4_mock.c          |    3 +-
 .../gpu/drm/vc4/tests/vc4_test_pv_muxing.c    |    9 +-
 drivers/iio/Kconfig                           |    3 +
 drivers/iio/Makefile                          |    1 +
 drivers/iio/industrialio-gts-helper.c         |  990 +++++++++++
 drivers/iio/light/Kconfig                     |   14 +
 drivers/iio/light/Makefile                    |    1 +
 drivers/iio/light/rohm-bu27034.c              | 1491 +++++++++++++++++
 drivers/iio/test/Kconfig                      |   16 +
 drivers/iio/test/Makefile                     |    1 +
 drivers/iio/test/iio-test-gts.c               |  461 +++++
 include/drm/drm_kunit_helpers.h               |    7 +-
 include/linux/iio/iio-gts-helper.h            |  113 ++
 25 files changed, 3265 insertions(+), 87 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
 create mode 100644 drivers/base/test/test_kunit_device.c
 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/linux/iio/iio-gts-helper.h


base-commit: eeac8ede17557680855031c6f305ece2378af326
-- 
2.39.2


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

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

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

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

* [PATCH v4 1/8] drm/tests: helpers: rename generic helpers
  2023-03-17 14:40 [PATCH v4 0/8] Support ROHM BU27034 ALS sensor Matti Vaittinen
@ 2023-03-17 14:41 ` Matti Vaittinen
  2023-03-17 14:42 ` [PATCH v4 2/8] kunit: drm/tests: move " Matti Vaittinen
  2023-03-19 16:57 ` [PATCH v4 0/8] Support ROHM BU27034 ALS sensor Jonathan Cameron
  2 siblings, 0 replies; 12+ messages in thread
From: Matti Vaittinen @ 2023-03-17 14:41 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Thomas Zimmermann, Emma Anholt, Matti Vaittinen, Stephen Boyd,
	Maíra Canal, Javier Martinez Canillas, linux-kernel,
	Noralf Trønnes, dri-devel

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

The creation of a dummy device in order to test managed interfaces is a
generally useful test feature. The drm test helpers
drm_kunit_helper_alloc_device() and drm_kunit_helper_free_device()
are doing exactly this. It makes no sense that each and every component
which intends to be testing managed interfaces will create similar
helpers so rename these to more generic format.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/gpu/drm/tests/drm_client_modeset_test.c |  4 ++--
 drivers/gpu/drm/tests/drm_kunit_helpers.c       | 16 ++++++++--------
 drivers/gpu/drm/tests/drm_managed_test.c        |  4 ++--
 drivers/gpu/drm/tests/drm_modes_test.c          |  4 ++--
 drivers/gpu/drm/tests/drm_probe_helper_test.c   |  4 ++--
 drivers/gpu/drm/vc4/tests/vc4_mock.c            |  2 +-
 drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c  |  8 ++++----
 include/drm/drm_kunit_helpers.h                 |  8 ++++----
 8 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c
index 416a279b6dae..27ab03d1c518 100644
--- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -60,7 +60,7 @@ static int drm_client_modeset_test_init(struct kunit *test)
 
 	test->priv = priv;
 
-	priv->dev = drm_kunit_helper_alloc_device(test);
+	priv->dev = test_kunit_helper_alloc_device(test);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev);
 
 	priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev,
@@ -86,7 +86,7 @@ static void drm_client_modeset_test_exit(struct kunit *test)
 {
 	struct drm_client_modeset_test_priv *priv = test->priv;
 
-	drm_kunit_helper_free_device(test, priv->dev);
+	test_kunit_helper_free_device(test, priv->dev);
 }
 
 static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test)
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index e98b4150f556..ec93b0300f03 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -33,7 +33,7 @@ static struct platform_driver fake_platform_driver = {
 };
 
 /**
- * drm_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
+ * test_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
  * @test: The test context object
  *
  * This allocates a fake struct &device to create a mock for a KUnit
@@ -41,13 +41,13 @@ static struct platform_driver fake_platform_driver = {
  * able to leverage the usual infrastructure and most notably the
  * device-managed resources just like a "real" device.
  *
- * Callers need to make sure drm_kunit_helper_free_device() on the
+ * Callers need to make sure test_kunit_helper_free_device() on the
  * device when done.
  *
  * Returns:
  * A pointer to the new device, or an ERR_PTR() otherwise.
  */
-struct device *drm_kunit_helper_alloc_device(struct kunit *test)
+struct device *test_kunit_helper_alloc_device(struct kunit *test)
 {
 	struct platform_device *pdev;
 	int ret;
@@ -63,23 +63,23 @@ struct device *drm_kunit_helper_alloc_device(struct kunit *test)
 
 	return &pdev->dev;
 }
-EXPORT_SYMBOL_GPL(drm_kunit_helper_alloc_device);
+EXPORT_SYMBOL_GPL(test_kunit_helper_alloc_device);
 
 /**
- * drm_kunit_helper_free_device - Frees a mock device
+ * test_kunit_helper_free_device - Frees a mock device
  * @test: The test context object
  * @dev: The device to free
  *
- * Frees a device allocated with drm_kunit_helper_alloc_device().
+ * Frees a device allocated with test_kunit_helper_alloc_device().
  */
-void drm_kunit_helper_free_device(struct kunit *test, struct device *dev)
+void test_kunit_helper_free_device(struct kunit *test, struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 
 	platform_device_unregister(pdev);
 	platform_driver_unregister(&fake_platform_driver);
 }
-EXPORT_SYMBOL_GPL(drm_kunit_helper_free_device);
+EXPORT_SYMBOL_GPL(test_kunit_helper_free_device);
 
 struct drm_device *
 __drm_kunit_helper_alloc_drm_device_with_driver(struct kunit *test,
diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
index 1652dca11d30..53f870493577 100644
--- a/drivers/gpu/drm/tests/drm_managed_test.c
+++ b/drivers/gpu/drm/tests/drm_managed_test.c
@@ -35,7 +35,7 @@ static void drm_test_managed_run_action(struct kunit *test)
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
 	init_waitqueue_head(&priv->action_wq);
 
-	dev = drm_kunit_helper_alloc_device(test);
+	dev = test_kunit_helper_alloc_device(test);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
 
 	drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_MODESET);
@@ -48,7 +48,7 @@ static void drm_test_managed_run_action(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, ret, 0);
 
 	drm_dev_unregister(drm);
-	drm_kunit_helper_free_device(test, dev);
+	test_kunit_helper_free_device(test, dev);
 
 	ret = wait_event_interruptible_timeout(priv->action_wq, priv->action_done,
 					       msecs_to_jiffies(TEST_TIMEOUT_MS));
diff --git a/drivers/gpu/drm/tests/drm_modes_test.c b/drivers/gpu/drm/tests/drm_modes_test.c
index bc4aa2ce78be..1bd8540086e9 100644
--- a/drivers/gpu/drm/tests/drm_modes_test.c
+++ b/drivers/gpu/drm/tests/drm_modes_test.c
@@ -23,7 +23,7 @@ static int drm_test_modes_init(struct kunit *test)
 	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 
-	priv->dev = drm_kunit_helper_alloc_device(test);
+	priv->dev = test_kunit_helper_alloc_device(test);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev);
 
 	priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev,
@@ -40,7 +40,7 @@ static void drm_test_modes_exit(struct kunit *test)
 {
 	struct drm_test_modes_priv *priv = test->priv;
 
-	drm_kunit_helper_free_device(test, priv->dev);
+	test_kunit_helper_free_device(test, priv->dev);
 }
 
 static void drm_test_modes_analog_tv_ntsc_480i(struct kunit *test)
diff --git a/drivers/gpu/drm/tests/drm_probe_helper_test.c b/drivers/gpu/drm/tests/drm_probe_helper_test.c
index 0ee65828623e..bc4b271bec09 100644
--- a/drivers/gpu/drm/tests/drm_probe_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_probe_helper_test.c
@@ -40,7 +40,7 @@ static int drm_probe_helper_test_init(struct kunit *test)
 	KUNIT_ASSERT_NOT_NULL(test, priv);
 	test->priv = priv;
 
-	priv->dev = drm_kunit_helper_alloc_device(test);
+	priv->dev = test_kunit_helper_alloc_device(test);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv->dev);
 
 	priv->drm = __drm_kunit_helper_alloc_drm_device(test, priv->dev,
@@ -64,7 +64,7 @@ static void drm_probe_helper_test_exit(struct kunit *test)
 {
 	struct drm_probe_helper_test_priv *priv = test->priv;
 
-	drm_kunit_helper_free_device(test, priv->dev);
+	test_kunit_helper_free_device(test, priv->dev);
 }
 
 typedef struct drm_display_mode *(*expected_mode_func_t)(struct drm_device *);
diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock.c b/drivers/gpu/drm/vc4/tests/vc4_mock.c
index a4bed26af32f..5bb1fa828a3f 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_mock.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_mock.c
@@ -162,7 +162,7 @@ static struct vc4_dev *__mock_device(struct kunit *test, bool is_vc5)
 	struct device *dev;
 	int ret;
 
-	dev = drm_kunit_helper_alloc_device(test);
+	dev = test_kunit_helper_alloc_device(test);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
 
 	vc4 = drm_kunit_helper_alloc_drm_device_with_driver(test, dev,
diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index ae0bd0f81698..8b373fa76d6f 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -762,7 +762,7 @@ static void vc4_pv_muxing_test_exit(struct kunit *test)
 	drm_modeset_drop_locks(&priv->ctx);
 	drm_modeset_acquire_fini(&priv->ctx);
 	drm_dev_unregister(drm);
-	drm_kunit_helper_free_device(test, vc4->dev);
+	test_kunit_helper_free_device(test, vc4->dev);
 }
 
 static struct kunit_case vc4_pv_muxing_tests[] = {
@@ -873,7 +873,7 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 	drm_dev_unregister(drm);
-	drm_kunit_helper_free_device(test, vc4->dev);
+	test_kunit_helper_free_device(test, vc4->dev);
 }
 
 static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
@@ -963,7 +963,7 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 	drm_dev_unregister(drm);
-	drm_kunit_helper_free_device(test, vc4->dev);
+	test_kunit_helper_free_device(test, vc4->dev);
 }
 
 static void
@@ -1017,7 +1017,7 @@ drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct ku
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 	drm_dev_unregister(drm);
-	drm_kunit_helper_free_device(test, vc4->dev);
+	test_kunit_helper_free_device(test, vc4->dev);
 }
 
 static struct kunit_case vc5_pv_muxing_bugs_tests[] = {
diff --git a/include/drm/drm_kunit_helpers.h b/include/drm/drm_kunit_helpers.h
index ed013fdcc1ff..8e3aae6a5ed5 100644
--- a/include/drm/drm_kunit_helpers.h
+++ b/include/drm/drm_kunit_helpers.h
@@ -8,8 +8,8 @@
 struct drm_device;
 struct kunit;
 
-struct device *drm_kunit_helper_alloc_device(struct kunit *test);
-void drm_kunit_helper_free_device(struct kunit *test, struct device *dev);
+struct device *test_kunit_helper_alloc_device(struct kunit *test);
+void test_kunit_helper_free_device(struct kunit *test, struct device *dev);
 
 struct drm_device *
 __drm_kunit_helper_alloc_drm_device_with_driver(struct kunit *test,
@@ -27,7 +27,7 @@ __drm_kunit_helper_alloc_drm_device_with_driver(struct kunit *test,
  *
  * This function creates a struct &drm_device from @_dev and @_drv.
  *
- * @_dev should be allocated using drm_kunit_helper_alloc_device().
+ * @_dev should be allocated using test_kunit_helper_alloc_device().
  *
  * The driver is tied to the @_test context and will get cleaned at the
  * end of the test. The drm_device is allocated through
@@ -72,7 +72,7 @@ __drm_kunit_helper_alloc_drm_device(struct kunit *test,
  * This function creates a struct &drm_driver and will create a struct
  * &drm_device from @_dev and that driver.
  *
- * @_dev should be allocated using drm_kunit_helper_alloc_device().
+ * @_dev should be allocated using test_kunit_helper_alloc_device().
  *
  * The driver is tied to the @_test context and will get cleaned at the
  * end of the test. The drm_device is allocated through
-- 
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] 12+ messages in thread

* [PATCH v4 2/8] kunit: drm/tests: move generic helpers
  2023-03-17 14:40 [PATCH v4 0/8] Support ROHM BU27034 ALS sensor Matti Vaittinen
  2023-03-17 14:41 ` [PATCH v4 1/8] drm/tests: helpers: rename generic helpers Matti Vaittinen
@ 2023-03-17 14:42 ` Matti Vaittinen
  2023-03-17 15:09   ` Maxime Ripard
  2023-03-19 16:57 ` [PATCH v4 0/8] Support ROHM BU27034 ALS sensor Jonathan Cameron
  2 siblings, 1 reply; 12+ messages in thread
From: Matti Vaittinen @ 2023-03-17 14:42 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki, Stephen Boyd,
	Greg Kroah-Hartman, Matti Vaittinen, Javier Martinez Canillas,
	linux-kernel, Maíra Canal, Noralf Trønnes, dri-devel,
	Thomas Zimmermann, Andy Shevchenko

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

The creation of a dummy device in order to test managed interfaces is a
generally useful test feature. The drm test helpers
test_kunit_helper_alloc_device() and test_kunit_helper_free_device()
are doing exactly this. It makes no sense that each and every component
which intends to be testing managed interfaces will create similar
helpers.

Move these functions to place where it is more obvious they can be used
also by other subsystems but drm.

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

---

Please note that there's something similat ongoing in the CCF:
https://lore.kernel.org/all/20230302013822.1808711-1-sboyd@kernel.org/

I do like the simplicity of these DRM-originated helpers so I think
they're worth. I do equally like the Stephen's idea of having the
"dummy platform device" related helpers under drivers/base and the
header being in include/kunit/platform_device.h which is similar to real
platform device under include/linux/platform_device.h - so, in the end
of the day I hope Stephen's changes as well as the changes this patch
introduces to end up in those files. This, however, will require some
co-operation to avoid conflicts.
---
 drivers/base/test/Kconfig                     |  5 ++
 drivers/base/test/Makefile                    |  2 +
 drivers/base/test/test_kunit_device.c         | 83 +++++++++++++++++++
 drivers/gpu/drm/Kconfig                       |  2 +
 .../gpu/drm/tests/drm_client_modeset_test.c   |  1 +
 drivers/gpu/drm/tests/drm_kunit_helpers.c     | 69 ---------------
 drivers/gpu/drm/tests/drm_managed_test.c      |  1 +
 drivers/gpu/drm/tests/drm_modes_test.c        |  1 +
 drivers/gpu/drm/tests/drm_probe_helper_test.c |  1 +
 drivers/gpu/drm/vc4/Kconfig                   |  1 +
 drivers/gpu/drm/vc4/tests/vc4_mock.c          |  1 +
 .../gpu/drm/vc4/tests/vc4_test_pv_muxing.c    |  1 +
 include/drm/drm_kunit_helpers.h               |  3 -
 13 files changed, 99 insertions(+), 72 deletions(-)
 create mode 100644 drivers/base/test/test_kunit_device.c

diff --git a/drivers/base/test/Kconfig b/drivers/base/test/Kconfig
index 610a1ba7a467..642b5b183c10 100644
--- a/drivers/base/test/Kconfig
+++ b/drivers/base/test/Kconfig
@@ -1,4 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
+
+config TEST_KUNIT_DEVICE_HELPERS
+	depends on KUNIT
+	tristate
+
 config TEST_ASYNC_DRIVER_PROBE
 	tristate "Build kernel module to test asynchronous driver probing"
 	depends on m
diff --git a/drivers/base/test/Makefile b/drivers/base/test/Makefile
index 7f76fee6f989..49926524ec6f 100644
--- a/drivers/base/test/Makefile
+++ b/drivers/base/test/Makefile
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_TEST_ASYNC_DRIVER_PROBE)	+= test_async_driver_probe.o
 
+obj-$(CONFIG_TEST_KUNIT_DEVICE_HELPERS) += test_kunit_device.o
+
 obj-$(CONFIG_DRIVER_PE_KUNIT_TEST) += property-entry-test.o
 CFLAGS_property-entry-test.o += $(DISABLE_STRUCTLEAK_PLUGIN)
diff --git a/drivers/base/test/test_kunit_device.c b/drivers/base/test/test_kunit_device.c
new file mode 100644
index 000000000000..75790e15b85c
--- /dev/null
+++ b/drivers/base/test/test_kunit_device.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * These helpers have been extracted from drm test code at
+ * drm_kunit_helpers.c which was authored by
+ * Maxime Ripard <maxime@cerno.tech>
+ */
+
+#include <linux/device.h>
+#include <linux/platform_device.h>
+
+#include <kunit/platform_device.h>
+
+#define KUNIT_DEVICE_NAME	"test-kunit-mock-device"
+
+static int fake_probe(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static int fake_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver fake_platform_driver = {
+	.probe	= fake_probe,
+	.remove	= fake_remove,
+	.driver = {
+		.name	= KUNIT_DEVICE_NAME,
+	},
+};
+
+/**
+ * test_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
+ * @test: The test context object
+ *
+ * This allocates a fake struct &device to create a mock for a KUnit
+ * test. The device will also be bound to a fake driver. It will thus be
+ * able to leverage the usual infrastructure and most notably the
+ * device-managed resources just like a "real" device.
+ *
+ * Callers need to make sure test_kunit_helper_free_device() on the
+ * device when done.
+ *
+ * Returns:
+ * A pointer to the new device, or an ERR_PTR() otherwise.
+ */
+struct device *test_kunit_helper_alloc_device(struct kunit *test)
+{
+	struct platform_device *pdev;
+	int ret;
+
+	ret = platform_driver_register(&fake_platform_driver);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	pdev = platform_device_alloc(KUNIT_DEVICE_NAME, PLATFORM_DEVID_NONE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+	ret = platform_device_add(pdev);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	return &pdev->dev;
+}
+EXPORT_SYMBOL_GPL(test_kunit_helper_alloc_device);
+
+/**
+ * test_kunit_helper_free_device - Frees a mock device
+ * @test: The test context object
+ * @dev: The device to free
+ *
+ * Frees a device allocated with test_kunit_helper_alloc_device().
+ */
+void test_kunit_helper_free_device(struct kunit *test, struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	platform_device_unregister(pdev);
+	platform_driver_unregister(&fake_platform_driver);
+}
+EXPORT_SYMBOL_GPL(test_kunit_helper_free_device);
+
+MODULE_AUTHOR("Matti Vaittinen <mazziesaccount@gmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index dc0f94f02a82..0ee8ebe64f57 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -66,6 +66,7 @@ config DRM_USE_DYNAMIC_DEBUG
 config DRM_KUNIT_TEST_HELPERS
 	tristate
 	depends on DRM && KUNIT
+	select TEST_KUNIT_DEVICE_HELPERS
 	help
 	  KUnit Helpers for KMS drivers.
 
@@ -80,6 +81,7 @@ config DRM_KUNIT_TEST
 	select DRM_BUDDY
 	select DRM_EXPORT_FOR_TESTS if m
 	select DRM_KUNIT_TEST_HELPERS
+	select TEST_KUNIT_DEVICE_HELPERS
 	default KUNIT_ALL_TESTS
 	help
 	  This builds unit tests for DRM. This option is not useful for
diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c
index 27ab03d1c518..d7eaa0938eb4 100644
--- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2022 Maxime Ripard <mripard@kernel.org>
  */
 
+#include <kunit/platform_device.h>
 #include <kunit/test.h>
 
 #include <drm/drm_connector.h>
diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c b/drivers/gpu/drm/tests/drm_kunit_helpers.c
index ec93b0300f03..ae84d0ed8744 100644
--- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
+++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
@@ -9,78 +9,9 @@
 #include <linux/device.h>
 #include <linux/platform_device.h>
 
-#define KUNIT_DEVICE_NAME	"drm-kunit-mock-device"
-
 static const struct drm_mode_config_funcs drm_mode_config_funcs = {
 };
 
-static int fake_probe(struct platform_device *pdev)
-{
-	return 0;
-}
-
-static int fake_remove(struct platform_device *pdev)
-{
-	return 0;
-}
-
-static struct platform_driver fake_platform_driver = {
-	.probe	= fake_probe,
-	.remove	= fake_remove,
-	.driver = {
-		.name	= KUNIT_DEVICE_NAME,
-	},
-};
-
-/**
- * test_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
- * @test: The test context object
- *
- * This allocates a fake struct &device to create a mock for a KUnit
- * test. The device will also be bound to a fake driver. It will thus be
- * able to leverage the usual infrastructure and most notably the
- * device-managed resources just like a "real" device.
- *
- * Callers need to make sure test_kunit_helper_free_device() on the
- * device when done.
- *
- * Returns:
- * A pointer to the new device, or an ERR_PTR() otherwise.
- */
-struct device *test_kunit_helper_alloc_device(struct kunit *test)
-{
-	struct platform_device *pdev;
-	int ret;
-
-	ret = platform_driver_register(&fake_platform_driver);
-	KUNIT_ASSERT_EQ(test, ret, 0);
-
-	pdev = platform_device_alloc(KUNIT_DEVICE_NAME, PLATFORM_DEVID_NONE);
-	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
-
-	ret = platform_device_add(pdev);
-	KUNIT_ASSERT_EQ(test, ret, 0);
-
-	return &pdev->dev;
-}
-EXPORT_SYMBOL_GPL(test_kunit_helper_alloc_device);
-
-/**
- * test_kunit_helper_free_device - Frees a mock device
- * @test: The test context object
- * @dev: The device to free
- *
- * Frees a device allocated with test_kunit_helper_alloc_device().
- */
-void test_kunit_helper_free_device(struct kunit *test, struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-
-	platform_device_unregister(pdev);
-	platform_driver_unregister(&fake_platform_driver);
-}
-EXPORT_SYMBOL_GPL(test_kunit_helper_free_device);
-
 struct drm_device *
 __drm_kunit_helper_alloc_drm_device_with_driver(struct kunit *test,
 						struct device *dev,
diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
index 53f870493577..6b39d2cde164 100644
--- a/drivers/gpu/drm/tests/drm_managed_test.c
+++ b/drivers/gpu/drm/tests/drm_managed_test.c
@@ -4,6 +4,7 @@
 #include <drm/drm_kunit_helpers.h>
 #include <drm/drm_managed.h>
 
+#include <kunit/platform_device.h>
 #include <kunit/resource.h>
 
 #include <linux/device.h>
diff --git a/drivers/gpu/drm/tests/drm_modes_test.c b/drivers/gpu/drm/tests/drm_modes_test.c
index 1bd8540086e9..addc4d923a26 100644
--- a/drivers/gpu/drm/tests/drm_modes_test.c
+++ b/drivers/gpu/drm/tests/drm_modes_test.c
@@ -7,6 +7,7 @@
 #include <drm/drm_kunit_helpers.h>
 #include <drm/drm_modes.h>
 
+#include <kunit/platform_device.h>
 #include <kunit/test.h>
 
 #include <linux/units.h>
diff --git a/drivers/gpu/drm/tests/drm_probe_helper_test.c b/drivers/gpu/drm/tests/drm_probe_helper_test.c
index bc4b271bec09..f23213464d34 100644
--- a/drivers/gpu/drm/tests/drm_probe_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_probe_helper_test.c
@@ -13,6 +13,7 @@
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_probe_helper.h>
 
+#include <kunit/platform_device.h>
 #include <kunit/test.h>
 
 struct drm_probe_helper_test_priv {
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 91dcf8d174d6..a4bd96445315 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -39,6 +39,7 @@ config DRM_VC4_KUNIT_TEST
 	tristate "KUnit tests for VC4" if !KUNIT_ALL_TESTS
 	depends on DRM_VC4 && KUNIT
 	select DRM_KUNIT_TEST_HELPERS
+	select TEST_KUNIT_DEVICE_HELPERS
 	default KUNIT_ALL_TESTS
 	help
 	  This builds unit tests for the VC4 DRM/KMS driver. This option is
diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock.c b/drivers/gpu/drm/vc4/tests/vc4_mock.c
index 5bb1fa828a3f..29eb045b0db1 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_mock.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_mock.c
@@ -3,6 +3,7 @@
 #include <drm/drm_drv.h>
 #include <drm/drm_kunit_helpers.h>
 
+#include <kunit/platform_device.h>
 #include <kunit/test.h>
 
 #include "vc4_mock.h"
diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index 8b373fa76d6f..64b90e2e3706 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -12,6 +12,7 @@
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_plane.h>
 
+#include <kunit/platform_device.h>
 #include <kunit/test.h>
 
 #include "../vc4_drv.h"
diff --git a/include/drm/drm_kunit_helpers.h b/include/drm/drm_kunit_helpers.h
index 8e3aae6a5ed5..ab438d97aed3 100644
--- a/include/drm/drm_kunit_helpers.h
+++ b/include/drm/drm_kunit_helpers.h
@@ -8,9 +8,6 @@
 struct drm_device;
 struct kunit;
 
-struct device *test_kunit_helper_alloc_device(struct kunit *test);
-void test_kunit_helper_free_device(struct kunit *test, struct device *dev);
-
 struct drm_device *
 __drm_kunit_helper_alloc_drm_device_with_driver(struct kunit *test,
 						struct device *dev,
-- 
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] 12+ messages in thread

* Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
  2023-03-17 14:42 ` [PATCH v4 2/8] kunit: drm/tests: move " Matti Vaittinen
@ 2023-03-17 15:09   ` Maxime Ripard
  2023-03-19  6:36     ` Matti Vaittinen
  0 siblings, 1 reply; 12+ messages in thread
From: Maxime Ripard @ 2023-03-17 15:09 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki, Stephen Boyd,
	Greg Kroah-Hartman, Maíra Canal, Javier Martinez Canillas,
	dri-devel, linux-kernel, Noralf Trønnes, Matti Vaittinen,
	Thomas Zimmermann, Andy Shevchenko

Hi Matti,

On Fri, Mar 17, 2023 at 04:42:25PM +0200, Matti Vaittinen wrote:
> The creation of a dummy device in order to test managed interfaces is a
> generally useful test feature. The drm test helpers
> test_kunit_helper_alloc_device() and test_kunit_helper_free_device()
> are doing exactly this. It makes no sense that each and every component
> which intends to be testing managed interfaces will create similar
> helpers.
> 
> Move these functions to place where it is more obvious they can be used
> also by other subsystems but drm.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
> 
> Please note that there's something similat ongoing in the CCF:
> https://lore.kernel.org/all/20230302013822.1808711-1-sboyd@kernel.org/
> 
> I do like the simplicity of these DRM-originated helpers so I think
> they're worth. I do equally like the Stephen's idea of having the
> "dummy platform device" related helpers under drivers/base and the
> header being in include/kunit/platform_device.h which is similar to real
> platform device under include/linux/platform_device.h - so, in the end
> of the day I hope Stephen's changes as well as the changes this patch
> introduces to end up in those files. This, however, will require some
> co-operation to avoid conflicts.

I think you would have an easier time if you just copied and renamed
them into the kunit folder as an preparation series.

That way, you wouldn't have to coordinate DRM, CCF and IIO, you'd just
create new helpers that can be reused/converted to by everyone eventually.

Maxime

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

* Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
  2023-03-17 15:09   ` Maxime Ripard
@ 2023-03-19  6:36     ` Matti Vaittinen
  2023-03-20 19:23       ` Stephen Boyd
  0 siblings, 1 reply; 12+ messages in thread
From: Matti Vaittinen @ 2023-03-19  6:36 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki, Stephen Boyd,
	Greg Kroah-Hartman, Maíra Canal, Javier Martinez Canillas,
	dri-devel, linux-kernel, Noralf Trønnes, Matti Vaittinen,
	Thomas Zimmermann, Andy Shevchenko

Hi Maxime & All

First of all - I am sorry. During the last minute rebase I accidentally 
dropped the header file from this series. Will fix that for v5. (Also 
the build bot pointed this mistake).

On 3/17/23 17:09, Maxime Ripard wrote:
> Hi Matti,
> 
> On Fri, Mar 17, 2023 at 04:42:25PM +0200, Matti Vaittinen wrote:
>> The creation of a dummy device in order to test managed interfaces is a
>> generally useful test feature. The drm test helpers
>> test_kunit_helper_alloc_device() and test_kunit_helper_free_device()
>> are doing exactly this. It makes no sense that each and every component
>> which intends to be testing managed interfaces will create similar
>> helpers.
>>
>> Move these functions to place where it is more obvious they can be used
>> also by other subsystems but drm.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>>
>> Please note that there's something similat ongoing in the CCF:
>> https://lore.kernel.org/all/20230302013822.1808711-1-sboyd@kernel.org/
>>
>> I do like the simplicity of these DRM-originated helpers so I think
>> they're worth. I do equally like the Stephen's idea of having the
>> "dummy platform device" related helpers under drivers/base and the
>> header being in include/kunit/platform_device.h which is similar to real
>> platform device under include/linux/platform_device.h - so, in the end
>> of the day I hope Stephen's changes as well as the changes this patch
>> introduces to end up in those files. This, however, will require some
>> co-operation to avoid conflicts.
> 
> I think you would have an easier time if you just copied and renamed
> them into the kunit folder as an preparation series.

Yes. That would simplify the syncing between the trees. It slightly bugs 
me to add dublicate code in kernel-but the clean-up series for DRM users 
could be prepared at the same time. It would be even possible to just 
change the drm-helper to be a wrapper for the generic one - and leave 
the callers intact - although it leaves some seemingly unnecessary 
"onion code" there.

> That way, you wouldn't have to coordinate DRM, CCF and IIO, you'd just
> create new helpers that can be reused/converted to by everyone eventually

Yes. Thanks - I think I may go with this approach for the v5 :)

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] 12+ messages in thread

* Re: [PATCH v4 0/8] Support ROHM BU27034 ALS sensor
  2023-03-17 14:40 [PATCH v4 0/8] Support ROHM BU27034 ALS sensor Matti Vaittinen
  2023-03-17 14:41 ` [PATCH v4 1/8] drm/tests: helpers: rename generic helpers Matti Vaittinen
  2023-03-17 14:42 ` [PATCH v4 2/8] kunit: drm/tests: move " Matti Vaittinen
@ 2023-03-19 16:57 ` Jonathan Cameron
  2023-03-20 10:10   ` Matti Vaittinen
  2 siblings, 1 reply; 12+ messages in thread
From: Jonathan Cameron @ 2023-03-19 16:57 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki, linux-iio,
	dri-devel, Krzysztof Kozlowski, Zhigang Shi, Masahiro Yamada,
	Maíra Canal, Javier Martinez Canillas, Dmitry Osipenko,
	devicetree, Paul Gazzillo, Liam Beguin, Rob Herring,
	Andy Shevchenko, Stephen Boyd, Greg Kroah-Hartman, Randy Dunlap,
	Matti Vaittinen, linux-kernel, Noralf Trønnes,
	Thomas Zimmermann, Shreeya Patel

On Fri, 17 Mar 2023 16:40:16 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Support ROHM BU27034 ALS sensor

Hi Matti,

For ease of when this is ready to apply, better to just keep
key mailing lists and individuals cc'd on all patches.

Mind you cc list is random enough I'm guessing it wasn't
deliberate (like the maintainers patch 8 only went to lkml
where no one will notice it)

I can scrape these all of lore, but it's a step that not
all reviewers are going to bother with.

Jonathan



> 
> 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'. There were neat helpers in DRM tests for creating and freeing
> such a device. This series moves those helpers to more generic location.
> What is worth noting is that there is something similar ongoing in the
> CCF territory:
> https://lore.kernel.org/all/20230302013822.1808711-1-sboyd@kernel.org/
> These efforts should be somehow coordinated in order to avoid any ajor
> conflicts.
> 
> Finally, these added helpers do provide some value also for drivers
> which only:
>  a) allow gain change
>   or
>  b) allow changing both the time and gain while trying to maintain the
>     scale.
> 
> For a) we provide the gain - selector (register value) table format +
> selector to gain look-ups, gain <-> scale conversions and the available
> scales helpers.
> 
> For latter case we also provide the time-tables, and actually all the
> APIs should be usable by setting the time multiplier to 1. (not testeted
> thoroughly though).
> 
> Revision history:
> v3 => v4: (Stil ostly 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 (8):
>   drm/tests: helpers: rename generic helpers
>   kunit: drm/tests: move generic helpers
>   dt-bindings: iio: light: Support ROHM BU27034
>   iio: light: Add gain-time-scale helpers
>   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/base/test/Kconfig                     |    5 +
>  drivers/base/test/Makefile                    |    2 +
>  drivers/base/test/test_kunit_device.c         |   83 +
>  drivers/gpu/drm/Kconfig                       |    2 +
>  .../gpu/drm/tests/drm_client_modeset_test.c   |    5 +-
>  drivers/gpu/drm/tests/drm_kunit_helpers.c     |   69 -
>  drivers/gpu/drm/tests/drm_managed_test.c      |    5 +-
>  drivers/gpu/drm/tests/drm_modes_test.c        |    5 +-
>  drivers/gpu/drm/tests/drm_probe_helper_test.c |    5 +-
>  drivers/gpu/drm/vc4/Kconfig                   |    1 +
>  drivers/gpu/drm/vc4/tests/vc4_mock.c          |    3 +-
>  .../gpu/drm/vc4/tests/vc4_test_pv_muxing.c    |    9 +-
>  drivers/iio/Kconfig                           |    3 +
>  drivers/iio/Makefile                          |    1 +
>  drivers/iio/industrialio-gts-helper.c         |  990 +++++++++++
>  drivers/iio/light/Kconfig                     |   14 +
>  drivers/iio/light/Makefile                    |    1 +
>  drivers/iio/light/rohm-bu27034.c              | 1491 +++++++++++++++++
>  drivers/iio/test/Kconfig                      |   16 +
>  drivers/iio/test/Makefile                     |    1 +
>  drivers/iio/test/iio-test-gts.c               |  461 +++++
>  include/drm/drm_kunit_helpers.h               |    7 +-
>  include/linux/iio/iio-gts-helper.h            |  113 ++
>  25 files changed, 3265 insertions(+), 87 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/rohm,bu27034.yaml
>  create mode 100644 drivers/base/test/test_kunit_device.c
>  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/linux/iio/iio-gts-helper.h
> 
> 
> base-commit: eeac8ede17557680855031c6f305ece2378af326


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

* Re: [PATCH v4 0/8] Support ROHM BU27034 ALS sensor
  2023-03-19 16:57 ` [PATCH v4 0/8] Support ROHM BU27034 ALS sensor Jonathan Cameron
@ 2023-03-20 10:10   ` Matti Vaittinen
  0 siblings, 0 replies; 12+ messages in thread
From: Matti Vaittinen @ 2023-03-20 10:10 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki, linux-iio,
	dri-devel, Krzysztof Kozlowski, Zhigang Shi, Masahiro Yamada,
	Maíra Canal, Javier Martinez Canillas, Dmitry Osipenko,
	devicetree, Paul Gazzillo, Liam Beguin, Rob Herring,
	Andy Shevchenko, Stephen Boyd, Greg Kroah-Hartman, Randy Dunlap,
	Matti Vaittinen, linux-kernel, Noralf Trønnes,
	Thomas Zimmermann, Shreeya Patel

On 3/19/23 18:57, Jonathan Cameron wrote:
> On Fri, 17 Mar 2023 16:40:16 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> Support ROHM BU27034 ALS sensor
> 
> Hi Matti,
> 
> For ease of when this is ready to apply, better to just keep
> key mailing lists and individuals cc'd on all patches.

Right. Sorry about this. I kind of rushed the sending at last friday - 
which resulted bunch of errors in the process. I forgot to do the 
spell-check, missed a header and messed the recipients... I should 
really learn to not try meeting artificial deadlines like friday EOB. 
There is Saturday and Sunday - and even if I spent weekend off the 
computer there will likely be the next Monday. (and if there is not, 
then I should probably not care about sending the patches).

> Mind you cc list is random enough I'm guessing it wasn't
> deliberate (like the maintainers patch 8 only went to lkml
> where no one will notice it)

I am using a script which generates the recipients "per patch" using the 
get_maintaner.pl underneath because in many cases certain people are 
only interested in seeing a subset of a series. This avoids polluting 
inboxes when sending large series. For v2 and v3 I did manually add the 
relevant lists / recipients to MAINTAINERS patches which only pick-up 
the LKML list.

> I can scrape these all of lore, but it's a step that not
> all reviewers are going to bother with.

I appreciate the extra mile you're ready to go here as well :) However, 
you should not need to do that. This whole series should've been CC'd to 
you and the iio-list. Sorry again.


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] 12+ messages in thread

* Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
  2023-03-19  6:36     ` Matti Vaittinen
@ 2023-03-20 19:23       ` Stephen Boyd
  2023-03-21  5:45         ` Matti Vaittinen
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2023-03-20 19:23 UTC (permalink / raw)
  To: Matti Vaittinen, Maxime Ripard
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki,
	Greg Kroah-Hartman, Maíra Canal, Javier Martinez Canillas,
	dri-devel, linux-kernel, Noralf Trønnes, Matti Vaittinen,
	Thomas Zimmermann, Andy Shevchenko

Quoting Matti Vaittinen (2023-03-18 23:36:20)
> > 
> > I think you would have an easier time if you just copied and renamed
> > them into the kunit folder as an preparation series.
> 
> Yes. That would simplify the syncing between the trees. It slightly bugs 
> me to add dublicate code in kernel-but the clean-up series for DRM users 
> could be prepared at the same time. It would be even possible to just 
> change the drm-helper to be a wrapper for the generic one - and leave 
> the callers intact - although it leaves some seemingly unnecessary 
> "onion code" there.
> 
> > That way, you wouldn't have to coordinate DRM, CCF and IIO, you'd just
> > create new helpers that can be reused/converted to by everyone eventually
> 
> Yes. Thanks - I think I may go with this approach for the v5 :)

Which kunit directory? I imagine if there are conflicts they will be
trivial so it probably doesn't matter. Have you Cced kunit folks and the
list on the kunit patches? They may have some opinion.

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

* Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
  2023-03-20 19:23       ` Stephen Boyd
@ 2023-03-21  5:45         ` Matti Vaittinen
  2023-03-21 18:59           ` Stephen Boyd
  0 siblings, 1 reply; 12+ messages in thread
From: Matti Vaittinen @ 2023-03-21  5:45 UTC (permalink / raw)
  To: Stephen Boyd, Maxime Ripard
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki,
	Greg Kroah-Hartman, Maíra Canal, Javier Martinez Canillas,
	dri-devel, linux-kernel, Noralf Trønnes, Matti Vaittinen,
	Thomas Zimmermann, Andy Shevchenko

Morning Stephen,

On 3/20/23 21:23, Stephen Boyd wrote:
> Quoting Matti Vaittinen (2023-03-18 23:36:20)
>>>
>>> I think you would have an easier time if you just copied and renamed
>>> them into the kunit folder as an preparation series.
>>
>> Yes. That would simplify the syncing between the trees. It slightly bugs
>> me to add dublicate code in kernel-but the clean-up series for DRM users
>> could be prepared at the same time. It would be even possible to just
>> change the drm-helper to be a wrapper for the generic one - and leave
>> the callers intact - although it leaves some seemingly unnecessary
>> "onion code" there.
>>
>>> That way, you wouldn't have to coordinate DRM, CCF and IIO, you'd just
>>> create new helpers that can be reused/converted to by everyone eventually
>>
>> Yes. Thanks - I think I may go with this approach for the v5 :)
> 
> Which kunit directory?

I was thinking of adding the platform_device.h (I liked your suggestion) 
in the include/kunit/

> I imagine if there are conflicts they will be
> trivial so it probably doesn't matter.

Probably so. Still, I am not the one who needs to deal with the 
conflicts. Hence I like at least asking if people see good way to avoid 
them in the first place.

Besides, I was not sure if you were planning to add similar helper or 
just wrappers to individual functions. Wanted to ping you just in case 
this has some impact to what you do.

> Have you Cced kunit folks and the
> list on the kunit patches? They may have some opinion.

This patch was should have contained the 
include/kunit/platform_device.h. That file was pulling the Kunit people 
in recipients but I messed up things with last minute changes so both 
the header and people were dropped. I'll fix this for v5.

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] 12+ messages in thread

* Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
  2023-03-21  5:45         ` Matti Vaittinen
@ 2023-03-21 18:59           ` Stephen Boyd
  2023-03-22  6:16             ` Vaittinen, Matti
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2023-03-21 18:59 UTC (permalink / raw)
  To: Matti Vaittinen, Maxime Ripard
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki,
	Greg Kroah-Hartman, Maíra Canal, Javier Martinez Canillas,
	dri-devel, linux-kernel, Noralf Trønnes, Matti Vaittinen,
	Thomas Zimmermann, Andy Shevchenko

Quoting Matti Vaittinen (2023-03-20 22:45:52)
> Morning Stephen,
> 
> On 3/20/23 21:23, Stephen Boyd wrote:
> > Quoting Matti Vaittinen (2023-03-18 23:36:20)
> >>>
> >>> I think you would have an easier time if you just copied and renamed
> >>> them into the kunit folder as an preparation series.
> >>
> >> Yes. That would simplify the syncing between the trees. It slightly bugs
> >> me to add dublicate code in kernel-but the clean-up series for DRM users
> >> could be prepared at the same time. It would be even possible to just
> >> change the drm-helper to be a wrapper for the generic one - and leave
> >> the callers intact - although it leaves some seemingly unnecessary
> >> "onion code" there.
> >>
> >>> That way, you wouldn't have to coordinate DRM, CCF and IIO, you'd just
> >>> create new helpers that can be reused/converted to by everyone eventually
> >>
> >> Yes. Thanks - I think I may go with this approach for the v5 :)
> > 
> > Which kunit directory?
> 
> I was thinking of adding the platform_device.h (I liked your suggestion) 
> in the include/kunit/

Ok, thanks for clarifying.

> 
> > I imagine if there are conflicts they will be
> > trivial so it probably doesn't matter.
> 
> Probably so. Still, I am not the one who needs to deal with the 
> conflicts. Hence I like at least asking if people see good way to avoid 
> them in the first place.

Same for me. I'm not the maintainer of the drivers/base directory.

> 
> Besides, I was not sure if you were planning to add similar helper or 
> just wrappers to individual functions. Wanted to ping you just in case 
> this has some impact to what you do.

I don't have a need to bind a device to a driver to satisfy devm APIs
currently. I could probably use it though to test some devm code in the
clk APIs. The only impact is that we're modifying the same files.

> 
> > Have you Cced kunit folks and the
> > list on the kunit patches? They may have some opinion.
> 
> This patch was should have contained the 
> include/kunit/platform_device.h. That file was pulling the Kunit people 
> in recipients but I messed up things with last minute changes so both 
> the header and people were dropped. I'll fix this for v5.
> 

Ok, I'll be on the lookout for v5. From what I can tell kunit goes
through the kernel selftest tree and there isn't a kunit git tree as
such.

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

* Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
  2023-03-21 18:59           ` Stephen Boyd
@ 2023-03-22  6:16             ` Vaittinen, Matti
  2023-03-22  9:44               ` Andy Shevchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Vaittinen, Matti @ 2023-03-22  6:16 UTC (permalink / raw)
  To: Stephen Boyd, Matti Vaittinen, Maxime Ripard
  Cc: Heikki Krogerus, Thomas Zimmermann, Emma Anholt,
	Rafael J. Wysocki, Greg Kroah-Hartman, Maíra Canal,
	Javier Martinez Canillas, dri-devel, linux-kernel,
	Noralf Trønnes, Andy Shevchenko

On 3/21/23 20:59, Stephen Boyd wrote:
> Quoting Matti Vaittinen (2023-03-20 22:45:52)
>> Morning Stephen,
>>
>> On 3/20/23 21:23, Stephen Boyd wrote:
>>> Quoting Matti Vaittinen (2023-03-18 23:36:20)
>> Besides, I was not sure if you were planning to add similar helper or
>> just wrappers to individual functions. Wanted to ping you just in case
>> this has some impact to what you do.
> 
> I don't have a need to bind a device to a driver to satisfy devm APIs
> currently. I could probably use it though to test some devm code in the
> clk APIs. The only impact is that we're modifying the same files.

Thanks for clarifying this.

>>> Have you Cced kunit folks and the
>>> list on the kunit patches? They may have some opinion.
>>
>> This patch was should have contained the
>> include/kunit/platform_device.h. That file was pulling the Kunit people
>> in recipients but I messed up things with last minute changes so both
>> the header and people were dropped. I'll fix this for v5.
>>
> 
> Ok, I'll be on the lookout for v5. From what I can tell kunit goes
> through the kernel selftest tree and there isn't a kunit git tree as
> such.

Right. I am not sure what will be the best tree to carry the testability 
changes. It seems that all of the IIO-tests in v5 will depend on the 
kunit stuff, and I think I will also include the DRM test fixes in this 
series as well just to keep things sorted in my mailbox. Anyways, I hope 
to finish the changes for v5 soon(ish) - maybe already Today and in any 
case during this week. I'll be CC:ing you and Brendan with (relevant 
patches of) v5 as well.

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] 12+ messages in thread

* Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
  2023-03-22  6:16             ` Vaittinen, Matti
@ 2023-03-22  9:44               ` Andy Shevchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2023-03-22  9:44 UTC (permalink / raw)
  To: Vaittinen, Matti
  Cc: Heikki Krogerus, Emma Anholt, Rafael J. Wysocki, Stephen Boyd,
	Greg Kroah-Hartman, Matti Vaittinen, Javier Martinez Canillas,
	dri-devel, linux-kernel, Maíra Canal, Noralf Trønnes,
	Maxime Ripard, Thomas Zimmermann

On Wed, Mar 22, 2023 at 06:16:27AM +0000, Vaittinen, Matti wrote:
> On 3/21/23 20:59, Stephen Boyd wrote:
> > Quoting Matti Vaittinen (2023-03-20 22:45:52)
> >> On 3/20/23 21:23, Stephen Boyd wrote:
> >>> Quoting Matti Vaittinen (2023-03-18 23:36:20)


> >> Besides, I was not sure if you were planning to add similar helper or
> >> just wrappers to individual functions. Wanted to ping you just in case
> >> this has some impact to what you do.
> > 
> > I don't have a need to bind a device to a driver to satisfy devm APIs
> > currently. I could probably use it though to test some devm code in the
> > clk APIs. The only impact is that we're modifying the same files.
> 
> Thanks for clarifying this.
> 
> >>> Have you Cced kunit folks and the
> >>> list on the kunit patches? They may have some opinion.
> >>
> >> This patch was should have contained the
> >> include/kunit/platform_device.h. That file was pulling the Kunit people
> >> in recipients but I messed up things with last minute changes so both
> >> the header and people were dropped. I'll fix this for v5.
> > 
> > Ok, I'll be on the lookout for v5. From what I can tell kunit goes
> > through the kernel selftest tree and there isn't a kunit git tree as
> > such.
> 
> Right. I am not sure what will be the best tree to carry the testability
> changes. It seems that all of the IIO-tests in v5 will depend on the
> kunit stuff, and I think I will also include the DRM test fixes in this
> series as well just to keep things sorted in my mailbox. Anyways, I hope
> to finish the changes for v5 soon(ish) - maybe already Today and in any
> case during this week. I'll be CC:ing you and Brendan with (relevant
> patches of) v5 as well.

Thank you, folks, for doing this. Let's make Linux kernel greater
(than it is already is).

-- 
With Best Regards,
Andy Shevchenko



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

end of thread, other threads:[~2023-03-22  9:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-17 14:40 [PATCH v4 0/8] Support ROHM BU27034 ALS sensor Matti Vaittinen
2023-03-17 14:41 ` [PATCH v4 1/8] drm/tests: helpers: rename generic helpers Matti Vaittinen
2023-03-17 14:42 ` [PATCH v4 2/8] kunit: drm/tests: move " Matti Vaittinen
2023-03-17 15:09   ` Maxime Ripard
2023-03-19  6:36     ` Matti Vaittinen
2023-03-20 19:23       ` Stephen Boyd
2023-03-21  5:45         ` Matti Vaittinen
2023-03-21 18:59           ` Stephen Boyd
2023-03-22  6:16             ` Vaittinen, Matti
2023-03-22  9:44               ` Andy Shevchenko
2023-03-19 16:57 ` [PATCH v4 0/8] Support ROHM BU27034 ALS sensor Jonathan Cameron
2023-03-20 10:10   ` Matti Vaittinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).