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 <[email protected]>
---
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
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 <[email protected]>
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
On Wed, Feb 12, 2020 at 4:13 PM Viresh Kumar <[email protected]> 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 <[email protected]>
>
> 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
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
On Wed, Feb 12, 2020 at 5:04 PM Viresh Kumar <[email protected]> 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