Re: [PATCH v5 0/8] Support ROHM BU27034 ALS sensor
On 3/22/23 12:34, Javier Martinez Canillas wrote: Andy Shevchenko writes: Hello Andy, On Wed, Mar 22, 2023 at 11:05:23AM +0200, Matti Vaittinen wrote: Revision history: 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 1) Why do you think this is a problem? 2) How would we avoid spreading more copies of the same code in the future? 1) Merge conflicts is not a bad thing. It shows that people tested their code in isolation and stabilized it before submitting to the upper maintainer. https://yarchive.net/comp/linux/git_merges_from_upstream.html 2) Spreading the same code when we _know_ this, should be very well justified. Merge conflict is an administrative point, and not a technical obstacle to avoid. I definitely agree. This is also why I did the renaming and not copying in the first version. In this version I did still add the subsequent patch 2/8 - which drops the duplicates from DRM tree. I believe this was suggested by Maxime and the rationale is that by just copying the helpers for now, that would make it easier to land instead of requiring coordination between different subystems. This is correct. Otherwise the IIO tree will need to provide an inmutable branch for the DRM tree to merge and so on. Or, if we carry the patch 1/8 via self-test tree, then we get even more players here. Still, I am not opposing immutable branch because that would allow fast applying of the patch 2/8 as well. Longer that is delayed, more likely we will see more users of the DRM helpers and harder it gets to remove the duplicates later. I agree with Maxime that a little bit of duplication (that can be cleaned up by each subsystem at their own pace) is the path of least resistance. I'd say this depends. It probably is the path of least resistance for people maintaining the trees. It can also be the path of least resistance in general - but it depends on if there will be no new users for those DRM helpers while waiting the new APIs being merged in DRM tree. More users we see in DRM, more effort the clean-up requires. I have no strong opinion on this specific case. I'd just be happy to see the IIO tests getting in preferably sooner than later - although 'soon' and 'late' does also depend on other factors besides these helpers... Yours, -- Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
[PATCH v5 2/8] drm/tests: helpers: Use generic helpers
Replace DRM specific managed device creation test-helpers with generic
ones.
Signed-off-by: Matti Vaittinen
---
v4 => v5:
- do not rename + move helpers from DRM but add temporary dublicates to
simplify merging. This patch depends on interface added at patch 1/8.
---
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 +--
include/drm/drm_kunit_helpers.h | 7 +-
10 files changed, 24 insertions(+), 87 deletions(-)
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 416a279b6dae..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
*/
+#include
#include
#include
@@ -60,7 +61,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 +87,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..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
#include
-#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,
- },
-};
-
-/**
- * drm_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
- * @test: The test context object
- *
- * This allocates a fake struct 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 drm_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 platform_device *pdev;
- int ret;
-
- ret = platform_driver_register(_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 >dev;
-}
-EXPORT_SYMBOL_GPL(drm_kunit_helper_alloc_device);
-
-/**
- * drm_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().
- */
-void drm_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(_platform_driver);
-}
-EXPORT_S
[PATCH v5 0/8] Support ROHM BU27034 ALS sensor
ers 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 0xLLU 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):
drivers: kunit: Generic helpers for test device creation
drm/tests: helpers: Use 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 | 1064
drivers/iio/light/Kconfig | 14 +
drivers/iio/light/Makefile|1 +
drivers/iio/light/rohm-bu27034.c | 1482 +
drivers/iio/test/Kconfig | 14 +
drivers/iio/test/Makefile |1 +
drivers/iio/test/iio-test-gts.c | 542 ++
include/drm/drm_kunit_helpers.h |7 +-
include/kunit/platform_device.h | 13 +
include/linux/iio/iio-gts-helper.h| 206 +++
26 files changed, 3515 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/kunit/platform_device.h
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 =]
signature.asc
Description: PGP signature
Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
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! ~~
Re: [PATCH v4 0/8] Support ROHM BU27034 ALS sensor
On 3/19/23 18:57, Jonathan Cameron wrote: On Fri, 17 Mar 2023 16:40:16 +0200 Matti Vaittinen 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! ~~
Re: [PATCH v4 2/8] kunit: drm/tests: move generic helpers
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
---
Please note that there's something similat ongoing in the CCF:
https://lore.kernel.org/all/20230302013822.1808711-1-sb...@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! ~~
[PATCH v4 2/8] kunit: drm/tests: move generic helpers
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
---
Please note that there's something similat ongoing in the CCF:
https://lore.kernel.org/all/20230302013822.1808711-1-sb...@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 ..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
+ */
+
+#include
+#include
+
+#include
+
+#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 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(_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 >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 devi
[PATCH v4 1/8] drm/tests: helpers: rename generic helpers
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
---
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 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 >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(_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(>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
[PATCH v4 0/8] Support ROHM BU27034 ALS sensor
entation to few functions
- replace 0xLLU 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 =]
signature.asc
Description: PGP signature
Re: [RESEND2, v4, 2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
Hi Marek & All,
ma 9. tammik. 2023 klo 17.23 Neil Armstrong
(neil.armstr...@linaro.org) kirjoitti:
>
> On 09/01/2023 14:46, Marek Szyprowski wrote:
> > Hi Matti,
> >
> > On 30.11.2022 10:23, Matti Vaittinen wrote:
> >> Simplify using the devm_regulator_get_enable_optional(). Also drop the
> >> now unused struct member 'hdmi_supply'.
> >>
> >> Signed-off-by: Matti Vaittinen
> >> Martin Blumenstingl
> >> ---
> >> v4 resend 2:
> >> Respinning unchanged code with the commit title changed as wa suggested
> >> by Robert Foss and commit message changed as was suggested by Martin.
> >>
> >> I am doing a clean-up for my local git and encountered this one.
> >> Respinning as it seems this one fell through the cracks.
> >> ---
> >>drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
> >>1 file changed, 3 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> >> b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> >> index 5cd2b2ebbbd3..7642f740272b 100644
> >> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> >> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> >> @@ -140,7 +140,6 @@ struct meson_dw_hdmi {
> >> struct reset_control *hdmitx_apb;
> >> struct reset_control *hdmitx_ctrl;
> >> struct reset_control *hdmitx_phy;
> >> -struct regulator *hdmi_supply;
> >> u32 irq_stat;
> >> struct dw_hdmi *hdmi;
> >> struct drm_bridge *bridge;
> >> @@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
> >> *meson_dw_hdmi)
> >>
> >>}
> >>
> >> -static void meson_disable_regulator(void *data)
> >> -{
> >> -regulator_disable(data);
> >> -}
> >> -
> >>static void meson_disable_clk(void *data)
> >>{
> >> clk_disable_unprepare(data);
> >> @@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev,
> >> struct device *master,
> >> meson_dw_hdmi->data = match;
> >> dw_plat_data = _dw_hdmi->dw_plat_data;
> >>
> >> -meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
> >> -if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
> >> -if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
> >> -return -EPROBE_DEFER;
> >> -meson_dw_hdmi->hdmi_supply = NULL;
> >> -} else {
> >> -ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
> >> -if (ret)
> >> -return ret;
> >> -ret = devm_add_action_or_reset(dev, meson_disable_regulator,
> >> - meson_dw_hdmi->hdmi_supply);
> >> -if (ret)
> >> -return ret;
> >> -}
> >> +ret = devm_regulator_get_enable_optional(dev, "hdmi");
> >> +if (ret != -ENODEV)
> >
> > The above line should be "if (ret < 0)", otherwise it breaks hdmi support.
> >
Sorry for the trouble and thanks for pointing this out! I believe the
condition I was looking for was
if (ret && ret != -ENODEV).
At least in my eyes the use of regulator_get_optional() sounds like
the -ENODEV should not be treated as an error. The obvious mistake is
regarding return value 0 as such. Thanks for spotting this!
> > I've noticed this once this change has been merged to linux-next and all
> > my Amlogic Meson based boards failed to initialize HDMI. Is it possible
> > to fix this in drm tree or do I need to send the incremental fixup?
>
> Nop, please send an incremental fix and I'll apply it asap.
Thanks for cleaning-up the mess I caused Marek! Please, let me know if
you want me to cook the fix.
Yours,
- -Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));
Re: [PATCH RESEND2 v4 2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
On 12/1/22 10:38, Neil Armstrong wrote: On 30/11/2022 10:23, Matti Vaittinen wrote: Simplify using the devm_regulator_get_enable_optional(). Also drop the now unused struct member 'hdmi_supply'. Signed-off-by: Matti Vaittinen Martin Blumenstingl Missing Acked-by, I'll add it while applying. Oh, well spotted. I should've been more careful.. Sorry and thanks for sorting it out! --Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
[PATCH RESEND2 v4 2/2] drm/meson: dw-hdmi: Use devm_regulator_*get_enable*()
Simplify using the devm_regulator_get_enable_optional(). Also drop the
now unused struct member 'hdmi_supply'.
Signed-off-by: Matti Vaittinen
Martin Blumenstingl
---
v4 resend 2:
Respinning unchanged code with the commit title changed as wa suggested
by Robert Foss and commit message changed as was suggested by Martin.
I am doing a clean-up for my local git and encountered this one.
Respinning as it seems this one fell through the cracks.
---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
*meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = _dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
2.38.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
[PATCH RESEND2 v4 1/2] drm/bridge: sii902x: Use devm_regulator_bulk_get_enable()
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
Acked-by: Robert Foss
---
v4 resend 2:
Resending unchanged code with commit title prefix adjusted as suggested
by Robert
I am doing a clean-up for my local git and encountered this one.
Respinning as it seems this one fell through the cracks.
---
drivers/gpu/drm/bridge/sii902x.c | 26 --
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 878fb7d3732b..f6e8b401069b 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -171,7 +171,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = >dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1122,27 +1122,11 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(>mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies),
supplies);
if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0) {
- dev_err_probe(dev, ret, "Failed to enable supplies");
- return ret;
- }
+ return dev_err_probe(dev, ret, "Failed to enable supplies");
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static void sii902x_remove(struct i2c_client *client)
@@ -1152,8 +1136,6 @@ static void sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(>bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
}
static const struct of_device_id sii902x_dt_ids[] = {
--
2.38.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
[PATCH RESEND2 v4 0/2] Use devm helpers for regulator get and enable
Simplify couple of drivers by using the new devm_regulator_*get_enable*() These patches were previously part of the series: https://lore.kernel.org/lkml/cover.1660934107.git.mazziesacco...@gmail.com/ "Devm helpers for regulator get and enable". I did keep the patch series versioning even though I changed the series name (subject of this mail) to "Use devm helpers for regulator get and enable". Name was changed because the devm helpers are already in 6.1-rc1. Also, most of the patches in the series are already merged to subsystem trees so this series now contains only the patches that have not yet been merged. I hope they can be now directly taken sirectly into respective subsystem trees as the dependencies should be in v6.1-rc1. Please note that these changes are only compile-tested as I don't have the HW to do proper testing. Thus, reviewing / testing is highly appreciated. Revision history: v4: RESEND2: - resend code unchanged. Commit titles and and commit messages improved. v4: RESEND: - resend only the drm patches - others have been applied v3 => v4: - Drop applied patches - rewrite cover-letter - rebase on v6.1-rc1 - split meson and sii902x into own patches as requested. - slightly modify dev_err_probe() return in sii902x. --- Matti Vaittinen (2): drm/bridge: sii902x: Use devm_regulator_bulk_get_enable() drm/meson: dw-hdmi: Use devm_regulator_*get_enable*() drivers/gpu/drm/bridge/sii902x.c | 26 -- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++ 2 files changed, 7 insertions(+), 42 deletions(-) base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa -- 2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =] signature.asc Description: PGP signature
[PATCH RESEND v4 2/2] gpu: drm: meson: Use devm_regulator_*get_enable*()
Simplify using the devm_regulator_get_enable_optional(). Also drop the
seemingly unused struct member 'hdmi_supply'.
Signed-off-by: Matti Vaittinen
---
I am doing a clean-up for my local git and encountered this one.
Respinning as it seems this one fell through the cracks.
---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
*meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = _dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
2.38.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
[PATCH RESEND v4 1/2] gpu: drm: sii902x: Use devm_regulator_bulk_get_enable()
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
Acked-by: Robert Foss
---
I am doing a clean-up for my local git and encountered this one.
Respinning as it seems this one fell through the cracks.
---
drivers/gpu/drm/bridge/sii902x.c | 26 --
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 878fb7d3732b..f6e8b401069b 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -171,7 +171,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = >dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1122,27 +1122,11 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(>mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies),
supplies);
if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0) {
- dev_err_probe(dev, ret, "Failed to enable supplies");
- return ret;
- }
+ return dev_err_probe(dev, ret, "Failed to enable supplies");
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static void sii902x_remove(struct i2c_client *client)
@@ -1152,8 +1136,6 @@ static void sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(>bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
}
static const struct of_device_id sii902x_dt_ids[] = {
--
2.38.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
[PATCH RESEND v4 0/2] Use devm helpers for regulator get and enable
Simplify couple of drivers by using the new devm_regulator_*get_enable*() Found these patches when doing some clean-up for my local git. Seems like these two fell through the cracks while other were merged. So, this is a respin of subset of original series v4. These patches were previously part of the series: https://lore.kernel.org/lkml/cover.1660934107.git.mazziesacco...@gmail.com/ "Devm helpers for regulator get and enable". I did keep the patch series versioning even though I changed the series name (subject of this mail) to "Use devm helpers for regulator get and enable". Name was changed because the devm helpers are already in 6.1-rc1. Also, most of the patches in the series are already merged to subsystem trees so this series now contains only the patches that have not yet been merged. I hope they can be now directly taken sirectly into respective subsystem trees as the dependencies should be in v6.1-rc1. Please note that these changes are only compile-tested as I don't have the HW to do proper testing. Thus, reviewing / testing is highly appreciated. Revision history: v3 => v4: - Drop applied patches - rewrite cover-letter - rebase on v6.1-rc1 - split meson and sii902x into own patches as requested. - slightly modify dev_err_probe() return in sii902x. Matti Vaittinen (2): gpu: drm: sii902x: Use devm_regulator_bulk_get_enable() gpu: drm: meson: Use devm_regulator_*get_enable*() drivers/gpu/drm/bridge/sii902x.c | 26 -- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++ 2 files changed, 7 insertions(+), 42 deletions(-) base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa -- 2.38.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =] signature.asc Description: PGP signature
Re: [PATCH v4 2/4] gpu: drm: sii902x: Use devm_regulator_bulk_get_enable()
On 10/21/22 16:18, Matti Vaittinen wrote:
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
Reviewed-by: Robert Foss
Robert, I did slightly modify the return from probe when using the
dev_err_probe(). I still decided to keep your RBT - please let me know
if you disagree.
Eg, this:
- if (ret < 0) {
- dev_err_probe(dev, ret, "Failed to enable supplies");
- return ret;
- }
was converted to:
>if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to enable supplies");
Yours
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
[PATCH v4 4/4] hwmon: adm1177: simplify using devm_regulator_get_enable()
Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(),
add_action_or_reset(regulator_disable)' and use the
devm_regulator_get_enable() and drop the pointer to the regulator.
This simplifies code and makes it less tempting to add manual control
for the regulator which is also controlled by devm.
Signed-off-by: Matti Vaittinen
Acked-by: Guenter Roeck
---
v2 => v3:
New patch
---
drivers/hwmon/adm1177.c | 27 +++
1 file changed, 3 insertions(+), 24 deletions(-)
diff --git a/drivers/hwmon/adm1177.c b/drivers/hwmon/adm1177.c
index 0c5dbc5e33b4..be17a26a84f1 100644
--- a/drivers/hwmon/adm1177.c
+++ b/drivers/hwmon/adm1177.c
@@ -26,14 +26,12 @@
/**
* struct adm1177_state - driver instance specific data
* @client:pointer to i2c client
- * @reg: regulator info for the power supply of the device
* @r_sense_uohm: current sense resistor value
* @alert_threshold_ua:current limit for shutdown
* @vrange_high: internal voltage divider
*/
struct adm1177_state {
struct i2c_client *client;
- struct regulator*reg;
u32 r_sense_uohm;
u32 alert_threshold_ua;
boolvrange_high;
@@ -189,13 +187,6 @@ static const struct hwmon_chip_info adm1177_chip_info = {
.info = adm1177_info,
};
-static void adm1177_remove(void *data)
-{
- struct adm1177_state *st = data;
-
- regulator_disable(st->reg);
-}
-
static int adm1177_probe(struct i2c_client *client)
{
struct device *dev = >dev;
@@ -210,21 +201,9 @@ static int adm1177_probe(struct i2c_client *client)
st->client = client;
- st->reg = devm_regulator_get_optional(>dev, "vref");
- if (IS_ERR(st->reg)) {
- if (PTR_ERR(st->reg) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
-
- st->reg = NULL;
- } else {
- ret = regulator_enable(st->reg);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(>dev, adm1177_remove,
- st);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(>dev, "vref");
+ if (ret == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
if (device_property_read_u32(dev, "shunt-resistor-micro-ohms",
>r_sense_uohm))
--
2.37.3
--
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 =]
signature.asc
Description: PGP signature
[PATCH v4 3/4] hwmon: lm90: simplify using devm_regulator_get_enable()
Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(),
add_action_or_reset(regulator_disable)' and use the
devm_regulator_get_enable().
Signed-off-by: Matti Vaittinen
Acked-by: Guenter Roeck
---
RFCv1 => v2:
- No changes
---
drivers/hwmon/lm90.c | 20 ++--
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index db595f7d01f8..a3f95ba00dbf 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -2663,11 +2663,6 @@ static void lm90_remove_pec(void *dev)
device_remove_file(dev, _attr_pec);
}
-static void lm90_regulator_disable(void *regulator)
-{
- regulator_disable(regulator);
-}
-
static int lm90_probe_channel_from_dt(struct i2c_client *client,
struct device_node *child,
struct lm90_data *data)
@@ -2749,24 +2744,13 @@ static int lm90_probe(struct i2c_client *client)
struct device *dev = >dev;
struct i2c_adapter *adapter = client->adapter;
struct hwmon_channel_info *info;
- struct regulator *regulator;
struct device *hwmon_dev;
struct lm90_data *data;
int err;
- regulator = devm_regulator_get(dev, "vcc");
- if (IS_ERR(regulator))
- return PTR_ERR(regulator);
-
- err = regulator_enable(regulator);
- if (err < 0) {
- dev_err(dev, "Failed to enable regulator: %d\n", err);
- return err;
- }
-
- err = devm_add_action_or_reset(dev, lm90_regulator_disable, regulator);
+ err = devm_regulator_get_enable(dev, "vcc");
if (err)
- return err;
+ return dev_err_probe(dev, err, "Failed to enable regulator\n");
data = devm_kzalloc(dev, sizeof(struct lm90_data), GFP_KERNEL);
if (!data)
--
2.37.3
--
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 =]
signature.asc
Description: PGP signature
[PATCH v4 2/4] gpu: drm: sii902x: Use devm_regulator_bulk_get_enable()
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
Reviewed-by: Robert Foss
---
v3 => v4:
- split to own patch.
- return directly the value returned by the dev_err_probe()
Please note - this is only compile-tested due to the lack of HW. Careful
review and testing is _highly_ appreciated
---
drivers/gpu/drm/bridge/sii902x.c | 26 --
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 878fb7d3732b..f6e8b401069b 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -171,7 +171,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = >dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1122,27 +1122,11 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(>mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies),
supplies);
if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0) {
- dev_err_probe(dev, ret, "Failed to enable supplies");
- return ret;
- }
+ return dev_err_probe(dev, ret, "Failed to enable supplies");
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static void sii902x_remove(struct i2c_client *client)
@@ -1152,8 +1136,6 @@ static void sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(>bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
}
static const struct of_device_id sii902x_dt_ids[] = {
--
2.37.3
--
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 =]
signature.asc
Description: PGP signature
[PATCH v4 1/4] gpu: drm: meson: Use devm_regulator_*get_enable*()
Simplify using the devm_regulator_get_enable_optional(). Also drop the
seemingly unused struct member 'hdmi_supply'.
Signed-off-by: Matti Vaittinen
---
v3 => v4:
- split meson part to own patch
RFCv1 => v2:
- Change also sii902x to use devm_regulator_bulk_get_enable()
Please note - this is only compile-tested due to the lack of HW. Careful
review and testing is _highly_ appreciated.
---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
*meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = _dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
2.37.3
--
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 =]
signature.asc
Description: PGP signature
[PATCH v4 0/4] Use devm helpers for regulator get and enable
Simplify couple of drivers by using the new devm_regulator_*get_enable*() These patches were previously part of the series: https://lore.kernel.org/lkml/cover.1660934107.git.mazziesacco...@gmail.com/ "Devm helpers for regulator get and enable". I did keep the patch series versioning even though I changed the series name (subject of this mail) to "Use devm helpers for regulator get and enable". Name was changed because the devm helpers are already in 6.1-rc1. Also, most of the patches in the series are already merged to subsystem trees so this series now contains only the patches that have not yet been merged. I hope they can be now directly taken sirectly into respective subsystem trees as the dependencies should be in v6.1-rc1. Please note that these changes are only compile-tested as I don't have the HW to do proper testing. Thus, reviewing / testing is highly appreciated. Revision history: v3 => v4: - Drop applied patches - rewrite cover-letter - rebase on v6.1-rc1 - split meson and sii902x into own patches as requested. - slightly modify dev_err_probe() return in sii902x. --- Matti Vaittinen (4): gpu: drm: meson: Use devm_regulator_*get_enable*() gpu: drm: sii902x: Use devm_regulator_bulk_get_enable() hwmon: lm90: simplify using devm_regulator_get_enable() hwmon: adm1177: simplify using devm_regulator_get_enable() drivers/gpu/drm/bridge/sii902x.c | 26 -- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++ drivers/hwmon/adm1177.c | 27 +++ drivers/hwmon/lm90.c | 20 ++-- 4 files changed, 12 insertions(+), 84 deletions(-) base-commit: 9abf2313adc1ca1b6180c508c25f22f9395cc780 -- 2.37.3 -- 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 =] signature.asc Description: PGP signature
Re: [PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod_get()
On 9/5/22 13:40, Andy Shevchenko wrote: On Mon, Sep 5, 2022 at 9:33 AM Dmitry Torokhov wrote: I would like to stop exporting OF-specific devm_gpiod_get_from_of_node() so that gpiolib can be cleaned a bit, so let's switch to the generic fwnode property API. While at it switch the rest of the calls to read properties in bd957x_probe() to the generic device property API as well. With or without below addressed, Reviewed-by: Andy Shevchenko Signed-off-by: Dmitry Torokhov diff --git a/drivers/regulator/bd9576-regulator.c b/drivers/regulator/bd9576-regulator.c index aa42da4d141e..393c8693b327 100644 --- a/drivers/regulator/bd9576-regulator.c +++ b/drivers/regulator/bd9576-regulator.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -939,8 +940,8 @@ static int bd957x_probe(struct platform_device *pdev) } ic_data->regmap = regmap; - vout_mode = of_property_read_bool(pdev->dev.parent->of_node, -"rohm,vout1-en-low"); + vout_mode = device_property_read_bool(pdev->dev.parent, + "rohm,vout1-en-low"); They all using parent device and you may make code neater by adding struct device *parent = pdev->dev.parent; This is a matter of personal preference. I prefer seeing pdev->dev.parent - as it is more obvious (to me) what the 'pdev' is than what 'parent' would be. I'd use the local variable only when it shortens at least one of the lines so that we avoid splitting it. After that being said - I'm not going to argue over this change either if one who is improving the driver wants to use the "helper" variable here. Best Regards -- Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod_get()
On 9/5/22 09:31, Dmitry Torokhov wrote: I would like to stop exporting OF-specific devm_gpiod_get_from_of_node() so that gpiolib can be cleaned a bit, so let's switch to the generic fwnode property API. While at it switch the rest of the calls to read properties in bd957x_probe() to the generic device property API as well. Signed-off-by: Dmitry Torokhov Reviewed-by: Matti Vaittinen Thanks! -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCH v1 08/11] regulator: bd71815: switch to using devm_fwnode_gpiod_get()
On 9/5/22 09:31, Dmitry Torokhov wrote:
I would like to stop exporting OF-specific devm_gpiod_get_from_of_node()
so that gpiolib can be cleaned a bit, so let's switch to the generic
fwnode property API.
Signed-off-by: Dmitry Torokhov
Reviewed-by: Matti Vaittinen
diff --git a/drivers/regulator/bd71815-regulator.c
b/drivers/regulator/bd71815-regulator.c
index acaa6607898e..c2b8b8be7824 100644
--- a/drivers/regulator/bd71815-regulator.c
+++ b/drivers/regulator/bd71815-regulator.c
@@ -571,11 +571,10 @@ static int bd7181x_probe(struct platform_device *pdev)
dev_err(>dev, "No parent regmap\n");
return -ENODEV;
}
- ldo4_en = devm_gpiod_get_from_of_node(>dev,
- pdev->dev.parent->of_node,
-"rohm,vsel-gpios", 0,
-GPIOD_ASIS, "ldo4-en");
+ ldo4_en = devm_fwnode_gpiod_get(>dev,
+ dev_fwnode(pdev->dev.parent),
+ "rohm,vsel", GPIOD_ASIS, "ldo4-en");
if (IS_ERR(ldo4_en)) {
ret = PTR_ERR(ldo4_en);
if (ret != -ENOENT)
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCH v9 07/10] power: supply: mt6370: Add MediaTek MT6370 charger driver
On 8/30/22 06:40, ChiaEn Wu wrote:
From: ChiaEn Wu
MediaTek MT6370 is a SubPMIC consisting of a single cell battery charger
with ADC monitoring, RGB LEDs, dual channel flashlight, WLED backlight
driver, display bias voltage supply, one general purpose LDO, and the
USB Type-C & PD controller complies with the latest USB Type-C and PD
standards.
Add support for the MediaTek MT6370 Charger driver. The charger module
of MT6370 supports High-Accuracy Voltage/Current Regulation,
Average Input Current Regulation, Battery Temperature Sensing,
Over-Temperature Protection, DPDM Detection for BC1.2.
Reviewed-by: Andy Shevchenko
Signed-off-by: ChiaEn Wu
---
+
+static const struct linear_range mt6370_chg_ranges[MT6370_RANGE_F_MAX] = {
+ LINEAR_RANGE_IDX(MT6370_RANGE_F_IAICR, 10, 0x0, 0x3F, 5),
+ LINEAR_RANGE_IDX(MT6370_RANGE_F_VOREG, 390, 0x0, 0x51, 1),
+ LINEAR_RANGE_IDX(MT6370_RANGE_F_VMIVR, 390, 0x0, 0x5F, 10),
+ LINEAR_RANGE_IDX(MT6370_RANGE_F_ICHG, 90, 0x08, 0x31, 10),
+ LINEAR_RANGE_IDX(MT6370_RANGE_F_IPREC, 10, 0x0, 0x0F, 5),
+ LINEAR_RANGE_IDX(MT6370_RANGE_F_IEOC, 10, 0x0, 0x0F, 5),
+};
This looks good to me now :) Thanks for the linear-range improvement!
+ INIT_DELAYED_WORK(>mivr_dwork, mt6370_chg_mivr_dwork_func);
+ ret = devm_add_action_or_reset(dev, mt6370_chg_cancel_mivr_dwork,
+ >mivr_dwork);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to init mivr dwork\n");
I just noticed this. Maybe this could be done using
devm_delayed_work_autocancel() ?
Yours
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCH v9 05/10] lib: add linear range index macro
On 8/30/22 06:40, ChiaEn Wu wrote:
From: ChiaEn Wu
Add linear_range_idx macro for declaring the linear_range struct simply.
Signed-off-by: ChiaEn Wu
Reviewed-by: Matti Vaittinen
---
v9
- Revise LINEAR_RANGE() and LINEAR_RANGE_IDX()
---
include/linux/linear_range.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/include/linux/linear_range.h b/include/linux/linear_range.h
index fd3d0b3..2e4f4c3 100644
--- a/include/linux/linear_range.h
+++ b/include/linux/linear_range.h
@@ -26,6 +26,17 @@ struct linear_range {
unsigned int step;
};
+#define LINEAR_RANGE(_min, _min_sel, _max_sel, _step) \
+ { \
+ .min = _min,\
+ .min_sel = _min_sel,\
+ .max_sel = _max_sel,\
+ .step = _step, \
+ }
+
+#define LINEAR_RANGE_IDX(_idx, _min, _min_sel, _max_sel, _step)\
+ [_idx] = LINEAR_RANGE(_min, _min_sel, _max_sel, _step)
+
unsigned int linear_range_values_in_range(const struct linear_range *r);
unsigned int linear_range_values_in_range_array(const struct linear_range *r,
int ranges);
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCH v3 03/14] gpu: drm: simplify drivers using devm_regulator_*get_enable*()
On 8/29/22 17:25, Robert Foss wrote:
Thanks for the review Robert.
Hi Matti,
On Fri, 19 Aug 2022 at 21:18, Matti Vaittinen wrote:
Simplify drivers using managed "regulator get and enable".
meson:
Use the devm_regulator_get_enable_optional(). Also drop the seemingly
unused struct member 'hdmi_supply'.
sii902x:
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
---
v2 => v3:
No changes
RFCv1 => v2:
- Change also sii902x to use devm_regulator_bulk_get_enable()
Please note - this is only compile-tested due to the lack of HW. Careful
review and testing is _highly_ appreciated.
---
drivers/gpu/drm/bridge/sii902x.c | 22 +++---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
2 files changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 7ab38d734ad6..162f9c87eeb2 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -171,7 +171,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = >dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1122,27 +1122,13 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(>mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies),
supplies);
if (ret < 0) {
dev_err_probe(dev, ret, "Failed to enable supplies");
return ret;
}
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static int sii902x_remove(struct i2c_client *client)
@@ -1152,8 +1138,6 @@ static int sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(>bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
return 0;
}
Ideally this patch would be split into two parts here, due to
maintainership boundaries.
Ok. I will in any case respin this series when the dependency patches
from Mark's tree have been merged to the -rc1. I can split this to
patch/driver if it is preferred. I just though I'll decrease amount of
mails by squashing these almost trivial changes.
For the sii902x part, please add my r-b.
Reviewed-by: Robert Foss
Thanks.
Best Regards
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));
[PATCH v3 03/14] gpu: drm: simplify drivers using devm_regulator_*get_enable*()
Simplify drivers using managed "regulator get and enable".
meson:
Use the devm_regulator_get_enable_optional(). Also drop the seemingly
unused struct member 'hdmi_supply'.
sii902x:
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
---
v2 => v3:
No changes
RFCv1 => v2:
- Change also sii902x to use devm_regulator_bulk_get_enable()
Please note - this is only compile-tested due to the lack of HW. Careful
review and testing is _highly_ appreciated.
---
drivers/gpu/drm/bridge/sii902x.c | 22 +++---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
2 files changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 7ab38d734ad6..162f9c87eeb2 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -171,7 +171,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1072,6 +1071,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = >dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1122,27 +1122,13 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(>mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies),
supplies);
if (ret < 0) {
dev_err_probe(dev, ret, "Failed to enable supplies");
return ret;
}
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static int sii902x_remove(struct i2c_client *client)
@@ -1152,8 +1138,6 @@ static int sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(>bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
return 0;
}
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
*meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = _dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
2.37.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland
[PATCH v3 00/14] Use devm helpers for regulator get and enable
Use devm helpers for regulator get and enable
NOTE: The series depends on commit
ee94aff2628b ("Devm helpers for regulator get and enable")
which currently sits in Mark's regulator/for-next
A few* drivers seem to pattern demonstrated by pseudocode:
- devm_regulator_get()
- regulator_enable()
- devm_add_action_or_reset(regulator_disable())
devm helpers for this pattern were added to remove bunch of code from
drivers. Typically following:
- replace 3 calls (devm_regulator_get[_optional](), regulator_enable(),
devm_add_action_or_reset()) with just one
(devm_regulator_get_enable[_optional]()).
- drop disable callback.
I believe this simplifies things by removing some dublicated code.
This series reowrks a few drivers. There is still plenty of fish in the
sea for people who like to improve the code (or count the beans ;]).
Finally - most of the converted drivers have not been tested (other than
compile-tested) due to lack of HW. All reviews and testing is _highly_
appreciated (as always!).
Revision history:
v3:
- Drop already applied helper patches
- Add a few more drivers
RFCv1 => v2:
- Add devm_regulator_bulk_get_enable() and
devm_regulator_bulk_put()
- Convert a couple of drivers to use the new
devm_regulator_bulk_get_enable().
- Squash all IIO patches into one.
Patch 1:
Add new devm-helper APIs to docs.
Patch 2:
simplified CLK driver(s)
Patch 3:
simplified GPU driver(s)
Patch 4 - 5:
simplified hwmon driver(s)
Patch 6 - 14:
simplified IIO driver(s)
---
Matti Vaittinen (14):
docs: devres: regulator: Add new get_enable functions to devres.rst
clk: cdce925: simplify using devm_regulator_get_enable()
gpu: drm: simplify drivers using devm_regulator_*get_enable*()
hwmon: lm90: simplify using devm_regulator_get_enable()
hwmon: adm1177: simplify using devm_regulator_get_enable()
iio: ad7192: Simplify using devm_regulator_get_enable()
iio: ltc2688: Simplify using devm_regulator_*get_enable()
iio: bmg160_core: Simplify using devm_regulator_*get_enable()
iio: st_lsm6dsx: Simplify using devm_regulator_*get_enable()
iio: ad7476: simplify using devm_regulator_get_enable()
iio: ad7606: simplify using devm_regulator_get_enable()
iio: max1241: simplify using devm_regulator_get_enable()
iio: max1363: simplify using devm_regulator_get_enable()
iio: hmc425a: simplify using devm_regulator_get_enable()
.../driver-api/driver-model/devres.rst| 4 +++
drivers/clk/clk-cdce925.c | 21 +++--
drivers/gpu/drm/bridge/sii902x.c | 22 ++
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 ++
drivers/hwmon/adm1177.c | 27 ++---
drivers/hwmon/lm90.c | 15 ++
drivers/iio/adc/ad7192.c | 15 ++
drivers/iio/adc/ad7476.c | 11 +--
drivers/iio/adc/ad7606.c | 22 ++
drivers/iio/adc/ad7606.h | 1 -
drivers/iio/adc/max1241.c | 28 ++---
drivers/iio/adc/max1363.c | 11 +--
drivers/iio/amplifiers/hmc425a.c | 17 +--
drivers/iio/dac/ltc2688.c | 23 ++
drivers/iio/gyro/bmg160_core.c| 24 ++-
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 --
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 30 ---
17 files changed, 41 insertions(+), 255 deletions(-)
--
2.37.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
Re: (subset) [PATCH v2 0/7] Devm helpers for regulator get and enable
Hi Mark, On 8/15/22 18:44, Mark Brown wrote: On Fri, 12 Aug 2022 13:08:17 +0300, Matti Vaittinen wrote: Devm helpers for regulator get and enable First patch in the series is actually just a simple documentation fix which could be taken in as it is now. A few* drivers seem to use pattern demonstrated by pseudocode: [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next Thanks! [1/7] docs: devres: regulator: Add missing devm_* functions to devres.rst commit: 9b6744f60b6b47bc0757a1955adb4d2c3ab22e13 [2/7] regulator: Add devm helpers for get and enable (no commit info) I was planning to send out the v3 (where IIO patches are no longer squashed into one). I didn't spot the above mentioned patch 2/7 from regulator/for-next. I'd just like to get confirmation the 2/7 was not merged even though it's mentioned in this mail before re-spinning the series with it. Best Regards --Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Re: (subset) [PATCH v2 0/7] Devm helpers for regulator get and enable
Hi dee Ho Mark, Laurent, Stephen, all
On 8/16/22 01:55, Mark Brown wrote:
On Tue, Aug 16, 2022 at 12:17:17AM +0300, Laurent Pinchart wrote:
On Mon, Aug 15, 2022 at 01:58:55PM -0700, Stephen Boyd wrote:
You will very quickly see drivers doing this (either directly or
indirectly):
probe()
{
devm_clk_get_enabled();
devm_regulator_get_enable();
}
Without a devres-based get+enable API drivers can get the resources they
need in any order, possibly moving some of those resource acquisition
operations to different functions, and then have a clear block of code
that enables the resources in the right order.
I agree. And I think that drivers which do that should stick with it.
Still, as you know the devm-unwinding is also done in well defined
order. I believe that instead of fighting against the devm we should try
educate people to pay attention in the order of unwinding (also when not
handled by the devm. Driver writers occasionally break things also w/o
devm for example by freeing resources needed by IRQ handlers prior
freeing the IRQ.)
If "purging" must not be done in reverse order compared to the
aquisition - then one should not use devm. I know people have done
errors with devm - OTOH, I've seen devm also fixing bunch of errors.
These devres helpers give
a false sense of security to driver authors and they will end up
introducing problems, the same way that devm_kzalloc() makes it
outrageously easy to crash the kernel by disconnecting a device that is
in use.
I think this is going a bit "off-topic" but I'd like to understand what
is behind this statement? From device-writer's perspective - I don't
know much better alternatives to free up the memory. I don't see how
freeing stuff at .remove would be any better? As far as I understand -
if someone is using driver's resources after the device has gone and the
driver is detached, then there is not much the driver could do to
free-up the stuff be it devm or not? This sounds like fundamentally
different problem (to me).
TBH I think the problem you have here is with devm not with this
particular function.
I must say I kind of agree with Mark. If we stop for a second to think
what would the Laurent's example look like if there were no
devm_regulator_get_enable() provided. I bet the poor driver author could
have used devm_clk_get_enabled() - and then implemented a .remove for
disabling the regulator. That would be even worse, right?
That's a different conversation, and a totally
valid one especially when you start looking at things like implementing
userspace APIs which need to cope with hardware going away while still
visible to userspace.
This is interesting. It's not easy for me to spot how devm changes
things here? If we consider some removable device - then I guess also
the .remove() is ran only after HW has already gone? Yes, devm might
make the time window when userspace can see hardware that has gone
longer but does it bring any new problem there? It seems to me devm can
make hitting the spot more likely - but I don't think it brings
completely new issues? (Well, I may be wrong here - wouldn't be the
first time :])
It's *probably* more of a subsystem conversation
than a driver one though, or at least I think subsystems should try to
arrange to make it so.
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));
Re: [RESEND PATCH v8 06/12] lib: add linear range index macro
Hi ChiaEn,
On 8/15/22 12:01, ChiaEn Wu wrote:
From: ChiaEn Wu
Add linear_range_idx macro for declaring the linear_range struct simply.
Signed-off-by: ChiaEn Wu
---
include/linux/linear_range.h | 8
1 file changed, 8 insertions(+)
diff --git a/include/linux/linear_range.h b/include/linux/linear_range.h
index fd3d0b358f22..fb53ea13c593 100644
--- a/include/linux/linear_range.h
+++ b/include/linux/linear_range.h
@@ -26,6 +26,14 @@ struct linear_range {
unsigned int step;
};
+#define LINEAR_RANGE_IDX(_min, _min_sel, _max_sel, _step) \
+ { \
+ .min = _min,\
+ .min_sel = _min_sel,\
+ .max_sel = _max_sel,\
+ .step = _step, \
+ }
+
I think this somewhat differs from what you had originally scetched. Eg,
if I didn't misread the patch earlier - you had:
#define MT6370_CHG_LINEAR_RANGE(_rfd, _min, _min_sel, _max_sel, _step) \
[_rfd] = { \
...
instead of the
> +#define LINEAR_RANGE_IDX(_min, _min_sel, _max_sel, _step) \
> + { \
I think the latter (without the []-index) is more generic, and very
welcome. However, the IDX-suffix does no longer make much sense, right?
I suggested name LINEAR_RANGE_IDX for macro taking the array index as it
would also be useful when dealing with arrays.
Do you think you could still drop the IDX from macro name or keep the
array index as the original did?
Maybe ideally introduce both macros (unless Mark has objections), one
with the [_rfd] and suffix IDX, and the other w/o the suffix and w/o the
[_rfd]?
Thanks for the improvements and the patience! ;)
Yours
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCH v7 10/13] power: supply: mt6370: Add MediaTek MT6370 charger driver
Hi ChiaEn,
pe 5. elok. 2022 klo 10.09 ChiaEn Wu (peterwu@gmail.com) kirjoitti:
>
> From: ChiaEn Wu
>
> MediaTek MT6370 is a SubPMIC consisting of a single cell battery charger
> with ADC monitoring, RGB LEDs, dual channel flashlight, WLED backlight
> driver, display bias voltage supply, one general purpose LDO, and the
> USB Type-C & PD controller complies with the latest USB Type-C and PD
> standards.
>
> Add a support for the MediaTek MT6370 Charger driver. The charger module
> of MT6370 supports High-Accuracy Voltage/Current Regulation,
> Average Input Current Regulation, Battery Temperature Sensing,
> Over-Temperature Protection, DPDM Detection for BC1.2.
>
> Signed-off-by: ChiaEn Wu
> ---
>
> +
> +#define MT6370_CHG_LINEAR_RANGE(_rfd, _min, _min_sel, _max_sel, _step) \
> +[_rfd] = { \
> + .min = _min,\
> + .min_sel = _min_sel,\
> + .max_sel = _max_sel,\
> + .step = _step, \
> +}
Just a minor thing but I think this macro could be useful also for
other drivers. Do you think you could rename it to LINEAR_RANGE_IDX()
(or some such) and move it to the linear_range.h? That would allow
also other drivers to use it instead of reinventing the wheel :)
Best Regards
-- Matti Vaittinen
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
[PATCH v2 5/7] gpu: drm: simplify drivers using devm_regulator_*get_enable*()
Simplify drivers using managed "regulator get and enable".
meson:
Use the devm_regulator_get_enable_optional(). Also drop the seemingly
unused struct member 'hdmi_supply'.
sii902x:
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen
---
RFCv1 => v2:
- Change also sii902x to use devm_regulator_bulk_get_enable()
Please note - this is only compile-tested due to the lack of HW. Careful
review and testing is _highly_ appreciated.
---
drivers/gpu/drm/bridge/sii902x.c | 22 +++---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
2 files changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 65549fbfdc87..4bf572b7ca77 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -170,7 +170,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1070,6 +1069,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = >dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1120,27 +1120,13 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(>mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies),
supplies);
if (ret < 0) {
dev_err_probe(dev, ret, "Failed to enable supplies");
return ret;
}
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static int sii902x_remove(struct i2c_client *client)
@@ -1150,8 +1136,6 @@ static int sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(>bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
return 0;
}
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
*meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = _dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
2.37.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenk
[PATCH v2 0/7] Devm helpers for regulator get and enable
Devm helpers for regulator get and enable First patch in the series is actually just a simple documentation fix which could be taken in as it is now. A few* drivers seem to use pattern demonstrated by pseudocode: - devm_regulator_get() - regulator_enable() - devm_add_action_or_reset(regulator_disable()) Introducing devm helpers for this pattern would remove bunch of code from drivers. Typically following: - replace 3 calls (devm_regulator_get[_optional](), regulator_enable(), devm_add_action_or_reset()) with just one (devm_regulator_get_enable[_optional]()). - drop disable callback. - remove stored pointer to struct regulator - which can lead to problem when an devm action for regulator_disable is used. I believe this simplifies things by removing some dublicated code. The suggested managed 'get_enable' APIs do not return the pointer to regulators for user because any call to regulator_disable() (or regulator_enable()) may easily lead to regulator enable count imbalance upon device detach. (Eg, if someone calls regulator_disable() and the device is then detached before user has re-enabled the regulator). Not returning the pointer to obtained regulator to caller is a good hint that the enable/disable should not be manually handled when these APIs are used. OTOH, not returning the pointer reduces the use-cases by not allowing the consumers to perform other regulator actions. For example request the voltages. A few drivers which used the "get, enable, devm_action_to_disable" did also query the voltages. The API does not suit needs of such users. This series reowrks only a few drivers as I am short of time. So, there is still plenty of fish in the sea for people who like to improve the code (or count the beans ;]). Finally - most of the converted drivers have not been tested (other than compile-tested) due to lack of HW. All reviews and testing is _highly_ appreciated (as always!). Revision history: RFCv1 => v2: - Add devm_regulator_bulk_get_enable() and devm_regulator_bulk_put() - Convert a couple of drivers to use the new devm_regulator_bulk_get_enable(). - Squash all IIO patches into one. Patch 1: Fix docmentation (devres API list) for regulator APIs Patch 2: The new devm helpers. Patch 3: Add new devm-helper APIs to docs. Patch 4: simplified CLK driver(s) Patch 5: simplified GPU driver(s) Patch 6: simplified hwmon driver(s) Patch 7: simplified IIO driver(s) --- Matti Vaittinen (7): docs: devres: regulator: Add missing devm_* functions to devres.rst regulator: Add devm helpers for get and enable docs: devres: regulator: Add new get_enable functions to devres.rst clk: cdce925: simplify using devm_regulator_get_enable() gpu: drm: simplify drivers using devm_regulator_*get_enable*() hwmon: lm90: simplify using devm_regulator_get_enable() iio: Simplify drivers using devm_regulator_*get_enable() .../driver-api/driver-model/devres.rst| 11 ++ drivers/clk/clk-cdce925.c | 21 +-- drivers/gpu/drm/bridge/sii902x.c | 22 +-- drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +-- drivers/hwmon/lm90.c | 21 +-- drivers/iio/adc/ad7192.c | 15 +- drivers/iio/dac/ltc2688.c | 23 +-- drivers/iio/gyro/bmg160_core.c| 24 +-- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 - drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 30 +--- drivers/regulator/devres.c| 164 ++ include/linux/regulator/consumer.h| 27 +++ 12 files changed, 227 insertions(+), 156 deletions(-) -- 2.37.1 -- Matti Vaittinen, Linux device drivers ROHM Semiconductors, Finland SWDC Kiviharjunlenkki 1E 90220 OULU FINLAND ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ Simon says - in Latin please. ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ Thanks to Simon Glass for the translation =] signature.asc Description: PGP signature
[RFC PATCH 5/7] gpu: drm: meson: simplify using devm_regulator_get_enable_optional()
Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(),
add_action_or_reset(regulator_disable)' and use the
devm_regulator_get_enable_optional(). Also drop the seemingly unused
struct member 'hdmi_supply'.
Signed-off-by: Matti Vaittinen
---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi
*meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct
device *master,
meson_dw_hdmi->data = match;
dw_plat_data = _dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
2.37.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
[RFC PATCH 0/7] Devm helpers for regulator get and enable
Devm helpers for regulator get and enable
First patch in the series is actually just a simple documentation fix
which could be taken in as it is now (even though the rest of the series
is a RFC).
A few* drivers seem to pattern demonstrated by pseudocode:
- devm_regulator_get()
- regulator_enable()
- devm_add_action_or_reset(regulator_disable())
(*) A rough idea what 'a few' means in this context can be get by issuing:
"git grep -In -A10 devm_regulator_get |grep -B5 -A5 add_action |less"
and then further checking some of the reported drivers. This is what I did
when I realized I needed to enable a regulator for accelerometer and
thought I'd go with devm-action...
Introducing devm helpers for this pattern would remove bunch of code from
drivers. Typically at least following:
- replace 3 calls (devm_regulator_get[_optional](), regulator_enable(),
devm_add_action_or_reset()) with just one
(devm_regulator_get_enable[_optional]()).
- drop disable callback.
I believe this simplifies things by removing some dublicated code.
The other RFC aspect besides the question if this actually is useful, is
whether the devm_regulator_get_enable[_optional]() should return a pointer
to the obtained regulator or not. This RFC version does not return the
pointer for user because any call to regulator_disable() may lead to
regulator enable count imbalance upon device detach. (Eg, if someone calls
regulator_disable() and the device is then detached before user has
re-enabled the regulator). Not returning the pointer to obtained regulator
to caller is a good hint that the enable/disable should not be manually
handled when this API is used.
OTOH, not returning the pointer reduces the use-cases by not allowing
the consumers to perform other regulator actions. For example request the
voltages. A few drivers which used the "get, enable,
devm_action_to_disable" did also query the voltages. The suggested form of
the API does not suit needs of such users. The new API in its current form
really allows to only cover the very dummy cases where regulator is only
enabled for a lifetime of the driver. I am unsure if this is really
beneficial (well, there seems to be bunch of drivers doing just this) - or
if we should go with a version returning the struct regulator *
Some drivers did also manually disable the regulator (even though they had
registered the devm-action for disable) for PM functionality. I am unsure
if such use for suspend is actually safe(?) I didn't check if we can
guarantee that the driver is not detached after the PM suspend has disabled
the regulator(?)
This RFC converts only few a drivers to demonstrate benefits. This makes it
easier to rework the series if people thinks returning the pointer to
struct regulator should be done. I can't promise I'll convert all drivers
so, there is still plenty of fish in the sea for people who like to improve
the code (or count the beans ;]).
Finally - most of the converted drivers have not been tested (other than
compile-tested) due to lack of HW. All reviews and testing is _highly_
appreciated (as always!). I have the driver changes in individual patches
to make reviewing easier. I will squash the driver changes into one patch /
subsystem when I'll drop the "RFC" from the series.
Patch 1:
Fix docmentation (devres API list) for regulator APIs
Patch 2:
The devm helpers.
Patch 3:
Add new devm-helper APIs to docs.
Patches 4 ... 7:
Example drivers.
---
Matti Vaittinen (7):
docs: devres: regulator: Add missing devm_* functions to devres.rst
regulator: Add devm helpers for get and enable
docs: devres: regulator: Add new get_enable functions to devres.rst
clk: cdce925: simplify using devm_regulator_get_enable()
gpu: drm: meson: simplify using devm_regulator_get_enable_optional()
hwmon: lm90: simplify using devm_regulator_get_enable()
adc: ad7192: simplify using devm_regulator_get_enable()
.../driver-api/driver-model/devres.rst| 9 +++
drivers/clk/clk-cdce925.c | 21 ++-
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +---
drivers/hwmon/lm90.c | 21 +--
drivers/iio/adc/ad7192.c | 15 +
drivers/regulator/devres.c| 59 +++
include/linux/regulator/consumer.h| 13
7 files changed, 92 insertions(+), 69 deletions(-)
--
2.37.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
signature.asc
Description: PGP signature
