diff mbox series

question: mx7ulp - LDO_ENABLED_MODE

Message ID CAOMZO5CREf4SbdHR_ReBtp2eKNUxKS-JkQBiezwyF4ooATK-iA@mail.gmail.com
State New
Headers show
Series question: mx7ulp - LDO_ENABLED_MODE | expand

Commit Message

Fabio Estevam Jan. 16, 2020, 9:33 p.m. UTC 
Hi Jorge,

On Thu, Jan 16, 2020 at 5:30 PM Jorge Ramirez-Ortiz, Foundries
<jorge at foundries.io> wrote:
>
> Hi Fabio,
>
> I am trying to enable LDO in an imx7ulp based board but somehow the
> board locks up as soon I  write to PMC1_RUN (using the init_ldo_mode
> sequence).

Just looked at the i.MX7UL Reference Manual and it says:

"28.5.9.1.1 Using internal LDO regulator
After a POR event, when the PMC 0 is running in RUN mode and the PMC 1 is turned
off, the process to turn on the PMC 1 using the internal LDO regulator
is as follows:
• Assert the LDOEN bit (PMC0_CTRL).
• Assert the LDOOKDIS bit (PMC0_CTRL) if required.
• Assert the PMC1ON bit (PMC0_CTRL)."

So it seems we need to change the order to:


Does this help?

> I think it is interesting that bit PMC0_CTRL_PMC1ON is already set so
> I am wondering if you think it is possible - in your experience- that
> ROM might have already configured LDO? or was this also the case -
> this bit already set- when you tested the feature?

I think it was not set by default. I can confirm tomorrow with a
i.MX7ULP Embedded Artists board.

Regards,

Fabio Estevam

Comments

Fabio Estevam Jan. 16, 2020, 9:51 p.m. UTC | #1 
Hi Jorge,

On Thu, Jan 16, 2020 at 6:33 PM Fabio Estevam <festevam at gmail.com> wrote:

> > I think it is interesting that bit PMC0_CTRL_PMC1ON is already set so
> > I am wondering if you think it is possible - in your experience- that
> > ROM might have already configured LDO? or was this also the case -
> > this bit already set- when you tested the feature?

I think I understand the problem now.

We are currently acessing the PMC0 registers, which control the M4 side.

M4 has LDO enabled set by the ROM, so that's why you see it enabled by default.

We need to access the PMC1 registers instead of PMC0.

Could you please test these two patches? (They were only compiled tested)

Regards,

Fabio Estevam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ldofixpmc.patch
Type: text/x-patch
Size: 2437 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200116/6cebbc2d/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-ldofixorder.patch
Type: text/x-patch
Size: 1558 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200116/6cebbc2d/attachment-0001.bin>
Jorge Ramirez-Ortiz, Gmail Jan. 16, 2020, 10:01 p.m. UTC | #2 
On 16/01/20 18:33:17, Fabio Estevam wrote:
> Hi Jorge,
> 
> On Thu, Jan 16, 2020 at 5:30 PM Jorge Ramirez-Ortiz, Foundries
> <jorge at foundries.io> wrote:
> >
> > Hi Fabio,
> >
> > I am trying to enable LDO in an imx7ulp based board but somehow the
> > board locks up as soon I  write to PMC1_RUN (using the init_ldo_mode
> > sequence).
> 
> Just looked at the i.MX7UL Reference Manual and it says:
> 
> "28.5.9.1.1 Using internal LDO regulator
> After a POR event, when the PMC 0 is running in RUN mode and the PMC 1 is turned
> off, the process to turn on the PMC 1 using the internal LDO regulator
> is as follows:
> • Assert the LDOEN bit (PMC0_CTRL).
> • Assert the LDOOKDIS bit (PMC0_CTRL) if required.
> • Assert the PMC1ON bit (PMC0_CTRL)."
> 
> So it seems we need to change the order to:
> 
> --- a/arch/arm/mach-imx/mx7ulp/soc.c
> +++ b/arch/arm/mach-imx/mx7ulp/soc.c
> @@ -122,9 +122,6 @@ static void init_ldo_mode(void)
>  {
>         unsigned int reg;
> 
> -       /* Set LDOOKDIS */
> -       setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_LDOOKDIS);
> -
>         /* Set LDOVL to 0.95V in PMC1_RUN */
>         reg = readl(PMC1_BASE_ADDR + PMC1_RUN);
>         reg &= ~PMC1_LDOVL_MASK;
> @@ -151,6 +148,9 @@ static void init_ldo_mode(void)
>         /* Set LDOEN bit */
>         setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_LDOEN);
> 
> +       /* Set LDOOKDIS */
> +       setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_LDOOKDIS);
> +
>         /* Set the PMC1ON bit */
>         setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_PMC1ON);
>  }
> 
> Does this help?

no, unfortunately the same thing.
I think PMC0_CTRL_PMC1ON should not be on but cant figure out who sets it up.


