Patchwork [U-Boot] i.MX28: bug fixes in PMU configuration code

login
register
mail settings
Submitter Stathis Voukelatos
Date June 27, 2012, 12:30 p.m.
Message ID <63F28649326B5F45BF5E97E219B5508138E62D2A@lhq-exchange.linn.co.uk>
Download mbox | patch
Permalink /patch/167646/
State Superseded
Headers show

Comments

Stathis Voukelatos - June 27, 2012, 12:30 p.m.
Fixed some typos in the i.MX28 PMU code that sets up the VDDD
and VDDIO power rails. In addition the VDDD and VDDIO brownout 
offset values should be divided by a step size before being 
programmed to the corresponding registers.

Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Marek Vasut <marek.vasut@gmail.com>
---
 arch/arm/cpu/arm926ejs/mx28/spl_power_init.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)
Marek Vasut - July 8, 2012, 7:15 p.m.
Dear Stathis Voukelatos,

> Fixed some typos in the i.MX28 PMU code that sets up the VDDD
> and VDDIO power rails. In addition the VDDD and VDDIO brownout
> offset values should be divided by a step size before being
> programmed to the corresponding registers.
> 
> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Marek Vasut <marek.vasut@gmail.com>

All right, yet another minus point for my responsiveness :-(

Tested-by: Marek Vasut <marex@denx.de>
Acked-by: Marek Vasut <marex@denx.de>

Fabio, can you give it a quick spin on mx28evk so we can roll this late into 
this release maybe ?

> ---
>  arch/arm/cpu/arm926ejs/mx28/spl_power_init.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
> b/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c index 4b09b0c..cc71af8
> 100644
> --- a/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
> +++ b/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
> @@ -716,7 +716,7 @@ int mx28_get_vddio_power_source_off(void)
>                 tmp = readl(&power_regs->hw_power_vddioctrl);
>                 if (tmp & POWER_VDDIOCTRL_DISABLE_FET) {
>                         if ((tmp & POWER_VDDIOCTRL_LINREG_OFFSET_MASK) ==
> -                               POWER_VDDDCTRL_LINREG_OFFSET_0STEPS) {
> +                               POWER_VDDIOCTRL_LINREG_OFFSET_0STEPS) {
>                                 return 1;
>                         }
>                 }
> @@ -724,7 +724,7 @@ int mx28_get_vddio_power_source_off(void)
>                 if (!(readl(&power_regs->hw_power_5vctrl) &
>                         POWER_5VCTRL_ENABLE_DCDC)) {
>                         if ((tmp & POWER_VDDIOCTRL_LINREG_OFFSET_MASK) ==
> -                               POWER_VDDDCTRL_LINREG_OFFSET_0STEPS) {
> +                               POWER_VDDIOCTRL_LINREG_OFFSET_0STEPS) {
>                                 return 1;
>                         }
>                 }
> @@ -772,7 +772,7 @@ void mx28_power_set_vddio(uint32_t new_target, uint32_t
> new_brownout) uint32_t cur_target, diff, bo_int = 0;
>         uint32_t powered_by_linreg = 0;
> 
> -       new_brownout = new_target - new_brownout;
> +       new_brownout = (new_target - new_brownout + 25) / 50;
> 
>         cur_target = readl(&power_regs->hw_power_vddioctrl);
>         cur_target &= POWER_VDDIOCTRL_TRG_MASK;
> @@ -858,8 +858,8 @@ void mx28_power_set_vddio(uint32_t new_target, uint32_t
> new_brownout) }
> 
>         clrsetbits_le32(&power_regs->hw_power_vddioctrl,
> -                       POWER_VDDDCTRL_BO_OFFSET_MASK,
> -                       new_brownout << POWER_VDDDCTRL_BO_OFFSET_OFFSET);
> +                       POWER_VDDIOCTRL_BO_OFFSET_MASK,
> +                       new_brownout << POWER_VDDIOCTRL_BO_OFFSET_OFFSET);
>  }
> 
>  void mx28_power_set_vddd(uint32_t new_target, uint32_t new_brownout)
> @@ -869,7 +869,7 @@ void mx28_power_set_vddd(uint32_t new_target, uint32_t
> new_brownout) uint32_t cur_target, diff, bo_int = 0;
>         uint32_t powered_by_linreg = 0;
> 
> -       new_brownout = new_target - new_brownout;
> +       new_brownout = (new_target - new_brownout + 12) / 25;
> 
>         cur_target = readl(&power_regs->hw_power_vdddctrl);
>         cur_target &= POWER_VDDDCTRL_TRG_MASK;

Best regards,
Marek Vasut
Fabio Estevam - July 8, 2012, 7:43 p.m.
Hi Marek,

On Sun, Jul 8, 2012 at 4:15 PM, Marek Vasut <marek.vasut@gmail.com> wrote:

> Fabio, can you give it a quick spin on mx28evk so we can roll this late into
> this release maybe ?

I don't have access to my mx28evk now and will be out of the office in
the following 2 weeks.

