diff mbox

[U-Boot] arm: mx23: Fix VDDMEM misconfiguration

Message ID 1367194665-9813-1-git-send-email-marex@denx.de
State Awaiting Upstream
Delegated to: Stefano Babic
Headers show

Commit Message

Marek Vasut April 29, 2013, 12:17 a.m. UTC
The VDDMEM ramped up in very weird way as it was horribly misconfigured.
Instead of setting up VDDMEM in one swipe, let it rise slowly the same
way as VDDD and VDDA in spl_power_init.c and then only clear ILIMIT before
memory gets inited. This makes sure the VDDMEM rises sanely, not jumps up
and down as it did till now.

The VDDMEM prior to this change did this:
     2V0____   .--------2V5
       |    `--'
 0V____|

The VDDMEM now does this:
    2V0_____,-----------2V5
      /
 0V__|

Moreover, VDDIO on MX23 uses 25mV steps while MX28 uses 50mV steps,
fix this difference too.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Otavio Salvador <otavio@ossystems.com.br>
Cc: Stefano Babic <sbabic@denx.de>
---
 arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c   |   12 ++-----
 arch/arm/cpu/arm926ejs/mxs/spl_power_init.c |   50 +++++++++++++++++++++------
 2 files changed, 42 insertions(+), 20 deletions(-)

Comments

Marek Vasut April 29, 2013, 3:04 a.m. UTC | #1
Dear Marek Vasut,

> The VDDMEM ramped up in very weird way as it was horribly misconfigured.
> Instead of setting up VDDMEM in one swipe, let it rise slowly the same
> way as VDDD and VDDA in spl_power_init.c and then only clear ILIMIT before
> memory gets inited. This makes sure the VDDMEM rises sanely, not jumps up
> and down as it did till now.
> 
> The VDDMEM prior to this change did this:
>      2V0____   .--------2V5
> 
>        |    `--'
> 
>  0V____|
> 
> The VDDMEM now does this:
>     2V0_____,-----------2V5
>       /
>  0V__|
> 
> Moreover, VDDIO on MX23 uses 25mV steps while MX28 uses 50mV steps,
> fix this difference too.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>

FYI, with this patch on top of stock 2013.04, the whole boot process :

http://twilight.ponies.cz/MX23-VDDALL.png

Best regards,
Marek Vasut
Otavio Salvador April 29, 2013, 12:30 p.m. UTC | #2
Hello Marek,

(adding Kiril in Cc as he been quite friendly in testing U-Boot in OlinuXino)