> 
> > I think it is interesting that bit PMC0_CTRL_PMC1ON is already set so
> > I am wondering if you think it is possible - in your experience- that
> > ROM might have already configured LDO? or was this also the case -
> > this bit already set- when you tested the feature?
> 
> I think it was not set by default. I can confirm tomorrow with a
> i.MX7ULP Embedded Artists board.

that would be awesome. thanks a lot!

> 
> Regards,
> 
> Fabio Estevam
Fabio Estevam Jan. 16, 2020, 10:04 p.m. UTC | #3 
Hi Jorge,

On Thu, Jan 16, 2020 at 6:51 PM Fabio Estevam <festevam at gmail.com> wrote:

> Could you please test these two patches? (They were only compiled tested)

Please discard these patches. Just realized that CTRL is at a
different offset for PMC1.

Please try these instead.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ldofixpmc.patch.patch
Type: text/x-patch
Size: 1890 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200116/941b10ca/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-ldofixorder.patch
Type: text/x-patch
Size: 1566 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200116/941b10ca/attachment-0001.bin>
Jorge Ramirez-Ortiz, Gmail Jan. 16, 2020, 10:24 p.m. UTC | #4 
On 16/01/20 19:04:09, Fabio Estevam wrote:
> Hi Jorge,
> 
> On Thu, Jan 16, 2020 at 6:51 PM Fabio Estevam <festevam at gmail.com> wrote:
> 
> > Could you please test these two patches? (They were only compiled tested)
> 
> Please discard these patches. Just realized that CTRL is at a
> different offset for PMC1.
> 
> Please try these instead.

um still nothing.
will debug more in the morning - will add more debug info.
thanks for the quick responses!

jorge


