Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] PM / OPP: Add support for multiple regulators
@ 2020-02-12  7:55 Nicolas Boichat
  2020-02-12  8:13 ` Viresh Kumar
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Boichat @ 2020-02-12  7:55 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Nishanth Menon, Stephen Boyd, linux-pm, linux-kernel

The OPP table can contain multiple voltages and regulators, and
implementing that logic does not add a lot of code or complexity,
so let's modify _generic_set_opp_regulator to support that use
case.

Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

---

This is required to add panfrost support for the Bifrost GPU
in MT8183, see this patch:
https://patchwork.kernel.org/patch/11369821/

 drivers/opp/core.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index ba43e6a3dc0aeed..ea4a12a8932014f 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -650,6 +650,24 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
 	return ret;
 }
 
+/*
+ * Set multiple voltages. The caller is responsible for restoring all the
+ * voltages if the function fails.
+ */
+static int _set_opp_voltages(struct device *dev, struct regulator **regs,
+			struct dev_pm_opp_supply *supplies, int count)
+{
+	int i, ret;
+
+	for (i = 0; i < count; i++) {
+		ret = _set_opp_voltage(dev, regs[i], &supplies[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static inline int _generic_set_opp_clk_only(struct device *dev, struct clk *clk,
 					    unsigned long freq)
 {
@@ -671,18 +689,13 @@ static int _generic_set_opp_regulator(const struct opp_table *opp_table,
 				      struct dev_pm_opp_supply *old_supply,
 				      struct dev_pm_opp_supply *new_supply)
 {
-	struct regulator *reg = opp_table->regulators[0];
+	struct regulator **regs = opp_table->regulators;
+	int count = opp_table->regulator_count;
 	int ret;
 
-	/* This function only supports single regulator per device */
-	if (WARN_ON(opp_table->regulator_count > 1)) {
-		dev_err(dev, "multiple regulators are not supported\n");
-		return -EINVAL;
-	}
-
 	/* Scaling up? Scale voltage before frequency */
 	if (freq >= old_freq) {
-		ret = _set_opp_voltage(dev, reg, new_supply);
+		ret = _set_opp_voltages(dev, regs, new_supply, count);
 		if (ret)
 			goto restore_voltage;
 	}
@@ -694,7 +707,7 @@ static int _generic_set_opp_regulator(const struct opp_table *opp_table,
 
 	/* Scaling down? Scale voltage after frequency */
 	if (freq < old_freq) {
-		ret = _set_opp_voltage(dev, reg, new_supply);
+		ret = _set_opp_voltages(dev, regs, new_supply, count);
 		if (ret)
 			goto restore_freq;
 	}
@@ -708,7 +721,7 @@ static int _generic_set_opp_regulator(const struct opp_table *opp_table,
 restore_voltage:
 	/* This shouldn't harm even if the voltages weren't updated earlier */
 	if (old_supply)
-		_set_opp_voltage(dev, reg, old_supply);
+		_set_opp_voltages(dev, regs, old_supply, count);
 
 	return ret;
 }
-- 
2.25.0.225.g125e21ebc7-goog


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

* Re: [PATCH] PM / OPP: Add support for multiple regulators
  2020-02-12  7:55 [PATCH] PM / OPP: Add support for multiple regulators Nicolas Boichat
@ 2020-02-12  8:13 ` Viresh Kumar
  2020-02-12  8:57   ` Nicolas Boichat
  0 siblings, 1 reply; 5+ messages in thread
From: Viresh Kumar @ 2020-02-12  8:13 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, linux-kernel

On 12-02-20, 15:55, Nicolas Boichat wrote:
> The OPP table can contain multiple voltages and regulators, and
> implementing that logic does not add a lot of code or complexity,
> so let's modify _generic_set_opp_regulator to support that use
> case.
> 
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

This is already supported in a different way. See how following driver
does this (hint dev_pm_opp_register_set_opp_helper()).

drivers/opp/ti-opp-supply.c

The problem with a generic solution is that we can't assume an order
for programming of the regulators, as this can be complex on few
platforms.

-- 
viresh

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

* Re: [PATCH] PM / OPP: Add support for multiple regulators
  2020-02-12  8:13 ` Viresh Kumar
@ 2020-02-12  8:57   ` Nicolas Boichat
  2020-02-12  9:03     ` Viresh Kumar
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Boichat @ 2020-02-12  8:57 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, lkml

On Wed, Feb 12, 2020 at 4:13 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 12-02-20, 15:55, Nicolas Boichat wrote:
> > The OPP table can contain multiple voltages and regulators, and
> > implementing that logic does not add a lot of code or complexity,
> > so let's modify _generic_set_opp_regulator to support that use
> > case.
> >
> > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
>
> This is already supported in a different way. See how following driver
> does this (hint dev_pm_opp_register_set_opp_helper()).
>
> drivers/opp/ti-opp-supply.c
>
> The problem with a generic solution is that we can't assume an order
> for programming of the regulators, as this can be complex on few
> platforms.

I see... And you're right that it's probably best to change the
voltages in a specific order (I just ignored that problem ,-P). I do
wonder if there's something we could do in the core/DT to specify that
order (if it's a simple order?), it's not really ideal to have to copy
paste code around...

> --
> viresh

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

* Re: [PATCH] PM / OPP: Add support for multiple regulators
  2020-02-12  8:57   ` Nicolas Boichat
@ 2020-02-12  9:03     ` Viresh Kumar
  2020-02-13  0:06       ` Nicolas Boichat
  0 siblings, 1 reply; 5+ messages in thread
From: Viresh Kumar @ 2020-02-12  9:03 UTC (permalink / raw)
  To: Nicolas Boichat
  Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, lkml

On 12-02-20, 16:57, Nicolas Boichat wrote:
> I see... And you're right that it's probably best to change the
> voltages in a specific order (I just ignored that problem ,-P). I do
> wonder if there's something we could do in the core/DT to specify that
> order (if it's a simple order?), it's not really ideal to have to copy
> paste code around...

I will suggest adding your own version (like TI) for now, if we later
feel that there is too much duplicate code, we can look for an
alternative. But as of now, there aren't a lot of platforms using
multiple regulators anyway.

-- 
viresh

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

* Re: [PATCH] PM / OPP: Add support for multiple regulators
  2020-02-12  9:03     ` Viresh Kumar
@ 2020-02-13  0:06       ` Nicolas Boichat
  0 siblings, 0 replies; 5+ messages in thread
From: Nicolas Boichat @ 2020-02-13  0:06 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, linux-pm, lkml

On Wed, Feb 12, 2020 at 5:04 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 12-02-20, 16:57, Nicolas Boichat wrote:
> > I see... And you're right that it's probably best to change the
> > voltages in a specific order (I just ignored that problem ,-P). I do
> > wonder if there's something we could do in the core/DT to specify that
> > order (if it's a simple order?), it's not really ideal to have to copy
> > paste code around...
>
> I will suggest adding your own version (like TI) for now, if we later
> feel that there is too much duplicate code, we can look for an
> alternative. But as of now, there aren't a lot of platforms using
> multiple regulators anyway.

Will do. Thanks!

> --
> viresh

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-12  7:55 [PATCH] PM / OPP: Add support for multiple regulators Nicolas Boichat
2020-02-12  8:13 ` Viresh Kumar
2020-02-12  8:57   ` Nicolas Boichat
2020-02-12  9:03     ` Viresh Kumar
2020-02-13  0:06       ` Nicolas Boichat

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org
	public-inbox-index linux-pm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git