[RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

[RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

[Alan Kay]

mxs_power_enable_4p2() was added to mxs_batt_boot() in
'commit a0f97610757d ("ARM: mxs: Enable DCDC converter for battery boot")'
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 e.g.

	setbits_le32(&power_regs->hw_power_vddioctrl,
		POWER_VDDIOCTRL_DISABLE_FET | POWER_VDDIOCTRL_PWDN_BRNOUT);

In case the mx28 is powered by the 5V source and linear regulators are
supplying power to the VDDIO, VDDA, VDDD rails there is no issue.

However if the mx28 is powered by the DCDC_BATT source only without 5V,
disabling the DCDC converter outputs causes brownout power down.

The proposed solution is not to call mxs_power_enable_4p2() at all
if the mx28 is powered by the DCDC_BATT source only. There is no
reason to enable 4P2 regulator in this case and setup of all registers
is done in mxs_batt_boot().

Also there is no need to enable 5V brownout in
mxs_power_init() in this case.

Please consider if this is acceptable and I will submit a proper patch.

Signed-off-by: Alan Kay <alan@londelec.com>
---

 arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
index c33170f06d..c8feadce54 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 CONFIG_SPL_MXS_NO_VDD5V_SOURCE
 	mxs_power_enable_4p2();
+#endif
 }
 
 /**
@@ -1267,7 +1269,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 CONFIG_SPL_MXS_NO_VDD5V_SOURCE
 	writel(POWER_5VCTRL_PWDN_5VBRNOUT, &power_regs->hw_power_5vctrl_set);
+#endif
 
 	early_delay(1000);
 }
-- 
2.25.1

Re: [RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

[Fabio Estevam]

Hi Alan,

[Adding Lukasz and Cody]

On Tue, Aug 30, 2022 at 2:49 PM Alan Kay <alan@londelec.com> wrote:
>
> mxs_power_enable_4p2() was added to mxs_batt_boot() in
> 'commit a0f97610757d ("ARM: mxs: Enable DCDC converter for battery boot")'
> 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 e.g.
>
>         setbits_le32(&power_regs->hw_power_vddioctrl,
>                 POWER_VDDIOCTRL_DISABLE_FET | POWER_VDDIOCTRL_PWDN_BRNOUT);
>
> In case the mx28 is powered by the 5V source and linear regulators are
> supplying power to the VDDIO, VDDA, VDDD rails there is no issue.
>
> However if the mx28 is powered by the DCDC_BATT source only without 5V,
> disabling the DCDC converter outputs causes brownout power down.
>
> The proposed solution is not to call mxs_power_enable_4p2() at all
> if the mx28 is powered by the DCDC_BATT source only. There is no
> reason to enable 4P2 regulator in this case and setup of all registers
> is done in mxs_batt_boot().
>
> Also there is no need to enable 5V brownout in
> mxs_power_init() in this case.
>
> Please consider if this is acceptable and I will submit a proper patch.
>
> Signed-off-by: Alan Kay <alan@londelec.com>

Cody also sent a fix for this issue:

https://patchwork.ozlabs.org/project/uboot/patch/20230703163340.101490-2-cody@londelec.com/

Re: [RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2271 bytes --]

Hi Fabio, Alan,

> Hi Alan,
> 
> [Adding Lukasz and Cody]
> 
> On Tue, Aug 30, 2022 at 2:49 PM Alan Kay <alan@londelec.com> wrote:
> >
> > mxs_power_enable_4p2() was added to mxs_batt_boot() in
> > 'commit a0f97610757d ("ARM: mxs: Enable DCDC converter for battery
> > boot")' 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 e.g.
> >
> >         setbits_le32(&power_regs->hw_power_vddioctrl,
> >                 POWER_VDDIOCTRL_DISABLE_FET |
> > POWER_VDDIOCTRL_PWDN_BRNOUT);
> >
> > In case the mx28 is powered by the 5V source and linear regulators
> > are supplying power to the VDDIO, VDDA, VDDD rails there is no
> > issue.
> >
> > However if the mx28 is powered by the DCDC_BATT source only without
> > 5V, disabling the DCDC converter outputs causes brownout power down.
> >
> > The proposed solution is not to call mxs_power_enable_4p2() at all
> > if the mx28 is powered by the DCDC_BATT source only. There is no
> > reason to enable 4P2 regulator in this case and setup of all
> > registers is done in mxs_batt_boot().
> >
> > Also there is no need to enable 5V brownout in
> > mxs_power_init() in this case.
> >
> > Please consider if this is acceptable and I will submit a proper
> > patch.
> >
> > Signed-off-by: Alan Kay <alan@londelec.com>  
> 
> Cody also sent a fix for this issue:
> 
> https://patchwork.ozlabs.org/project/uboot/patch/20230703163340.101490-2-cody@londelec.com/

Isn't this already addressed with:
https://source.denx.de/u-boot/u-boot/-/commit/79230640cb4fb780e2be9bb9a47b31976b18cac4

Please also look into the recent patches for imx287 (xea board for
reference):

tig -- arch/arm/cpu/arm926ejs/mxs/spl_power_init.c

At least SHA1: 65b9b3462bec2966911658836983819ab4e4823e


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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

[Alan Kay]

Hi Lukasz,

On Mon, Oct 9, 2023 at 8:49 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Fabio, Alan,
>
> > Hi Alan,
> >
> > [Adding Lukasz and Cody]
> >
> > On Tue, Aug 30, 2022 at 2:49 PM Alan Kay <alan@londelec.com> wrote:
> > >
> > > mxs_power_enable_4p2() was added to mxs_batt_boot() in
> > > 'commit a0f97610757d ("ARM: mxs: Enable DCDC converter for battery
> > > boot")' 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 e.g.
> > >
> > >         setbits_le32(&power_regs->hw_power_vddioctrl,
> > >                 POWER_VDDIOCTRL_DISABLE_FET |
> > > POWER_VDDIOCTRL_PWDN_BRNOUT);
> > >
> > > In case the mx28 is powered by the 5V source and linear regulators
> > > are supplying power to the VDDIO, VDDA, VDDD rails there is no
> > > issue.
> > >
> > > However if the mx28 is powered by the DCDC_BATT source only without
> > > 5V, disabling the DCDC converter outputs causes brownout power down.
> > >
> > > The proposed solution is not to call mxs_power_enable_4p2() at all
> > > if the mx28 is powered by the DCDC_BATT source only. There is no
> > > reason to enable 4P2 regulator in this case and setup of all
> > > registers is done in mxs_batt_boot().
> > >
> > > Also there is no need to enable 5V brownout in
> > > mxs_power_init() in this case.
> > >
> > > Please consider if this is acceptable and I will submit a proper
> > > patch.
> > >
> > > Signed-off-by: Alan Kay <alan@londelec.com>
> >
> > Cody also sent a fix for this issue:
> >
> > https://patchwork.ozlabs.org/project/uboot/patch/20230703163340.101490-2-cody@londelec.com/
>
> Isn't this already addressed with:
> https://source.denx.de/u-boot/u-boot/-/commit/79230640cb4fb780e2be9bb9a47b31976b18cac4
>
> Please also look into the recent patches for imx287 (xea board for
> reference):
>
> tig -- arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
>
> At least SHA1: 65b9b3462bec2966911658836983819ab4e4823e
>
>
> 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

The patch:
https://source.denx.de/u-boot/u-boot/-/commit/79230640cb4fb780e2be9bb9a47b31976b18cac4
does fix the issue, please feel free to discard my patch, thank you.

Best regards,
Alan Kay

Re: [RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

[Fabio Estevam]

Hi Alan and Cody,

On Mon, Oct 9, 2023 at 9:45 AM Alan Kay <alan@londelec.com> wrote:

> The patch:
> https://source.denx.de/u-boot/u-boot/-/commit/79230640cb4fb780e2be9bb9a47b31976b18cac4
> does fix the issue, please feel free to discard my patch, thank you.

Ok, what about the patch below?

https://patchwork.ozlabs.org/project/uboot/patch/20230703205700.9120-1-cody@londelec.com/

Is this still needed?

Re: [RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

[Fabio Estevam]

On Fri, Dec 15, 2023 at 1:20 PM Fabio Estevam <festevam@gmail.com> wrote:

> Ok, what about the patch below?
>
> https://patchwork.ozlabs.org/project/uboot/patch/20230703205700.9120-1-cody@londelec.com/
>
> Is this still needed?

I looked in the i.MX28 Reference Manual, and the one above looks a
correct fix. I will queue it.

Re: [RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

[Cody Green]

On Fri, Dec 15, 2023 at 7:23 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Fri, Dec 15, 2023 at 1:20 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> > Ok, what about the patch below?
> >
> > https://patchwork.ozlabs.org/project/uboot/patch/20230703205700.9120-1-cody@londelec.com/
> >
> > Is this still needed?
>
> I looked in the i.MX28 Reference Manual, and the one above looks a
> correct fix. I will queue it.

Yes, this patch is needed, thank you for queuing it.