ARM: imx: Set LDO regulator supply
v2 cover ARM: imx: Set LDO regulator supply
Setting the LDO regulator parent is optional but beneficial. It will cause the PMIC output voltage to be dynamically set to the minimum input for the LDOs, this should be more efficient.
This propagation was introduced by:
commit fc42112c0eaa ("regulator: core: Propagate voltage changes to supply regulators")
Changes since v1:
* Drop patch 1 since it only avoids logging a warning and the gpc driver
is going through more changes.
* Initialize cpufreq->suspend_freq based on policy->max instead.
* Remove reference to ldo-bypass from suspend_freq patch message.
It's not clear if policy->cpuinfo.max would be preferable, at init time they should be identical.
Irina Tirdea (1):
cpufreq: imx6q: Fix handling EPROBE_DEFER from regulator
Leonard Crestez (3):
cpufreq: imx6q: Set max suspend_freq to avoid changes during suspend
ARM: dts: imx6qdl-sabresd: Set LDO regulator supply
ARM: dts: imx6qp-sabresd: Set reg_arm regulator supply
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 12 ++++++++++++ arch/arm/boot/dts/imx6qp-sabresd.dts | 4 ++-- drivers/cpufreq/imx6q-cpufreq.c | 15 ++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-)
Signature · 1 line
2.7.4
PATCH v2 1/4 cpufreq: imx6q: Fix handling EPROBE_DEFER from regulator
From: Irina Tirdea <[email protected]>
If there are any errors in getting the cpu0 regulators, the driver returns -ENOENT. In case the regulators are not yet available, the devm_regulator_get calls will return -EPROBE_DEFER, so that the driver can be probed later. If we return -ENOENT, the driver will fail its initialization and will not try to probe again (when the regulators become available).
Return the actual error received from regulator_get in probe. Print a differentiated message in case we need to probe the device later and in case we actually failed. Also add a message to inform when the driver has been successfully registered.
--- drivers/cpufreq/imx6q-cpufreq.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/cpufreq/imx6q-cpufreq.c +7 -0
@@ -222,6 +222,13 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) arm_reg =regulator_get (cpu_dev,"arm" ); pu_reg =regulator_get_optional (cpu_dev,"pu" ); soc_reg =regulator_get (cpu_dev,"soc" ); +if (PTR_ERR (arm_reg) == -EPROBE_DEFER || +PTR_ERR (soc_reg) == -EPROBE_DEFER || +PTR_ERR (pu_reg) == -EPROBE_DEFER ) { + ret = -EPROBE_DEFER ; +dev_dbg (cpu_dev,"regulators not ready, defer\n" ); + goto put_reg; + }if (IS_ERR (arm_reg) ||IS_ERR (soc_reg)) {dev_err (cpu_dev,"failed to get regulators\n" ); ret = -ENOENT ;
Signature · 1 line
2.7.4
PATCH v2 2/4 cpufreq: imx6q: Set max suspend_freq to avoid changes during suspend
In reply to Leonard CrestezIf the cpufreq driver tries to modify voltage/freq during suspend/resume it might need to control an external PMIC via I2C or SPI but those devices might be already suspended. This issue is likely to happen whenever the LDOs have their vin-supply set.
To avoid this scenario we just increase cpufreq to the maximum before suspend.
--- drivers/cpufreq/imx6q-cpufreq.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/cpufreq/imx6q-cpufreq.c +7 -1
@@ -161,8 +161,13 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)static int imx6q_cpufreq_init (struct cpufreq_policy *policy) { +int ret; + policy->clk = arm_clk; -return cpufreq_generic_init (policy, freq_table, transition_latency); + ret =cpufreq_generic_init (policy, freq_table, transition_latency); + policy->suspend_freq = policy->max; + +return ret; }static struct cpufreq_driver imx6q_cpufreq_driver = {
@@ -173,6 +178,7 @@ static struct cpufreq_driver imx6q_cpufreq_driver = { .init = imx6q_cpufreq_init, .name = "imx6q-cpufreq", .attr = cpufreq_generic_attr, + .suspend = cpufreq_generic_suspend, };static int imx6q_cpufreq_probe (struct platform_device *pdev)
Signature · 1 line
2.7.4
Re: [PATCH v2 2/4] cpufreq: imx6q: Set max suspend_freq to avoid changes during suspend
Am Dienstag, den 04.04.2017, 20:04 +0300 schrieb Leonard Crestez:
Quoted 9 lines
If the cpufreq driver tries to modify voltage/freq during suspend/resume it might need to control an external PMIC via I2C or SPI but those devices might be already suspended. This issue is likely to happen whenever the LDOs have their vin-supply set.
To avoid this scenario we just increase cpufreq to the maximum before suspend.
Signed-off-by Leonard Crestez <[email protected]>
Quoted 31 lines
--- drivers/cpufreq/imx6q-cpufreq.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)drivers/cpufreq/imx6q-cpufreq.c +7
-1@@ -161,8 +161,13 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)static int imx6q_cpufreq_init (struct cpufreq_policy *policy) { +int ret; + policy->clk = arm_clk; -return cpufreq_generic_init (policy, freq_table, transition_latency); + ret =cpufreq_generic_init (policy, freq_table, transition_latency); + policy->suspend_freq = policy->max; + +return ret; }static struct cpufreq_driver imx6q_cpufreq_driver = {@@ -173,6 +178,7 @@ static struct cpufreq_driver imx6q_cpufreq_driver = { .init = imx6q_cpufreq_init, .name = "imx6q-cpufreq", .attr = cpufreq_generic_attr, + .suspend = cpufreq_generic_suspend, };static int imx6q_cpufreq_probe (struct platform_device *pdev)
Viresh Kumar
maintainerRe: [PATCH v2 2/4] cpufreq: imx6q: Set max suspend_freq to avoid changes during suspend
In reply to Leonard CrestezQuoted 5 lines · Leonard Crestez
If the cpufreq driver tries to modify voltage/freq during suspend/resume it might need to control an external PMIC via I2C or SPI but those devices might be already suspended. This issue is likely to happen whenever the LDOs have their vin-supply set.
Quoted 7 lines
To avoid this scenario we just increase cpufreq to the maximum before suspend.
Signed-off-by Leonard Crestez <[email protected]>--- drivers/cpufreq/imx6q-cpufreq.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
I have already Acked this earlier, why didn't you add it ?
Signature · 1 line
viresh
Re: [PATCH v2 2/4] cpufreq: imx6q: Set max suspend_freq to avoid changes during suspend
Quoted 9 lines · Viresh Kumar
Quoted 8 lines · Leonard Crestez
If the cpufreq driver tries to modify voltage/freq during suspend/resume it might need to control an external PMIC via I2C or SPI but those devices might be already suspended. This issue is likely to happen whenever the LDOs have their vin-supply set.
To avoid this scenario we just increase cpufreq to the maximum before suspend.
Quoted 6 lines
Quoted 4 lines
Signed-off-by Leonard Crestez <[email protected]>--- drivers/cpufreq/imx6q-cpufreq.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)I have already Acked this earlier, why didn't you add it ?
Because v2 is different based on comments from Lucas. It didn't seem appropriate to keep an old "Acked-by" in this case.
Re: [PATCH v2 2/4] cpufreq: imx6q: Set max suspend_freq to avoid changes during suspend
In reply to Leonard CrestezQuoted 33 lines · Leonard Crestez
If the cpufreq driver tries to modify voltage/freq during suspend/resume it might need to control an external PMIC via I2C or SPI but those devices might be already suspended. This issue is likely to happen whenever the LDOs have their vin-supply set.
To avoid this scenario we just increase cpufreq to the maximum before suspend.
Signed-off-by Leonard Crestez <[email protected]>--- drivers/cpufreq/imx6q-cpufreq.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)drivers/cpufreq/imx6q-cpufreq.c +6
-1@@ -161,8 +161,13 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)static int imx6q_cpufreq_init (struct cpufreq_policy *policy) { +int ret; + policy->clk = arm_clk; -return cpufreq_generic_init (policy, freq_table, transition_latency); + ret =cpufreq_generic_init (policy, freq_table, transition_latency); + policy->suspend_freq = policy->max; + +return ret; }static struct cpufreq_driver imx6q_cpufreq_driver = {
drivers/cpufreq/imx6q-cpufreq.c static struct cpufreq_driver imx6q_cpufreq_driver = { · line 184
Quoted 7 lines
drivers/cpufreq/imx6q-cpufreq.c +1
-0@@ -173,6 +178,7 @@ static struct cpufreq_driver imx6q_cpufreq_driver = { .init = imx6q_cpufreq_init, .name = "imx6q-cpufreq", .attr = cpufreq_generic_attr, + .suspend = cpufreq_generic_suspend, };static int imx6q_cpufreq_probe (struct platform_device *pdev)
Signature · 1 line
viresh
PATCH v2 3/4 ARM: dts: imx6qdl-sabresd: Set LDO regulator supply
In reply to Leonard CrestezSetting the supply is optional but beneficial, it will cause PMIC voltages to be dynamically changed with cpu frequency.
--- arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 12 ++++++++++++ 1 file changed, 12 insertions(+)
arch/arm/boot/dts/imx6qdl-sabresd.dtsi +12 -0
@@ -548,6 +548,18 @@ status = "okay"; }; +®_arm { + vin-supply = <&sw1a_reg>; +}; + +®_pu { + vin-supply = <&sw1c_reg>; +}; + +®_soc { + vin-supply = <&sw1c_reg>; +}; + &snvs_poweroff { status = "okay"; };
Signature · 1 line
2.7.4
Shawn Guo
AppliedRe: [PATCH v2 3/4] ARM: dts: imx6qdl-sabresd: Set LDO regulator supply
Quoted 5 lines · Leonard Crestez
Setting the supply is optional but beneficial, it will cause PMIC voltages to be dynamically changed with cpu frequency.
Signed-off-by Leonard Crestez <[email protected]>Reviewed-by Lucas Stach <[email protected]>
Applied, thanks.
PATCH v2 4/4 ARM: dts: imx6qp-sabresd: Set reg_arm regulator supply
In reply to Leonard CrestezOn imx6qp-sabresd LDO_ARM is connected to a different PMIC output than the other imx6qdl-sabresd boards.
Setting cpu0 arm-supply to sw2_reg is wrong, this must have mistakenly slipped out of the vendor tree where this is are used for LDO bypass.
--- arch/arm/boot/dts/imx6qp-sabresd.dts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/imx6qp-sabresd.dts +2 -2
@@ -50,8 +50,8 @@ compatible = "fsl,imx6qp-sabresd", "fsl,imx6qp"; }; -&cpu0 { - arm-supply = <&sw2_reg>; +®_arm { + vin-supply = <&sw2_reg>; }; &iomuxc {
Signature · 1 line
2.7.4
Shawn Guo
AppliedRe: [PATCH v2 4/4] ARM: dts: imx6qp-sabresd: Set reg_arm regulator supply
Quoted 8 lines · Leonard Crestez
On imx6qp-sabresd
LDO_ARMis connected to a different PMIC output than the other imx6qdl-sabresd boards.Setting cpu0 arm-supply to
sw2_regis wrong, this must have mistakenly slipped out of the vendor tree where this is are used for LDO bypass.Signed-off-by Leonard Crestez <[email protected]>Reviewed-by Lucas Stach <[email protected]>
Applied, thanks.
Quoted 8 lines · Leonard Crestez
Setting the LDO regulator parent is optional but beneficial. It will cause the PMIC output voltage to be dynamically set to the minimum input for the LDOs, this should be more efficient.
This propagation was introduced by:
commitfc42112c0eaa("regulator: core: Propagate voltage changes to supply regulators")
Quoted 5 lines
Changes since v1:
* Drop patch 1 since it only avoids logging a warning and the gpc driver
is going through more changes.
* Initializecpufreq->suspend_freqbased onpolicy->maxinstead.
* Remove reference to ldo-bypass fromsuspend_freqpatch message.
Hello,
This is a gentle reminder that this was sent ~2 weeks ago and patches 1 and 2 are still waiting (with no objections). Patches 3/4 were applied.
--
Regards,
Leonard