On Sun, Apr 28, 2013 at 9:17 PM, Marek Vasut <marex@denx.de> wrote:
> The VDDMEM ramped up in very weird way as it was horribly misconfigured.
> Instead of setting up VDDMEM in one swipe, let it rise slowly the same
> way as VDDD and VDDA in spl_power_init.c and then only clear ILIMIT before
> memory gets inited. This makes sure the VDDMEM rises sanely, not jumps up
> and down as it did till now.
>
> The VDDMEM prior to this change did this:
>      2V0____   .--------2V5
>        |    `--'
>  0V____|
>
> The VDDMEM now does this:
>     2V0_____,-----------2V5
>       /
>  0V__|
>
> Moreover, VDDIO on MX23 uses 25mV steps while MX28 uses 50mV steps,
> fix this difference too.
>
> Signed-off-by: Marek Vasut <marex@denx.de>

Can you rebase it on top of imx branch? it does not apply there.

Using your patch (after fixing the merge issue) it does not fix the
memory issue. Did you test it against OlinuXino and checked memory? it
seems to not fix the memory issue. Setting it to 2V6 seems to improve
it but still does not fix the issue.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Kiril Zyapkov April 29, 2013, 1:14 p.m. UTC | #3
Hello everyone,

On 29/04/13 15:30, Otavio Salvador wrote:
> Hello Marek,
>
> (adding Kiril in Cc as he been quite friendly in testing U-Boot in
> OlinuXino)

Per Otavio's instructions:
   * checked out git://git.denx.de/u-boot-imx.git, branch master
   * applied http://pastebin.com/qub5fP1P (skips writing to some
registers on mx23)
   * applied this patch. The conflict in mx23_mem_setup_vddmem() was
resolved by using Marek's code.
   * changed VDDMEM form 2V5 to 2V6

The complete changeset: http://sprunge.us/AHAU

This was tested on an imx233-olinuxino-maxi, bootlog: http://sprunge.us/RIhf

Without the patch skipping registers, the console output gets scrambled.

Thanks,

Kiril
Marek Vasut April 29, 2013, 3:40 p.m. UTC | #4
Dear Kiril Zyapkov,

> Hello everyone,
> 
> On 29/04/13 15:30, Otavio Salvador wrote:
> > Hello Marek,
> > 
> > (adding Kiril in Cc as he been quite friendly in testing U-Boot in
> > OlinuXino)
> 
> Per Otavio's instructions:
>    * checked out git://git.denx.de/u-boot-imx.git, branch master
>    * applied http://pastebin.com/qub5fP1P (skips writing to some
> registers on mx23)
>    * applied this patch. The conflict in mx23_mem_setup_vddmem() was
> resolved by using Marek's code.
>    * changed VDDMEM form 2V5 to 2V6
> 
> The complete changeset: http://sprunge.us/AHAU
> 
> This was tested on an imx233-olinuxino-maxi, bootlog:
> http://sprunge.us/RIhf
> 
> Without the patch skipping registers, the console output gets scrambled.

Ok, so we have issue in spl_mem_init.c for MX23.

Best regards,
Marek Vasut
Fabio Estevam April 29, 2013, 3:45 p.m. UTC | #5
Hi Marek,

On Mon, Apr 29, 2013 at 12:40 PM, Marek Vasut <marex@denx.de> wrote:

>> Without the patch skipping registers, the console output gets scrambled.
>
> Ok, so we have issue in spl_mem_init.c for MX23.

This is my thought as well.

I sent the "skipping registers" to Kiril after comparing U-boot code
against the bootlets. Will post later to the list.

Another important thing we currently miss in U-boot is the setting of
drive strength of DDR pins as done in bootlets.

Will try to work on this as soon as time permits.

Regards,

Fabio Estevam
Marek Vasut April 29, 2013, 5:19 p.m. UTC | #6
Dear Fabio Estevam,

> Hi Marek,
> 
> On Mon, Apr 29, 2013 at 12:40 PM, Marek Vasut <marex@denx.de> wrote:
> >> Without the patch skipping registers, the console output gets scrambled.
> > 
> > Ok, so we have issue in spl_mem_init.c for MX23.
> 
> This is my thought as well.
> 
> I sent the "skipping registers" to Kiril after comparing U-boot code
> against the bootlets. Will post later to the list.
> 
> Another important thing we currently miss in U-boot is the setting of
> drive strength of DDR pins as done in bootlets.
> 
> Will try to work on this as soon as time permits.

This should be done in board/..../mx23evk/spl_boot.c alongside iomux, no?

Best regards,
Marek Vasut
Fabio Estevam April 29, 2013, 7:11 p.m. UTC | #7
Hi Marek,

On Mon, Apr 29, 2013 at 2:19 PM, Marek Vasut <marex@denx.de> wrote:

> This should be done in board/..../mx23evk/spl_boot.c alongside iomux, no?

Yes, but the end result of DDR IOMUX settings are not correct in this file.

Let's take HW_PINCTRL_DRIVE9 register for example:

=> md.l 0x80018290
80018290: 77777770 77777770 77777770 77777770

,which does not match what bootlets and the mx23 reference manual recommend.

Bits 30/26/22/18/14/10/6 should be 0 instead of 1 (1 is reserved).

Regards,

Fabio Estevam
Marek Vasut May 5, 2013, 9:28 p.m. UTC | #8
Hi Stefano,

this one too ;-)

> The VDDMEM ramped up in very weird way as it was horribly misconfigured.
> Instead of setting up VDDMEM in one swipe, let it rise slowly the same
> way as VDDD and VDDA in spl_power_init.c and then only clear ILIMIT before
> memory gets inited. This makes sure the VDDMEM rises sanely, not jumps up
> and down as it did till now.
> 
> The VDDMEM prior to this change did this:
>      2V0____   .--------2V5
> 
>        |    `--'
> 
>  0V____|
> 
> The VDDMEM now does this:
>     2V0_____,-----------2V5
>       /
>  0V__|
> 
> Moreover, VDDIO on MX23 uses 25mV steps while MX28 uses 50mV steps,
> fix this difference too.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c   |   12 ++-----
>  arch/arm/cpu/arm926ejs/mxs/spl_power_init.c |   50
> +++++++++++++++++++++------ 2 files changed, 42 insertions(+), 20
> deletions(-)
> 
> diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index bc2d69c..19dd8fb 100644
> --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> @@ -234,17 +234,9 @@ static void mx23_mem_setup_vddmem(void)
>  	struct mxs_power_regs *power_regs =
>  		(struct mxs_power_regs *)MXS_POWER_BASE;
> 
> -	writel((0x10 << POWER_VDDMEMCTRL_TRG_OFFSET) |
> -		POWER_VDDMEMCTRL_ENABLE_ILIMIT |
> -		POWER_VDDMEMCTRL_ENABLE_LINREG |
> -		POWER_VDDMEMCTRL_PULLDOWN_ACTIVE,
> -		&power_regs->hw_power_vddmemctrl);
> +	clrbits_le32(&power_regs->hw_power_vddmemctrl,
> +		POWER_VDDMEMCTRL_ENABLE_ILIMIT);
> 
> -	early_delay(10000);
> -
> -	writel((0x10 << POWER_VDDMEMCTRL_TRG_OFFSET) |
> -		POWER_VDDMEMCTRL_ENABLE_LINREG,
> -		&power_regs->hw_power_vddmemctrl);
>  }
> 
>  static void mx23_mem_init(void)
> diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
> b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 287c698..21cac7b
> 100644
> --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
> +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
> @@ -687,6 +687,12 @@ static void mxs_power_configure_power_source(void)
>  	mxs_init_batt_bo();
> 
>  	mxs_switch_vddd_to_dcdc_source();
> +
> +#ifdef CONFIG_MX23
> +	/* Fire up the VDDMEM LinReg now that we're all set. */
> +	writel(POWER_VDDMEMCTRL_ENABLE_LINREG | POWER_VDDMEMCTRL_ENABLE_ILIMIT,
> +		&power_regs->hw_power_vddmemctrl);
> +#endif
>  }
> 
>  static void mxs_enable_output_rail_protection(void)
> @@ -781,7 +787,11 @@ struct mxs_vddx_cfg {
>  static const struct mxs_vddx_cfg mxs_vddio_cfg = {
>  	.reg			= &(((struct mxs_power_regs *)MXS_POWER_BASE)->
>  					hw_power_vddioctrl),
> +#if defined(CONFIG_MX23)
> +	.step_mV		= 25,
> +#else
>  	.step_mV		= 50,
> +#endif
>  	.lowest_mV		= 2800,
>  	.powered_by_linreg	= mxs_get_vddio_power_source_off,
>  	.trg_mask		= POWER_VDDIOCTRL_TRG_MASK,
> @@ -804,6 +814,21 @@ static const struct mxs_vddx_cfg mxs_vddd_cfg = {
>  	.bo_offset_offset	= POWER_VDDDCTRL_BO_OFFSET_OFFSET,
>  };
> 
> +#ifdef CONFIG_MX23
> +static const struct mxs_vddx_cfg mxs_vddmem_cfg = {
> +	.reg			= &(((struct mxs_power_regs *)MXS_POWER_BASE)->
> +					hw_power_vddmemctrl),
> +	.step_mV		= 50,
> +	.lowest_mV		= 1700,
> +	.powered_by_linreg	= NULL,
> +	.trg_mask		= POWER_VDDMEMCTRL_TRG_MASK,
> +	.bo_irq			= 0,
> +	.bo_enirq		= 0,
> +	.bo_offset_mask		= 0,
> +	.bo_offset_offset	= 0,
> +};
> +#endif
> +
>  static void mxs_power_set_vddx(const struct mxs_vddx_cfg *cfg,
>  				uint32_t new_target, uint32_t new_brownout)
>  {
> @@ -821,9 +846,10 @@ static void mxs_power_set_vddx(const struct
> mxs_vddx_cfg *cfg, cur_target += cfg->lowest_mV;
> 
>  	adjust_up = new_target > cur_target;
> -	powered_by_linreg = cfg->powered_by_linreg();
> +	if (cfg->powered_by_linreg)
> +		powered_by_linreg = cfg->powered_by_linreg();
> 
> -	if (adjust_up) {
> +	if (adjust_up && cfg->bo_irq) {
>  		if (powered_by_linreg) {
>  			bo_int = readl(cfg->reg);
>  			clrbits_le32(cfg->reg, cfg->bo_enirq);
> @@ -864,14 +890,16 @@ static void mxs_power_set_vddx(const struct
> mxs_vddx_cfg *cfg, cur_target += cfg->lowest_mV;
>  	} while (new_target > cur_target);
> 
> -	if (adjust_up && powered_by_linreg) {
> -		writel(cfg->bo_irq, &power_regs->hw_power_ctrl_clr);
> -		if (bo_int & cfg->bo_enirq)
> -			setbits_le32(cfg->reg, cfg->bo_enirq);
> -	}
> +	if (cfg->bo_irq) {
> +		if (adjust_up && powered_by_linreg) {
> +			writel(cfg->bo_irq, &power_regs->hw_power_ctrl_clr);
> +			if (bo_int & cfg->bo_enirq)
> +				setbits_le32(cfg->reg, cfg->bo_enirq);
> +		}
> 
> -	clrsetbits_le32(cfg->reg, cfg->bo_offset_mask,
> -			new_brownout << cfg->bo_offset_offset);
> +		clrsetbits_le32(cfg->reg, cfg->bo_offset_mask,
> +				new_brownout << cfg->bo_offset_offset);
> +	}
>  }
> 
>  static void mxs_setup_batt_detect(void)
> @@ -910,7 +938,9 @@ void mxs_power_init(void)
> 
>  	mxs_power_set_vddx(&mxs_vddio_cfg, 3300, 3150);
>  	mxs_power_set_vddx(&mxs_vddd_cfg, 1500, 1000);
> -
> +#ifdef CONFIG_MX23
> +	mxs_power_set_vddx(&mxs_vddmem_cfg, 2500, 1700);
> +#endif
>  	writel(POWER_CTRL_VDDD_BO_IRQ | POWER_CTRL_VDDA_BO_IRQ |
>  		POWER_CTRL_VDDIO_BO_IRQ | POWER_CTRL_VDD5V_DROOP_IRQ |
>  		POWER_CTRL_VBUS_VALID_IRQ | POWER_CTRL_BATT_BO_IRQ |

Best regards,
Marek Vasut
Stefano Babic May 6, 2013, 8:22 a.m. UTC | #9
On 29/04/2013 02:17, Marek Vasut wrote:
> The VDDMEM ramped up in very weird way as it was horribly misconfigured.
> Instead of setting up VDDMEM in one swipe, let it rise slowly the same
> way as VDDD and VDDA in spl_power_init.c and then only clear ILIMIT before
> memory gets inited. This makes sure the VDDMEM rises sanely, not jumps up
> and down as it did till now.
> 
> The VDDMEM prior to this change did this:
>      2V0____   .--------2V5
>        |    `--'
>  0V____|
> 
> The VDDMEM now does this:
>     2V0_____,-----------2V5
>       /
>  0V__|
> 
> Moreover, VDDIO on MX23 uses 25mV steps while MX28 uses 50mV steps,
> fix this difference too.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>
> ---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic
diff mbox

Patch

diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
index bc2d69c..19dd8fb 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
@@ -234,17 +234,9 @@  static void mx23_mem_setup_vddmem(void)
 	struct mxs_power_regs *power_regs =
 		(struct mxs_power_regs *)MXS_POWER_BASE;
 
-	writel((0x10 << POWER_VDDMEMCTRL_TRG_OFFSET) |
-		POWER_VDDMEMCTRL_ENABLE_ILIMIT |
-		POWER_VDDMEMCTRL_ENABLE_LINREG |
-		POWER_VDDMEMCTRL_PULLDOWN_ACTIVE,
-		&power_regs->hw_power_vddmemctrl);
+	clrbits_le32(&power_regs->hw_power_vddmemctrl,
+		POWER_VDDMEMCTRL_ENABLE_ILIMIT);
 
-	early_delay(10000);
-
-	writel((0x10 << POWER_VDDMEMCTRL_TRG_OFFSET) |
-		POWER_VDDMEMCTRL_ENABLE_LINREG,
-		&power_regs->hw_power_vddmemctrl);
 }
 
 static void mx23_mem_init(void)
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
index 287c698..21cac7b 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
@@ -687,6 +687,12 @@  static void mxs_power_configure_power_source(void)
 	mxs_init_batt_bo();
 
 	mxs_switch_vddd_to_dcdc_source();
+
+#ifdef CONFIG_MX23
+	/* Fire up the VDDMEM LinReg now that we're all set. */
+	writel(POWER_VDDMEMCTRL_ENABLE_LINREG | POWER_VDDMEMCTRL_ENABLE_ILIMIT,
+		&power_regs->hw_power_vddmemctrl);
+#endif
 }
 
 static void mxs_enable_output_rail_protection(void)
@@ -781,7 +787,11 @@  struct mxs_vddx_cfg {
 static const struct mxs_vddx_cfg mxs_vddio_cfg = {
 	.reg			= &(((struct mxs_power_regs *)MXS_POWER_BASE)->
 					hw_power_vddioctrl),
+#if defined(CONFIG_MX23)
+	.step_mV		= 25,
+#else
 	.step_mV		= 50,
+#endif
 	.lowest_mV		= 2800,
 	.powered_by_linreg	= mxs_get_vddio_power_source_off,
 	.trg_mask		= POWER_VDDIOCTRL_TRG_MASK,
@@ -804,6 +814,21 @@  static const struct mxs_vddx_cfg mxs_vddd_cfg = {
 	.bo_offset_offset	= POWER_VDDDCTRL_BO_OFFSET_OFFSET,
 };
 
+#ifdef CONFIG_MX23
+static const struct mxs_vddx_cfg mxs_vddmem_cfg = {
+	.reg			= &(((struct mxs_power_regs *)MXS_POWER_BASE)->
+					hw_power_vddmemctrl),
+	.step_mV		= 50,
+	.lowest_mV		= 1700,
+	.powered_by_linreg	= NULL,
+	.trg_mask		= POWER_VDDMEMCTRL_TRG_MASK,
+	.bo_irq			= 0,
+	.bo_enirq		= 0,
+	.bo_offset_mask		= 0,
+	.bo_offset_offset	= 0,
+};
+#endif
+
 static void mxs_power_set_vddx(const struct mxs_vddx_cfg *cfg,
 				uint32_t new_target, uint32_t new_brownout)
 {
@@ -821,9 +846,10 @@  static void mxs_power_set_vddx(const struct mxs_vddx_cfg *cfg,
 	cur_target += cfg->lowest_mV;
 
 	adjust_up = new_target > cur_target;
-	powered_by_linreg = cfg->powered_by_linreg();
+	if (cfg->powered_by_linreg)
+		powered_by_linreg = cfg->powered_by_linreg();
 
-	if (adjust_up) {
+	if (adjust_up && cfg->bo_irq) {
 		if (powered_by_linreg) {
 			bo_int = readl(cfg->reg);
 			clrbits_le32(cfg->reg, cfg->bo_enirq);
@@ -864,14 +890,16 @@  static void mxs_power_set_vddx(const struct mxs_vddx_cfg *cfg,
 		cur_target += cfg->lowest_mV;
 	} while (new_target > cur_target);
 
-	if (adjust_up && powered_by_linreg) {
-		writel(cfg->bo_irq, &power_regs->hw_power_ctrl_clr);
-		if (bo_int & cfg->bo_enirq)
-			setbits_le32(cfg->reg, cfg->bo_enirq);
-	}
+	if (cfg->bo_irq) {
+		if (adjust_up && powered_by_linreg) {
+			writel(cfg->bo_irq, &power_regs->hw_power_ctrl_clr);
+			if (bo_int & cfg->bo_enirq)
+				setbits_le32(cfg->reg, cfg->bo_enirq);
+		}
 
-	clrsetbits_le32(cfg->reg, cfg->bo_offset_mask,
-			new_brownout << cfg->bo_offset_offset);
+		clrsetbits_le32(cfg->reg, cfg->bo_offset_mask,
+				new_brownout << cfg->bo_offset_offset);
+	}
 }
 
 static void mxs_setup_batt_detect(void)
@@ -910,7 +938,9 @@  void mxs_power_init(void)
 
 	mxs_power_set_vddx(&mxs_vddio_cfg, 3300, 3150);
 	mxs_power_set_vddx(&mxs_vddd_cfg, 1500, 1000);
-
+#ifdef CONFIG_MX23
+	mxs_power_set_vddx(&mxs_vddmem_cfg, 2500, 1700);
+#endif
 	writel(POWER_CTRL_VDDD_BO_IRQ | POWER_CTRL_VDDA_BO_IRQ |
 		POWER_CTRL_VDDIO_BO_IRQ | POWER_CTRL_VDD5V_DROOP_IRQ |
 		POWER_CTRL_VBUS_VALID_IRQ | POWER_CTRL_BATT_BO_IRQ |