Maybe Otavio could test it on a mx28evk?
Marek Vasut - July 19, 2012, 5:14 p.m.
Dear Stathis Voukelatos,

> Fixed some typos in the i.MX28 PMU code that sets up the VDDD
> and VDDIO power rails. In addition the VDDD and VDDIO brownout
> offset values should be divided by a step size before being
> programmed to the corresponding registers.
> 
> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Marek Vasut <marek.vasut@gmail.com>

Stefano, I think this should be applied.

THanks!

Best regards,
Marek Vasut
Stefano Babic - July 20, 2012, 10:05 a.m.
On 19/07/2012 19:14, Marek Vasut wrote:
> Dear Stathis Voukelatos,
> 
>> Fixed some typos in the i.MX28 PMU code that sets up the VDDD
>> and VDDIO power rails. In addition the VDDD and VDDIO brownout
>> offset values should be divided by a step size before being
>> programmed to the corresponding registers.
>>
>> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Marek Vasut <marek.vasut@gmail.com>
> 
> Stefano, I think this should be applied.

There are checkpatch errors that should be fixed before applying.
Stathis, can you check them and resubmit ? Thanks.

Best regards,
Stefano Babic
Marek Vasut - July 20, 2012, 11:43 a.m.
Dear Stefano Babic,

> On 19/07/2012 19:14, Marek Vasut wrote:
> > Dear Stathis Voukelatos,
> > 
> >> Fixed some typos in the i.MX28 PMU code that sets up the VDDD
> >> and VDDIO power rails. In addition the VDDD and VDDIO brownout
> >> offset values should be divided by a step size before being
> >> programmed to the corresponding registers.
> >> 
> >> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> >> Cc: Stefano Babic <sbabic@denx.de>
> >> Cc: Marek Vasut <marek.vasut@gmail.com>
> > 
> > Stefano, I think this should be applied.
> 
> There are checkpatch errors that should be fixed before applying.
> Stathis, can you check them and resubmit ? Thanks.

I can eventually resubmit if Stathis grew unhappy about the slow response time 
:-(

Best regards,
Marek Vasut

Patch

diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
index 4b09b0c..cc71af8 100644
--- a/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
+++ b/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
@@ -716,7 +716,7 @@  int mx28_get_vddio_power_source_off(void)
                tmp = readl(&power_regs->hw_power_vddioctrl);
                if (tmp & POWER_VDDIOCTRL_DISABLE_FET) {
                        if ((tmp & POWER_VDDIOCTRL_LINREG_OFFSET_MASK) ==
-                               POWER_VDDDCTRL_LINREG_OFFSET_0STEPS) {
+                               POWER_VDDIOCTRL_LINREG_OFFSET_0STEPS) {
                                return 1;
                        }
                }
@@ -724,7 +724,7 @@  int mx28_get_vddio_power_source_off(void)
                if (!(readl(&power_regs->hw_power_5vctrl) &
                        POWER_5VCTRL_ENABLE_DCDC)) {
                        if ((tmp & POWER_VDDIOCTRL_LINREG_OFFSET_MASK) ==
-                               POWER_VDDDCTRL_LINREG_OFFSET_0STEPS) {
+                               POWER_VDDIOCTRL_LINREG_OFFSET_0STEPS) {
                                return 1;
                        }
                }
@@ -772,7 +772,7 @@  void mx28_power_set_vddio(uint32_t new_target, uint32_t new_brownout)
        uint32_t cur_target, diff, bo_int = 0;
        uint32_t powered_by_linreg = 0;
 
-       new_brownout = new_target - new_brownout;
+       new_brownout = (new_target - new_brownout + 25) / 50;
 
        cur_target = readl(&power_regs->hw_power_vddioctrl);
        cur_target &= POWER_VDDIOCTRL_TRG_MASK;
@@ -858,8 +858,8 @@  void mx28_power_set_vddio(uint32_t new_target, uint32_t new_brownout)
        }
 
        clrsetbits_le32(&power_regs->hw_power_vddioctrl,
-                       POWER_VDDDCTRL_BO_OFFSET_MASK,
-                       new_brownout << POWER_VDDDCTRL_BO_OFFSET_OFFSET);
+                       POWER_VDDIOCTRL_BO_OFFSET_MASK,
+                       new_brownout << POWER_VDDIOCTRL_BO_OFFSET_OFFSET);
 }
 
 void mx28_power_set_vddd(uint32_t new_target, uint32_t new_brownout)
@@ -869,7 +869,7 @@  void mx28_power_set_vddd(uint32_t new_target, uint32_t new_brownout)
        uint32_t cur_target, diff, bo_int = 0;
        uint32_t powered_by_linreg = 0;
 
-       new_brownout = new_target - new_brownout;
+       new_brownout = (new_target - new_brownout + 12) / 25;
 
        cur_target = readl(&power_regs->hw_power_vdddctrl);
        cur_target &= POWER_VDDDCTRL_TRG_MASK;