> From 50d4598ae23c549fe3809bfa5f365364ac36d71b Mon Sep 17 00:00:00 2001
> From: Fabio Estevam <festevam at gmail.com>
> Date: Thu, 16 Jan 2020 18:59:30 -0300
> Subject: [PATCH 1/2] mx7ulp: Fix the PMC register set
> 
> On i.MX7ULP the PMC0 registers control the M4 side and
> the PMC1 controls the A7 side.
> 
> In order to enable the A7 LDO-enabled mode the PMC1 registers
> need to configured instead.
> 
> Signed-off-by: Fabio Estevam <festevam at gmail.com>
> ---
>  arch/arm/mach-imx/mx7ulp/soc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/mx7ulp/soc.c b/arch/arm/mach-imx/mx7ulp/soc.c
> index 8345b01398..481cfe226a 100644
> --- a/arch/arm/mach-imx/mx7ulp/soc.c
> +++ b/arch/arm/mach-imx/mx7ulp/soc.c
> @@ -16,6 +16,7 @@
>  #define PMC0_CTRL_LDOOKDIS	BIT(30)
>  #define PMC0_CTRL_PMC1ON	BIT(24)
>  #define PMC1_BASE_ADDR		0x40400000
> +#define PMC1_CTRL		0x24
>  #define PMC1_RUN		0x8
>  #define PMC1_STOP		0x10
>  #define PMC1_VLPS		0x14
> @@ -123,7 +124,7 @@ static void init_ldo_mode(void)
>  	unsigned int reg;
>  
>  	/* Set LDOOKDIS */
> -	setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_LDOOKDIS);
> +	setbits_le32(PMC1_BASE_ADDR + PMC1_CTRL, PMC0_CTRL_LDOOKDIS);
>  
>  	/* Set LDOVL to 0.95V in PMC1_RUN */
>  	reg = readl(PMC1_BASE_ADDR + PMC1_RUN);
> @@ -149,10 +150,10 @@ static void init_ldo_mode(void)
>  	writel(PMC1_BASE_ADDR + PMC1_VLPS, reg);
>  
>  	/* Set LDOEN bit */
> -	setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_LDOEN);
> +	setbits_le32(PMC1_BASE_ADDR + PMC1_CTRL, PMC0_CTRL_LDOEN);
>  
>  	/* Set the PMC1ON bit */
> -	setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_PMC1ON);
> +	setbits_le32(PMC1_BASE_ADDR + PMC1_CTRL, PMC0_CTRL_PMC1ON);
>  }
>  #endif
>  
> @@ -198,7 +199,7 @@ static bool ldo_mode_is_enabled(void)
>  {
>  	unsigned int reg;
>  
> -	reg = readl(PMC0_BASE_ADDR + PMC0_CTRL);
> +	reg = readl(PMC1_BASE_ADDR + PMC1_CTRL);
>  	if (reg & PMC0_CTRL_LDOEN)
>  		return true;
>  	else
> -- 
> 2.17.1
> 

> From d1a07fb70610184c042df7f593dd0ff8302235c8 Mon Sep 17 00:00:00 2001
> From: Fabio Estevam <festevam at gmail.com>
> Date: Thu, 16 Jan 2020 19:00:23 -0300
> Subject: [PATCH 2/2] mx7ulp: Fix the order for enabling LDO
> 
> As per the i.MX7ULP Reference Manual:
> 
> "28.5.9.1.1 Using internal LDO regulator
> After a POR event, when the PMC 0 is running in RUN mode and the PMC 1 is turned
> off, the process to turn on the PMC 1 using the internal LDO regulator
> is as follows:
> - Assert the LDOEN bit (PMC0_CTRL).
> - Assert the LDOOKDIS bit (PMC0_CTRL) if required.
> - Assert the PMC1ON bit (PMC0_CTRL)."
> 
> So follow the recommended intialization order.
> 
> Signed-off-by: Fabio Estevam <festevam at gmail.com>
> ---
>  arch/arm/mach-imx/mx7ulp/soc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/mx7ulp/soc.c b/arch/arm/mach-imx/mx7ulp/soc.c
> index 481cfe226a..9b114a8604 100644
> --- a/arch/arm/mach-imx/mx7ulp/soc.c
> +++ b/arch/arm/mach-imx/mx7ulp/soc.c
> @@ -123,9 +123,6 @@ static void init_ldo_mode(void)
>  {
>  	unsigned int reg;
>  
> -	/* Set LDOOKDIS */
> -	setbits_le32(PMC1_BASE_ADDR + PMC1_CTRL, PMC0_CTRL_LDOOKDIS);
> -
>  	/* Set LDOVL to 0.95V in PMC1_RUN */
>  	reg = readl(PMC1_BASE_ADDR + PMC1_RUN);
>  	reg &= ~PMC1_LDOVL_MASK;
> @@ -152,6 +149,9 @@ static void init_ldo_mode(void)
>  	/* Set LDOEN bit */
>  	setbits_le32(PMC1_BASE_ADDR + PMC1_CTRL, PMC0_CTRL_LDOEN);
>  
> +	/* Set LDOOKDIS */
> +	setbits_le32(PMC1_BASE_ADDR + PMC1_CTRL, PMC0_CTRL_LDOOKDIS);
> +
>  	/* Set the PMC1ON bit */
>  	setbits_le32(PMC1_BASE_ADDR + PMC1_CTRL, PMC0_CTRL_PMC1ON);
>  }
> -- 
> 2.17.1
>
Fabio Estevam Jan. 16, 2020, 10:38 p.m. UTC | #5 
On Thu, Jan 16, 2020 at 7:24 PM Jorge Ramirez-Ortiz, Gmail
<jorge.ramirez.ortiz at gmail.com> wrote:

> um still nothing.
> will debug more in the morning - will add more debug info.
> thanks for the quick responses!

Ok, just tested mainline U-Boot (without any changes) on my i.MX7ULP boards.

On mx7ulp_evk I see:

PMC1:  LDO bypass mode

and on mx7ul_com board I see:

PMC1:  LDO enabled mode

The code I did to set and check LDO enabled is according to the RM and
it seems to behave properly on these two boards.

Let me know if you make any progress.

Fabio Estevam
Jorge Ramirez-Ortiz, Gmail Jan. 17, 2020, 9:24 a.m. UTC | #6 
On 16/01/20 19:38:39, Fabio Estevam wrote:
> On Thu, Jan 16, 2020 at 7:24 PM Jorge Ramirez-Ortiz, Gmail
> <jorge.ramirez.ortiz at gmail.com> wrote:
> 
> > um still nothing.
> > will debug more in the morning - will add more debug info.
> > thanks for the quick responses!
> 
> Ok, just tested mainline U-Boot (without any changes) on my i.MX7ULP boards.
> 
> On mx7ulp_evk I see:
> 
> PMC1:  LDO bypass mode
> 
> and on mx7ul_com board I see:
> 
> PMC1:  LDO enabled mode

right but is the initialization code being executed? please could you check?
I also see that message on my board but only as long as the ldo init does not get called.

> 
> The code I did to set and check LDO enabled is according to the RM and
> it seems to behave properly on these two boards.
> 
> Let me know if you make any progress.
> 
> Fabio Estevam
diff mbox series

Patch

--- a/arch/arm/mach-imx/mx7ulp/soc.c
+++ b/arch/arm/mach-imx/mx7ulp/soc.c
@@ -122,9 +122,6 @@  static void init_ldo_mode(void)
 {
        unsigned int reg;

-       /* Set LDOOKDIS */
-       setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_LDOOKDIS);
-
        /* Set LDOVL to 0.95V in PMC1_RUN */
        reg = readl(PMC1_BASE_ADDR + PMC1_RUN);
        reg &= ~PMC1_LDOVL_MASK;
@@ -151,6 +148,9 @@  static void init_ldo_mode(void)
        /* Set LDOEN bit */
        setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_LDOEN);

+       /* Set LDOOKDIS */
+       setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_LDOOKDIS);
+
        /* Set the PMC1ON bit */
        setbits_le32(PMC0_BASE_ADDR + PMC0_CTRL, PMC0_CTRL_PMC1ON);
 }