Re: [PATCH] OMAP2+: hwmod: incorrect computation of autoidle_mask

Re: [PATCH] OMAP2+: hwmod: incorrect computation of autoidle_mask

[Cousson, Benoit]

Hi Tarun,

On 1/3/2011 10:59 PM, DebBarma, Tarun Kanti wrote:
> Autoidle is just a single bit setting across OMAP platforms.
> In _set_module_autoidle() I am seeing 0x3 where the mask is
> computed. I believe this should be 0x1.

Just a minor comment; it looks from this description that you are not 
100% sure it is a bug :-)
You need to refer to the various TRMs, stating that this is indeed a bug 
for all OMAPs!

Adding a "fix" in the subject seems more accurate to describe the patch.
OMAP2+: Fix incorrect computation of autoidle_mask

Since is it harmless for the moment, I think it will probably go for 2.6.39.

> Baseline:
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>
> Tested Info:
> Boot tested on OMAP 2/3/4.
>
> Signed-off-by: Tarun Kanti DebBarma<tarun.kanti@ti.com>
> Acked-by: Rajendra Nayak<rnayak@ti.com>

Acked-by: Benoit Cousson <b-cousson@ti.com>


Thanks,
Benoit

> ---
>   arch/arm/mach-omap2/omap_hwmod.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 5a30658..da49b56 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -373,7 +373,7 @@ static int _set_module_autoidle(struct omap_hwmod *oh, u8 autoidle,
>   	}
>
>   	autoidle_shift = oh->class->sysc->sysc_fields->autoidle_shift;
> -	autoidle_mask = (0x3<<  autoidle_shift);
> +	autoidle_mask = (0x1<<  autoidle_shift);
>
>   	*v&= ~autoidle_mask;
>   	*v |= autoidle<<  autoidle_shift;

[PATCH] OMAP2+: hwmod: incorrect computation of autoidle_mask

[Tarun Kanti DebBarma]

Autoidle is just a single bit setting across OMAP platforms.
In _set_module_autoidle() I am seeing 0x3 where the mask is
computed. I believe this should be 0x1.

Baseline:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git

Tested Info:
Boot tested on OMAP 2/3/4.

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Acked-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5a30658..da49b56 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -373,7 +373,7 @@ static int _set_module_autoidle(struct omap_hwmod *oh, u8 autoidle,
 	}
 
 	autoidle_shift = oh->class->sysc->sysc_fields->autoidle_shift;
-	autoidle_mask = (0x3 << autoidle_shift);
+	autoidle_mask = (0x1 << autoidle_shift);
 
 	*v &= ~autoidle_mask;
 	*v |= autoidle << autoidle_shift;
-- 
1.6.0.4

RE: [PATCH] OMAP2+: hwmod: incorrect computation of autoidle_mask

[DebBarma, Tarun Kanti]

Benoit,
> -----Original Message-----
> From: Cousson, Benoit
> Sent: Monday, January 03, 2011 8:12 PM
> To: DebBarma, Tarun Kanti
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH] OMAP2+: hwmod: incorrect computation of autoidle_mask
> 
> Hi Tarun,
> 
> On 1/3/2011 10:59 PM, DebBarma, Tarun Kanti wrote:
> > Autoidle is just a single bit setting across OMAP platforms.
> > In _set_module_autoidle() I am seeing 0x3 where the mask is
> > computed. I believe this should be 0x1.
> 
> Just a minor comment; it looks from this description that you are not
> 100% sure it is a bug :-)
> You need to refer to the various TRMs, stating that this is indeed a bug
> for all OMAPs!
Ok, I will make the change.
> 
> Adding a "fix" in the subject seems more accurate to describe the patch.
> OMAP2+: Fix incorrect computation of autoidle_mask
Yes, I will make the change.

> 
> Since is it harmless for the moment, I think it will probably go for
> 2.6.39.
> 
> > Baseline:
> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> >
> > Tested Info:
> > Boot tested on OMAP 2/3/4.
> >
> > Signed-off-by: Tarun Kanti DebBarma<tarun.kanti@ti.com>
> > Acked-by: Rajendra Nayak<rnayak@ti.com>
> 
> Acked-by: Benoit Cousson <b-cousson@ti.com>
> 
> 
> Thanks,
> Benoit
> 
> > ---
> >   arch/arm/mach-omap2/omap_hwmod.c |    2 +-
> >   1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-
> omap2/omap_hwmod.c
> > index 5a30658..da49b56 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod.c
> > @@ -373,7 +373,7 @@ static int _set_module_autoidle(struct omap_hwmod
> *oh, u8 autoidle,
> >   	}
> >
> >   	autoidle_shift = oh->class->sysc->sysc_fields->autoidle_shift;
> > -	autoidle_mask = (0x3<<  autoidle_shift);
> > +	autoidle_mask = (0x1<<  autoidle_shift);
> >
> >   	*v&= ~autoidle_mask;
> >   	*v |= autoidle<<  autoidle_shift;