[PATCH 2/2] mxs: Don't enable 4P2 reg if MXS is powered only from DCDC_BATT
'mxs_power_enable_4p2()' function call was added to 'mxs_batt_boot()' in 'commit a0f97610757d' to enable DCDC converter when board is powered from 5V and has detected sufficient battery voltage. This involves enabling 4P2 regulator and there is a code in 'mxs_power_enable_4p2()' that disables VDDIO, VDDA, VDDD outputs of the DCDC converter and enables BO for each power rail:
setbits_le32(&power_regs->hw_power_vddioctrl, POWER_VDDIOCTRL_DISABLE_FET | POWER_VDDIOCTRL_PWDN_BRNOUT);
There is no issue if the MXS is powered from the 5V source and linear regulators are supplying power to the VDDIO, VDDA, VDDD rails. However, if the MXS is powered only from the DCDC_BATT without 5V, disabling the DCDC converter outputs causes VDDIO, VDDA, VDDD rails to lose power.
The proposed solution is not to call the 'mxs_power_enable_4p2()' function if the MXS is powered only by the DCDC_BATT, because there is no reason to enable 4P2 regulator in this case. Also 5V brownout should not be enabled in 'mxs_power_init()' and linear regulator checks should be disabled in 'mxs_power_set_vddx()'.
Signed-off-by: Cody Green cody@londelec.com Cc: Stefano Babic sbabic@denx.de Cc: Marek Vasut marex@denx.de Cc: Fabio Estevam festevam@gmail.com --- arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 33b76533e4..72172705f2 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c @@ -752,7 +752,9 @@ static void mxs_batt_boot(void) POWER_5VCTRL_CHARGE_4P2_ILIMIT_MASK, 0x8 << POWER_5VCTRL_CHARGE_4P2_ILIMIT_OFFSET);
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE mxs_power_enable_4p2(); +#endif }
/** @@ -1137,8 +1139,11 @@ static void mxs_power_set_vddx(const struct mxs_vddx_cfg *cfg, cur_target += cfg->lowest_mV;
adjust_up = new_target > cur_target; + +#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE if (cfg->powered_by_linreg) powered_by_linreg = cfg->powered_by_linreg(); +#endif
if (adjust_up && cfg->bo_irq) { if (powered_by_linreg) { @@ -1269,7 +1274,9 @@ void mxs_power_init(void) POWER_CTRL_VBUS_VALID_IRQ | POWER_CTRL_BATT_BO_IRQ | POWER_CTRL_DCDC4P2_BO_IRQ, &power_regs->hw_power_ctrl_clr);
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE writel(POWER_5VCTRL_PWDN_5VBRNOUT, &power_regs->hw_power_5vctrl_set); +#endif
early_delay(1000); }
On 7/3/23 18:33, Cody Green wrote:
'mxs_power_enable_4p2()' function call was added to 'mxs_batt_boot()' in 'commit a0f97610757d' to enable DCDC converter when board is powered from 5V and has detected sufficient battery voltage. This involves enabling 4P2 regulator and there is a code in 'mxs_power_enable_4p2()' that disables VDDIO, VDDA, VDDD outputs of the DCDC converter and enables BO for each power rail:
setbits_le32(&power_regs->hw_power_vddioctrl, POWER_VDDIOCTRL_DISABLE_FET | POWER_VDDIOCTRL_PWDN_BRNOUT);
There is no issue if the MXS is powered from the 5V source and linear regulators are supplying power to the VDDIO, VDDA, VDDD rails. However, if the MXS is powered only from the DCDC_BATT without 5V, disabling the DCDC converter outputs causes VDDIO, VDDA, VDDD rails to lose power.
The proposed solution is not to call the 'mxs_power_enable_4p2()' function if the MXS is powered only by the DCDC_BATT, because there is no reason to enable 4P2 regulator in this case. Also 5V brownout should not be enabled in 'mxs_power_init()' and linear regulator checks should be disabled in 'mxs_power_set_vddx()'.
Signed-off-by: Cody Green cody@londelec.com Cc: Stefano Babic sbabic@denx.de Cc: Marek Vasut marex@denx.de Cc: Fabio Estevam festevam@gmail.com
arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 33b76533e4..72172705f2 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c @@ -752,7 +752,9 @@ static void mxs_batt_boot(void) POWER_5VCTRL_CHARGE_4P2_ILIMIT_MASK, 0x8 << POWER_5VCTRL_CHARGE_4P2_ILIMIT_OFFSET);
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE mxs_power_enable_4p2(); +#endif }
/** @@ -1137,8 +1139,11 @@ static void mxs_power_set_vddx(const struct mxs_vddx_cfg *cfg, cur_target += cfg->lowest_mV;
adjust_up = new_target > cur_target;
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE if (cfg->powered_by_linreg) powered_by_linreg = cfg->powered_by_linreg(); +#endif
if (adjust_up && cfg->bo_irq) { if (powered_by_linreg) { @@ -1269,7 +1274,9 @@ void mxs_power_init(void) POWER_CTRL_VBUS_VALID_IRQ | POWER_CTRL_BATT_BO_IRQ | POWER_CTRL_DCDC4P2_BO_IRQ, &power_regs->hw_power_ctrl_clr);
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE writel(POWER_5VCTRL_PWDN_5VBRNOUT, &power_regs->hw_power_5vctrl_set); +#endif
early_delay(1000); }
+CC Lukasz
Hi Marek, Cody,
On 7/3/23 18:33, Cody Green wrote:
'mxs_power_enable_4p2()' function call was added to 'mxs_batt_boot()' in 'commit a0f97610757d' to enable DCDC converter when board is powered from 5V and has detected sufficient battery voltage. This involves enabling 4P2 regulator and there is a code in 'mxs_power_enable_4p2()' that disables VDDIO, VDDA, VDDD outputs of the DCDC converter and enables BO for each power rail:
setbits_le32(&power_regs->hw_power_vddioctrl, POWER_VDDIOCTRL_DISABLE_FET | POWER_VDDIOCTRL_PWDN_BRNOUT);
There is no issue if the MXS is powered from the 5V source and linear regulators are supplying power to the VDDIO, VDDA, VDDD rails. However, if the MXS is powered only from the DCDC_BATT without 5V, disabling the DCDC converter outputs causes VDDIO, VDDA, VDDD rails to lose power.
I think that I've also hit the same issue with the XEA board (upstreamed).
I've prepared a set of patches: https://patchwork.ozlabs.org/project/uboot/patch/20230509143243.1523791-5-lu...
to fix this problem.
Those patches are now for some time on the mailing list for review - and I do hope that Stefano will pull them with next PR for u-boot-imx repository.
The proposed solution is not to call the 'mxs_power_enable_4p2()' function if the MXS is powered only by the DCDC_BATT, because there is no reason to enable 4P2 regulator in this case.
I think this patch: https://patchwork.ozlabs.org/project/uboot/patch/20230509143243.1523791-3-lu...
address exactly this issue.
Also 5V brownout should not be enabled in 'mxs_power_init()' and linear regulator checks should be disabled in 'mxs_power_set_vddx()'.
I've also added some code to limit the VDD5V current when we intend to use DCDC_BATT input as the _primary_ source of power (AN4199 advises this one for industrial applications as the most reliable).
Signed-off-by: Cody Green cody@londelec.com Cc: Stefano Babic sbabic@denx.de Cc: Marek Vasut marex@denx.de Cc: Fabio Estevam festevam@gmail.com
arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 33b76533e4..72172705f2 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c @@ -752,7 +752,9 @@ static void mxs_batt_boot(void) POWER_5VCTRL_CHARGE_4P2_ILIMIT_MASK, 0x8 << POWER_5VCTRL_CHARGE_4P2_ILIMIT_OFFSET);
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE mxs_power_enable_4p2(); +#endif }
/** @@ -1137,8 +1139,11 @@ static void mxs_power_set_vddx(const struct mxs_vddx_cfg *cfg, cur_target += cfg->lowest_mV;
adjust_up = new_target > cur_target;
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE if (cfg->powered_by_linreg) powered_by_linreg = cfg->powered_by_linreg(); +#endif
if (adjust_up && cfg->bo_irq) { if (powered_by_linreg) { @@ -1269,7 +1274,9 @@ void mxs_power_init(void) POWER_CTRL_VBUS_VALID_IRQ | POWER_CTRL_BATT_BO_IRQ | POWER_CTRL_DCDC4P2_BO_IRQ, &power_regs->hw_power_ctrl_clr); +#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE writel(POWER_5VCTRL_PWDN_5VBRNOUT, &power_regs->hw_power_5vctrl_set); +#endif
early_delay(1000); }
+CC Lukasz
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Erika Unter HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Tue, Jul 4, 2023 at 9:19 AM Lukasz Majewski lukma@denx.de wrote: Hi Lukasz,
Hi Marek, Cody,
On 7/3/23 18:33, Cody Green wrote:
'mxs_power_enable_4p2()' function call was added to 'mxs_batt_boot()' in 'commit a0f97610757d' to enable DCDC converter when board is powered from 5V and has detected sufficient battery voltage. This involves enabling 4P2 regulator and there is a code in 'mxs_power_enable_4p2()' that disables VDDIO, VDDA, VDDD outputs of the DCDC converter and enables BO for each power rail:
setbits_le32(&power_regs->hw_power_vddioctrl, POWER_VDDIOCTRL_DISABLE_FET | POWER_VDDIOCTRL_PWDN_BRNOUT);
There is no issue if the MXS is powered from the 5V source and linear regulators are supplying power to the VDDIO, VDDA, VDDD rails. However, if the MXS is powered only from the DCDC_BATT without 5V, disabling the DCDC converter outputs causes VDDIO, VDDA, VDDD rails to lose power.
I think that I've also hit the same issue with the XEA board (upstreamed).
I've prepared a set of patches: https://patchwork.ozlabs.org/project/uboot/patch/20230509143243.1523791-5-lu...
to fix this problem.
Those patches are now for some time on the mailing list for review - and I do hope that Stefano will pull them with next PR for u-boot-imx repository.
The proposed solution is not to call the 'mxs_power_enable_4p2()' function if the MXS is powered only by the DCDC_BATT, because there is no reason to enable 4P2 regulator in this case.
I think this patch: https://patchwork.ozlabs.org/project/uboot/patch/20230509143243.1523791-3-lu...
address exactly this issue.
Yes, your patch [3/5] addresses the issue perfectly! I was considering to add the Kconfig option, but decided against it, as I thought nobody else is running the MXS only from DCDC_BATT without 5V. In my opinion Kconfig option is better solution than my patch and I am happy to add 'Tested-by' tag to your patch [3/5], if you wish.
Also 5V brownout should not be enabled in 'mxs_power_init()' and linear regulator checks should be disabled in 'mxs_power_set_vddx()'.
I've also added some code to limit the VDD5V current when we intend to use DCDC_BATT input as the _primary_ source of power (AN4199 advises this one for industrial applications as the most reliable).
I checked other patches in your set and as far as I can tell the rest of them apply if the board is powered from both DCDC_BATT and 5V source. This is all good stuff, but what shall we do about enabling 5V brownout:
writel(POWER_5VCTRL_PWDN_5VBRNOUT, &power_regs->hw_power_5vctrl_set); in 'void mxs_power_init()'?
Would you consider using the new 'MXS_PMU_ENABLE_4P2_LINEAR_REGULATOR' option to not enable 5V brownout as well? e.g.
if (CONFIG_IS_ENABLED(MXS_PMU_ENABLE_4P2_LINEAR_REGULATOR)) writel(POWER_5VCTRL_PWDN_5VBRNOUT, &power_regs->hw_power_5vctrl_set);
My reasoning is the 4P2 regulator would be disabled only in the scenario if the board is running without 5V hence there is no need for a 5V brownout.
There is a slightly different problem with:
if (cfg->powered_by_linreg) powered_by_linreg = cfg->powered_by_linreg(); in 'mxs_power_set_vddx()'.
I noticed that function 'mxs_get_vddd_power_source_off()' returns '1' (i.e. powered_by_linreg) because of the following code:
if (!(tmp & POWER_VDDDCTRL_ENABLE_LINREG)) { if ((tmp & POWER_VDDDCTRL_LINREG_OFFSET_MASK) == POWER_VDDDCTRL_LINREG_OFFSET_1STEPS_BELOW) { return 1; } }
I am not sure why this piece of code even exists, but rather than risking to brake something by removing it, I decided that we can simply disable calling 'mxs_get_vddd_power_source_off()' if the board is powered from DCDC_BATT only. So would you consider the following as well?
if (CONFIG_IS_ENABLED(MXS_PMU_ENABLE_4P2_LINEAR_REGULATOR)) if (cfg->powered_by_linreg) powered_by_linreg = cfg->powered_by_linreg();
Signed-off-by: Cody Green cody@londelec.com Cc: Stefano Babic sbabic@denx.de Cc: Marek Vasut marex@denx.de Cc: Fabio Estevam festevam@gmail.com
arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 33b76533e4..72172705f2 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c @@ -752,7 +752,9 @@ static void mxs_batt_boot(void) POWER_5VCTRL_CHARGE_4P2_ILIMIT_MASK, 0x8 << POWER_5VCTRL_CHARGE_4P2_ILIMIT_OFFSET); +#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE mxs_power_enable_4p2(); +#endif } /** @@ -1137,8 +1139,11 @@ static void mxs_power_set_vddx(const struct mxs_vddx_cfg *cfg, cur_target += cfg->lowest_mV; adjust_up = new_target > cur_target;
+#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE if (cfg->powered_by_linreg) powered_by_linreg = cfg->powered_by_linreg(); +#endif if (adjust_up && cfg->bo_irq) { if (powered_by_linreg) { @@ -1269,7 +1274,9 @@ void mxs_power_init(void) POWER_CTRL_VBUS_VALID_IRQ | POWER_CTRL_BATT_BO_IRQ | POWER_CTRL_DCDC4P2_BO_IRQ, &power_regs->hw_power_ctrl_clr); +#ifndef CFG_SPL_MXS_NO_VDD5V_SOURCE writel(POWER_5VCTRL_PWDN_5VBRNOUT, &power_regs->hw_power_5vctrl_set); +#endif early_delay(1000); }
+CC Lukasz
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Erika Unter HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
participants (3)
-
Cody Green -
Lukasz Majewski -
Marek Vasut