All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
To: mazziesaccount@gmail.com, matti.vaittinen@fi.rohmeurope.com
Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Lee Jones <lee.jones@linaro.org>,
	linux-power@fi.rohmeurope.com, linux-kernel@vger.kernel.org
Subject: [PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels
Date: Fri, 15 Jan 2021 15:10:40 +0200	[thread overview]
Message-ID: <20210115131040.GA633776@localhost.localdomain> (raw)

The ROHM BD718x7 and BD71828 drivers support setting HW state
specific voltages from device-tree. This is used also by various
in-tree DTS files.

These drivers do incorrectly try to compose bit-map using enum
values. By a chance this works for first two valid levels having
values 1 and 2 - but setting values for the rest of the levels
do indicate capbility of setting values for first levels as
well. Luckily the regulators which support settin values for
SUSPEND/LPSR do usually also support setting values for RUN
and IDLE too - thus this has not been such a fatal issue.

Fix this by defining the old enum values as bits and using
new enum in parsing code. This allows keeping existing IC
specific drivers intact and only adding the defines and
slightly changing the rohm-regulator.c

Fixes: 21b72156ede8b ("regulator: bd718x7: Split driver to common and bd718x7 specific parts")
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/regulator/rohm-regulator.c |  8 ++++----
 include/linux/mfd/rohm-generic.h   | 22 ++++++++++++++++------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/rohm-regulator.c b/drivers/regulator/rohm-regulator.c
index 399002383b28..96caae7dbef4 100644
--- a/drivers/regulator/rohm-regulator.c
+++ b/drivers/regulator/rohm-regulator.c
@@ -55,25 +55,25 @@ int rohm_regulator_set_dvs_levels(const struct rohm_dvs_config *dvs,
 	for (i = 0; i < ROHM_DVS_LEVEL_MAX && !ret; i++) {
 		if (dvs->level_map & (1 << i)) {
 			switch (i + 1) {
-			case ROHM_DVS_LEVEL_RUN:
+			case _ROHM_DVS_LEVEL_RUN:
 				prop = "rohm,dvs-run-voltage";
 				reg = dvs->run_reg;
 				mask = dvs->run_mask;
 				omask = dvs->run_on_mask;
 				break;
-			case ROHM_DVS_LEVEL_IDLE:
+			case _ROHM_DVS_LEVEL_IDLE:
 				prop = "rohm,dvs-idle-voltage";
 				reg = dvs->idle_reg;
 				mask = dvs->idle_mask;
 				omask = dvs->idle_on_mask;
 				break;
-			case ROHM_DVS_LEVEL_SUSPEND:
+			case _ROHM_DVS_LEVEL_SUSPEND:
 				prop = "rohm,dvs-suspend-voltage";
 				reg = dvs->suspend_reg;
 				mask = dvs->suspend_mask;
 				omask = dvs->suspend_on_mask;
 				break;
-			case ROHM_DVS_LEVEL_LPSR:
+			case _ROHM_DVS_LEVEL_LPSR:
 				prop = "rohm,dvs-lpsr-voltage";
 				reg = dvs->lpsr_reg;
 				mask = dvs->lpsr_mask;
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 4283b5b33e04..a557988831d7 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -20,15 +20,25 @@ struct rohm_regmap_dev {
 	struct regmap *regmap;
 };
 
+/*
+ * Do not use these in IC specific driver - the bit map should be created using
+ * defines without the underscore. These should be used only in rohm-regulator.c
+ */
 enum {
-	ROHM_DVS_LEVEL_UNKNOWN,
-	ROHM_DVS_LEVEL_RUN,
-	ROHM_DVS_LEVEL_IDLE,
-	ROHM_DVS_LEVEL_SUSPEND,
-	ROHM_DVS_LEVEL_LPSR,
-	ROHM_DVS_LEVEL_MAX = ROHM_DVS_LEVEL_LPSR,
+	_ROHM_DVS_LEVEL_UNKNOWN,
+	_ROHM_DVS_LEVEL_RUN,
+	_ROHM_DVS_LEVEL_IDLE,
+	_ROHM_DVS_LEVEL_SUSPEND,
+	_ROHM_DVS_LEVEL_LPSR,
+	ROHM_DVS_LEVEL_MAX = _ROHM_DVS_LEVEL_LPSR,
 };
 
+#define ROHM_DVS_LEVEL_UNKNOWN	(1 << _ROHM_DVS_LEVEL_UNKNOWN)
+#define ROHM_DVS_LEVEL_RUN	(1 << _ROHM_DVS_LEVEL_RUN)
+#define ROHM_DVS_LEVEL_IDLE	(1 << _ROHM_DVS_LEVEL_IDLE)
+#define ROHM_DVS_LEVEL_SUSPEND	(1 << _ROHM_DVS_LEVEL_SUSPEND)
+#define ROHM_DVS_LEVEL_LPSR	(1 << _ROHM_DVS_LEVEL_LPSR)
+
 /**
  * struct rohm_dvs_config - dynamic voltage scaling register descriptions
  *

base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
-- 
2.25.4


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

             reply	other threads:[~2021-01-15 13:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15 13:10 Matti Vaittinen [this message]
2021-01-15 13:47 ` [PATCH] regulator: bd718x7, bd71828, Fix dvs voltage levels Lee Jones
2021-01-15 14:10   ` Vaittinen, Matti
  -- strict thread matches above, loose matches on Subject: below --
2021-01-15 14:33 Matti Vaittinen
2021-01-15 14:41 ` Lee Jones
2021-01-15 14:48   ` Vaittinen, Matti

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210115131040.GA633776@localhost.localdomain \
    --to=matti.vaittinen@fi.rohmeurope.com \
    --cc=broonie@kernel.org \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-power@fi.rohmeurope.com \
    --cc=mazziesaccount@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.