[PATCH resend] cpufreq: dt: disable unsupported OPPs

[Lucas Stach]

If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Signed-off-by: Lucas Stach <***@pengutronix.de>
---
resend:
- no functional change, split out from the imx5 cpufreq series
v2:
- rebase on top of pm/linux-next
---
drivers/cpufreq/cpufreq-cpu0.c | 66 +++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index a5f8c5f5f4f4..5d73efbd0240 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -185,6 +185,7 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
struct device *cpu_dev;
struct regulator *cpu_reg;
struct clk *cpu_clk;
+ unsigned long min_uV = ~0, max_uV = 0;
unsigned int transition_latency;
int ret;

@@ -204,16 +205,10 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
/* OPPs might be populated at runtime, don't check for error here */
of_init_opp_table(cpu_dev);

- ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
- if (ret) {
- dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret);
- goto out_put_node;
- }
-
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
ret = -ENOMEM;
- goto out_free_table;
+ goto out_put_node;
}

of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance);
@@ -222,30 +217,49 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
transition_latency = CPUFREQ_ETERNAL;

if (!IS_ERR(cpu_reg)) {
- struct dev_pm_opp *opp;
- unsigned long min_uV, max_uV;
- int i;
-
/*
- * OPP is maintained in order of increasing frequency, and
- * freq_table initialised from OPP is therefore sorted in the
- * same order.
+ * Disable any OPPs where the connected regulator isn't able to
+ * provide the specified voltage and record minimum and maximum
+ * voltage levels.
*/
- for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
- ;
- rcu_read_lock();
- opp = dev_pm_opp_find_freq_exact(cpu_dev,
- freq_table[0].frequency * 1000, true);
- min_uV = dev_pm_opp_get_voltage(opp);
- opp = dev_pm_opp_find_freq_exact(cpu_dev,
- freq_table[i-1].frequency * 1000, true);
- max_uV = dev_pm_opp_get_voltage(opp);
- rcu_read_unlock();
+ while (1) {
+ struct dev_pm_opp *opp;
+ unsigned long opp_freq = 0, opp_uV, tol_uV;
+
+ rcu_read_lock();
+ opp = dev_pm_opp_find_freq_ceil(cpu_dev, &opp_freq);
+ if (IS_ERR(opp)) {
+ rcu_read_unlock();
+ break;
+ }
+ opp_uV = dev_pm_opp_get_voltage(opp);
+ rcu_read_unlock();
+
+ tol_uV = opp_uV * priv->voltage_tolerance / 100;
+ if (regulator_is_supported_voltage(cpu_reg, opp_uV,
+ opp_uV + tol_uV)) {
+ if (opp_uV < min_uV)
+ min_uV = opp_uV;
+ if (opp_uV > max_uV)
+ max_uV = opp_uV;
+ } else {
+ dev_pm_opp_disable(cpu_dev, opp_freq);
+ }
+
+ opp_freq++;
+ }
+
ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
if (ret > 0)
transition_latency += ret * 1000;
}

+ ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
+ if (ret) {
+ pr_err("failed to init cpufreq table: %d\n", ret);
+ goto out_free_priv;
+ }
+
/*
* For now, just loading the cooling device;
* thermal DT code takes care of matching them.
@@ -274,9 +288,9 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)

out_cooling_unregister:
cpufreq_cooling_unregister(priv->cdev);
- kfree(priv);
-out_free_table:
dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
+out_free_priv:
+ kfree(priv);
out_put_node:
of_node_put(np);
out_put_reg_clk:

[Rafael J. Wysocki]

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.
---
- no functional change, split out from the imx5 cpufreq series
- rebase on top of pm/linux-next

This makes sense to me, but I need ACKs from people who are more familiar
with OPPs in general.

Post by Lucas Stach
---
drivers/cpufreq/cpufreq-cpu0.c | 66 +++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 26 deletions(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index a5f8c5f5f4f4..5d73efbd0240 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -185,6 +185,7 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
struct device *cpu_dev;
struct regulator *cpu_reg;
struct clk *cpu_clk;
+ unsigned long min_uV = ~0, max_uV = 0;
unsigned int transition_latency;
int ret;
@@ -204,16 +205,10 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
/* OPPs might be populated at runtime, don't check for error here */
of_init_opp_table(cpu_dev);
- ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
- if (ret) {
- dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret);
- goto out_put_node;
- }
-
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
ret = -ENOMEM;
- goto out_free_table;
+ goto out_put_node;
}
of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance);
@@ -222,30 +217,49 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
transition_latency = CPUFREQ_ETERNAL;
if (!IS_ERR(cpu_reg)) {
- struct dev_pm_opp *opp;
- unsigned long min_uV, max_uV;
- int i;
-
/*
- * OPP is maintained in order of increasing frequency, and
- * freq_table initialised from OPP is therefore sorted in the
- * same order.
+ * Disable any OPPs where the connected regulator isn't able to
+ * provide the specified voltage and record minimum and maximum
+ * voltage levels.
*/
- for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
- ;
- rcu_read_lock();
- opp = dev_pm_opp_find_freq_exact(cpu_dev,
- freq_table[0].frequency * 1000, true);
- min_uV = dev_pm_opp_get_voltage(opp);
- opp = dev_pm_opp_find_freq_exact(cpu_dev,
- freq_table[i-1].frequency * 1000, true);
- max_uV = dev_pm_opp_get_voltage(opp);
- rcu_read_unlock();
+ while (1) {
+ struct dev_pm_opp *opp;
+ unsigned long opp_freq = 0, opp_uV, tol_uV;
+
+ rcu_read_lock();
+ opp = dev_pm_opp_find_freq_ceil(cpu_dev, &opp_freq);
+ if (IS_ERR(opp)) {
+ rcu_read_unlock();
+ break;
+ }
+ opp_uV = dev_pm_opp_get_voltage(opp);
+ rcu_read_unlock();
+
+ tol_uV = opp_uV * priv->voltage_tolerance / 100;
+ if (regulator_is_supported_voltage(cpu_reg, opp_uV,
+ opp_uV + tol_uV)) {
+ if (opp_uV < min_uV)
+ min_uV = opp_uV;
+ if (opp_uV > max_uV)
+ max_uV = opp_uV;
+ } else {
+ dev_pm_opp_disable(cpu_dev, opp_freq);
+ }
+
+ opp_freq++;
+ }
+
ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
if (ret > 0)
transition_latency += ret * 1000;
}
+ ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
+ if (ret) {
+ pr_err("failed to init cpufreq table: %d\n", ret);
+ goto out_free_priv;
+ }
+
/*
* For now, just loading the cooling device;
* thermal DT code takes care of matching them.
@@ -274,9 +288,9 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy)
cpufreq_cooling_unregister(priv->cdev);
- kfree(priv);
dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
+ kfree(priv);
of_node_put(np);

[Viresh Kumar]

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.
---
- no functional change, split out from the imx5 cpufreq series
- rebase on top of pm/linux-next

This makes sense to me, but I need ACKs from people who are more familiar
with OPPs in general.

@Rafael: Where is this gone?

http://permalink.gmane.org/gmane.linux.power-management.general/49384

I am quite sure you applied this.. Have you dropped this while playing with
branches ?
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Rafael J. Wysocki]

Post by Viresh Kumar

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.
---
- no functional change, split out from the imx5 cpufreq series
- rebase on top of pm/linux-next

This makes sense to me, but I need ACKs from people who are more familiar
with OPPs in general.

@Rafael: Where is this gone?
http://permalink.gmane.org/gmane.linux.power-management.general/49384
I am quite sure you applied this..

Yes, I did.

Post by Viresh Kumar
Have you dropped this while playing with branches ?

I dropped it temporarily, because I wanted to fold

https://patchwork.kernel.org/patch/4983431/

into it, but then forgot to re-apply it.

Would you mind sending it again with the fix folded in for me?

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Viresh Kumar]

Post by Rafael J. Wysocki
Would you mind sending it again with the fix folded in for me?

Done.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Lucas Stach]

Post by Rafael J. Wysocki
Would you mind sending it again with the fix folded in for me?

Done.

I don't know how it makes sense to fold the fix into this patch. It has
nothing to do with the rename.
If you want to fold the fix in the relevant patch this would be
"cpufreq: cpu0: Move per-cluster initialization code to ->init()"

Regards,
Lucas

[Rafael J. Wysocki]

Post by Lucas Stach

Post by Rafael J. Wysocki
Would you mind sending it again with the fix folded in for me?

Done.

I don't know how it makes sense to fold the fix into this patch. It has
nothing to do with the rename.
If you want to fold the fix in the relevant patch this would be
"cpufreq: cpu0: Move per-cluster initialization code to ->init()"

OK, my confusion, sorry about that.

I'll take the original version of the Viresh's patch and the fix on top
of that separately, then.

That said, it would help if the information about when things broke was there
in the fix' chanelog.

[Rafael J. Wysocki]

Post by Viresh Kumar

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.
---
- no functional change, split out from the imx5 cpufreq series
- rebase on top of pm/linux-next

This makes sense to me, but I need ACKs from people who are more familiar
with OPPs in general.

@Rafael: Where is this gone?
http://permalink.gmane.org/gmane.linux.power-management.general/49384

OK, since this has been restored, what am I supposed to do with the
$subject patch?

[Viresh Kumar]

Post by Rafael J. Wysocki
OK, since this has been restored, what am I supposed to do with the
$subject patch?

Yeah, so the patch looked fine to me. If this can still be applied without
Lucas required to resend it, please add my Ack and apply it :)
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Rafael J. Wysocki]

Post by Viresh Kumar

Post by Rafael J. Wysocki
OK, since this has been restored, what am I supposed to do with the
$subject patch?

Yeah, so the patch looked fine to me. If this can still be applied without
Lucas required to resend it, please add my Ack and apply it :)

No, it didn't apply for me cleanly.

Lucas, can you please rebase this on top of the current Linus' tree and resend?

[Lucas Stach]

If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Signed-off-by: Lucas Stach <***@pengutronix.de>
Acked-by: Viresh Kumar <***@linaro.org>
---
resend 2:
- no functional change, rebase against latest Linus tree
- added Viresh's ack
resend:
- no functional change, split out from the imx5 cpufreq series
v2:
- rebase on top of pm/linux-next
---
drivers/cpufreq/cpufreq-dt.c | 66 +++++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 6bbb8b913446..4485c8eccdc2 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -185,6 +185,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
struct device *cpu_dev;
struct regulator *cpu_reg;
struct clk *cpu_clk;
+ unsigned long min_uV = ~0, max_uV = 0;
unsigned int transition_latency;
int ret;

@@ -204,16 +205,10 @@ static int cpufreq_init(struct cpufreq_policy *policy)
/* OPPs might be populated at runtime, don't check for error here */
of_init_opp_table(cpu_dev);

- ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
- if (ret) {
- dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret);
- goto out_put_node;
- }
-
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
ret = -ENOMEM;
- goto out_free_table;
+ goto out_put_node;
}

of_property_read_u32(np, "voltage-tolerance", &priv->voltage_tolerance);
@@ -222,30 +217,49 @@ static int cpufreq_init(struct cpufreq_policy *policy)
transition_latency = CPUFREQ_ETERNAL;

if (!IS_ERR(cpu_reg)) {
- struct dev_pm_opp *opp;
- unsigned long min_uV, max_uV;
- int i;
-
/*
- * OPP is maintained in order of increasing frequency, and
- * freq_table initialised from OPP is therefore sorted in the
- * same order.
+ * Disable any OPPs where the connected regulator isn't able to
+ * provide the specified voltage and record minimum and maximum
+ * voltage levels.
*/
- for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
- ;
- rcu_read_lock();
- opp = dev_pm_opp_find_freq_exact(cpu_dev,
- freq_table[0].frequency * 1000, true);
- min_uV = dev_pm_opp_get_voltage(opp);
- opp = dev_pm_opp_find_freq_exact(cpu_dev,
- freq_table[i-1].frequency * 1000, true);
- max_uV = dev_pm_opp_get_voltage(opp);
- rcu_read_unlock();
+ while (1) {
+ struct dev_pm_opp *opp;
+ unsigned long opp_freq = 0, opp_uV, tol_uV;
+
+ rcu_read_lock();
+ opp = dev_pm_opp_find_freq_ceil(cpu_dev, &opp_freq);
+ if (IS_ERR(opp)) {
+ rcu_read_unlock();
+ break;
+ }
+ opp_uV = dev_pm_opp_get_voltage(opp);
+ rcu_read_unlock();
+
+ tol_uV = opp_uV * priv->voltage_tolerance / 100;
+ if (regulator_is_supported_voltage(cpu_reg, opp_uV,
+ opp_uV + tol_uV)) {
+ if (opp_uV < min_uV)
+ min_uV = opp_uV;
+ if (opp_uV > max_uV)
+ max_uV = opp_uV;
+ } else {
+ dev_pm_opp_disable(cpu_dev, opp_freq);
+ }
+
+ opp_freq++;
+ }
+
ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
if (ret > 0)
transition_latency += ret * 1000;
}

+ ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
+ if (ret) {
+ pr_err("failed to init cpufreq table: %d\n", ret);
+ goto out_free_priv;
+ }
+
/*
* For now, just loading the cooling device;
* thermal DT code takes care of matching them.
@@ -275,9 +289,9 @@ static int cpufreq_init(struct cpufreq_policy *policy)

out_cooling_unregister:
cpufreq_cooling_unregister(priv->cdev);
- kfree(priv);
-out_free_table:
dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
+out_free_priv:
+ kfree(priv);
out_put_node:
of_node_put(np);
out_put_reg_clk:

[Rafael J. Wysocki]

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Applied, thanks!

[Geert Uytterhoeven]

Hi Rafael, Lucas,

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Applied, thanks!

This commit
(http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=d7bbd4cd0359d781b67c9e621d4bbfd1bb2f3783)
causes a boot regression on r8a7791/koelsch. It hangs after:

TCP: cubic registered
Initializing XFRM netlink socket
NET: Registered protocol family 17
NET: Registered protocol family 15
ata1: link resume succeeded after 1 retries
ata1: SATA link down (SStatus 0 SControl 300)
random: nonblocking pool is initialized

With more debugging, it seems to end up in an infinite loop
calling runtime_{suspend,resume}():

cpufreq-dt cpufreq-dt: pm_clk_notify() 4
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
MSTP i2c6 ON
i2c-sh_mobile e60b0000.i2c: pm_clk_suspend()
MSTP i2c6 OFF
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
MSTP i2c6 ON
i2c-sh_mobile e60b0000.i2c: pm_clk_suspend()
MSTP i2c6 OFF
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
MSTP i2c6 ON
i2c-sh_mobile e60b0000.i2c: pm_clk_suspend()
MSTP i2c6 OFF
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
MSTP i2c6 ON
i2c-sh_mobile e60b0000.i2c: pm_clk_suspend()
MSTP i2c6 OFF
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
...

Reverting this commit fixes the issue, and makes the boot continue with:

cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq: 1300000 KHz
cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
changed to: 1312500 KHz
cpu cpu1: failed to get cpu-2 clock: 1
cpufreq_dt: cpufreq_init: Failed to allocate resources: -2

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Lucas Stach]

Hi Geert,

Post by Geert Uytterhoeven
Hi Rafael, Lucas,

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Applied, thanks!

This commit
(http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=d7bbd4cd0359d781b67c9e621d4bbfd1bb2f3783)
TCP: cubic registered
Initializing XFRM netlink socket
NET: Registered protocol family 17
NET: Registered protocol family 15
ata1: link resume succeeded after 1 retries
ata1: SATA link down (SStatus 0 SControl 300)
random: nonblocking pool is initialized
With more debugging, it seems to end up in an infinite loop
cpufreq-dt cpufreq-dt: pm_clk_notify() 4
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
MSTP i2c6 ON
i2c-sh_mobile e60b0000.i2c: pm_clk_suspend()
MSTP i2c6 OFF
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
MSTP i2c6 ON
i2c-sh_mobile e60b0000.i2c: pm_clk_suspend()
MSTP i2c6 OFF
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
MSTP i2c6 ON
i2c-sh_mobile e60b0000.i2c: pm_clk_suspend()
MSTP i2c6 OFF
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
MSTP i2c6 ON
i2c-sh_mobile e60b0000.i2c: pm_clk_suspend()
MSTP i2c6 OFF
i2c-sh_mobile e60b0000.i2c: pm_clk_resume()
...
cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq: 1300000 KHz
cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
changed to: 1312500 KHz
cpu cpu1: failed to get cpu-2 clock: 1
cpufreq_dt: cpufreq_init: Failed to allocate resources: -2

Urgh, thanks for the report. Am I right that for koelsch you do
reference a regulator supply for the cpu, but don't actually have a
driver for it, so a dummy regulator gets plugged in there?

Regards,
Lucas

[Geert Uytterhoeven]

Hi Lucas,

Post by Lucas Stach

Post by Geert Uytterhoeven

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Applied, thanks!

This commit
(http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=d7bbd4cd0359d781b67c9e621d4bbfd1bb2f3783)
TCP: cubic registered
Initializing XFRM netlink socket
NET: Registered protocol family 17
NET: Registered protocol family 15
ata1: link resume succeeded after 1 retries
ata1: SATA link down (SStatus 0 SControl 300)
random: nonblocking pool is initialized
cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq: 1300000 KHz
cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
changed to: 1312500 KHz
cpu cpu1: failed to get cpu-2 clock: 1
cpufreq_dt: cpufreq_init: Failed to allocate resources: -2

Urgh, thanks for the report. Am I right that for koelsch you do
reference a regulator supply for the cpu, but don't actually have a
driver for it, so a dummy regulator gets plugged in there?

arch/arm/boot/dts/r8a7791-koelsch.dts has:

&cpu0 {
cpu0-supply = <&vdd_dvfs>;
};

&i2c6 {
status = "okay";
clock-frequency = <100000>;

vdd_dvfs: ***@68 {
compatible = "dlg,da9210";
reg = <0x68>;

regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
regulator-boot-on;
regulator-always-on;
};
};

CONFIG_REGULATOR_DA9210=y

and the driver does seem to run:

DA9210: 1000 mV at 4600 mA

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Lucas Stach]

Post by Geert Uytterhoeven
Hi Lucas,

Post by Lucas Stach

Post by Geert Uytterhoeven

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Applied, thanks!

This commit
(http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=d7bbd4cd0359d781b67c9e621d4bbfd1bb2f3783)
TCP: cubic registered
Initializing XFRM netlink socket
NET: Registered protocol family 17
NET: Registered protocol family 15
ata1: link resume succeeded after 1 retries
ata1: SATA link down (SStatus 0 SControl 300)
random: nonblocking pool is initialized
cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq: 1300000 KHz
cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
changed to: 1312500 KHz
cpu cpu1: failed to get cpu-2 clock: 1
cpufreq_dt: cpufreq_init: Failed to allocate resources: -2

Urgh, thanks for the report. Am I right that for koelsch you do
reference a regulator supply for the cpu, but don't actually have a
driver for it, so a dummy regulator gets plugged in there?

&cpu0 {
cpu0-supply = <&vdd_dvfs>;
};
&i2c6 {
status = "okay";
clock-frequency = <100000>;
compatible = "dlg,da9210";
reg = <0x68>;
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
regulator-boot-on;
regulator-always-on;
};
};
CONFIG_REGULATOR_DA9210=y
DA9210: 1000 mV at 4600 mA

Ah right, I misread your initial report. So the issue doesn't seem to be
directly related to the cpufreq-dt driver but seems to be located
somewhere deeper down the chain. The fact that this change triggers the
bug may be a hint here. The only thing which is new in the change is
that it tries to get the supported voltage from the regulator. As the
regulator can not change its voltage (min_uV == max_uV) this translates
directly to a get_voltage() which in turn only tries to read a register
of the i2c chip via regmap.

So it seems that somehow the i2c transaction fails and things spin
indefinitely there. Maybe this gives a clue on how to debug this
further.

Regards,
Lucas

[Rafael J. Wysocki]

Post by Lucas Stach

Post by Geert Uytterhoeven
Hi Lucas,

Post by Lucas Stach

Post by Geert Uytterhoeven

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Applied, thanks!

This commit
(http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=d7bbd4cd0359d781b67c9e621d4bbfd1bb2f3783)
TCP: cubic registered
Initializing XFRM netlink socket
NET: Registered protocol family 17
NET: Registered protocol family 15
ata1: link resume succeeded after 1 retries
ata1: SATA link down (SStatus 0 SControl 300)
random: nonblocking pool is initialized
cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq: 1300000 KHz
cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
changed to: 1312500 KHz
cpu cpu1: failed to get cpu-2 clock: 1
cpufreq_dt: cpufreq_init: Failed to allocate resources: -2

Urgh, thanks for the report. Am I right that for koelsch you do
reference a regulator supply for the cpu, but don't actually have a
driver for it, so a dummy regulator gets plugged in there?

&cpu0 {
cpu0-supply = <&vdd_dvfs>;
};
&i2c6 {
status = "okay";
clock-frequency = <100000>;
compatible = "dlg,da9210";
reg = <0x68>;
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
regulator-boot-on;
regulator-always-on;
};
};
CONFIG_REGULATOR_DA9210=y
DA9210: 1000 mV at 4600 mA

Ah right, I misread your initial report. So the issue doesn't seem to be
directly related to the cpufreq-dt driver but seems to be located
somewhere deeper down the chain. The fact that this change triggers the
bug may be a hint here. The only thing which is new in the change is
that it tries to get the supported voltage from the regulator. As the
regulator can not change its voltage (min_uV == max_uV) this translates
directly to a get_voltage() which in turn only tries to read a register
of the i2c chip via regmap.
So it seems that somehow the i2c transaction fails and things spin
indefinitely there. Maybe this gives a clue on how to debug this
further.

Well, I've dropped the patch for now. The underlying issue needs to be fixed
before we can apply it again.

[Khiem Nguyen]

++Inami-san, author of recent CPUFreq patches.

Post by Rafael J. Wysocki

Post by Lucas Stach

Post by Geert Uytterhoeven
Hi Lucas,

Post by Lucas Stach

Post by Geert Uytterhoeven

Post by Rafael J. Wysocki

Post by Lucas Stach
If the regulator connected to the CPU voltage plane doesn't
support an OPP specified voltage with the acceptable tolerance
it's better to just disable the OPP instead of constantly
failing the voltage scaling later on.

Applied, thanks!

This commit
(http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=d7bbd4cd0359d781b67c9e621d4bbfd1bb2f3783)
TCP: cubic registered
Initializing XFRM netlink socket
NET: Registered protocol family 17
NET: Registered protocol family 15
ata1: link resume succeeded after 1 retries
ata1: SATA link down (SStatus 0 SControl 300)
random: nonblocking pool is initialized
cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq: 1300000 KHz
cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency
changed to: 1312500 KHz
cpu cpu1: failed to get cpu-2 clock: 1
cpufreq_dt: cpufreq_init: Failed to allocate resources: -2

Urgh, thanks for the report. Am I right that for koelsch you do
reference a regulator supply for the cpu, but don't actually have a
driver for it, so a dummy regulator gets plugged in there?

&cpu0 {
cpu0-supply = <&vdd_dvfs>;
};
&i2c6 {
status = "okay";
clock-frequency = <100000>;
compatible = "dlg,da9210";
reg = <0x68>;
regulator-min-microvolt = <1000000>;
regulator-max-microvolt = <1000000>;
regulator-boot-on;
regulator-always-on;
};
};
CONFIG_REGULATOR_DA9210=y
DA9210: 1000 mV at 4600 mA

Ah right, I misread your initial report. So the issue doesn't seem to be
directly related to the cpufreq-dt driver but seems to be located
somewhere deeper down the chain. The fact that this change triggers the
bug may be a hint here. The only thing which is new in the change is
that it tries to get the supported voltage from the regulator. As the
regulator can not change its voltage (min_uV == max_uV) this translates
directly to a get_voltage() which in turn only tries to read a register
of the i2c chip via regmap.
So it seems that somehow the i2c transaction fails and things spin
indefinitely there. Maybe this gives a clue on how to debug this
further.

Well, I've dropped the patch for now. The underlying issue needs to be fixed
before we can apply it again.