diff mbox

ARM: OMAP2: hwmod: Fix "register offset NULL check" bug

Message ID 1355833929-25387-1-git-send-email-gururaja.hebbar@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hebbar, Gururaja Dec. 18, 2012, 12:32 p.m. UTC
From: "Hebbar, Gururaja" <gururaja.hebbar@ti.com>

omap4_cminst_wait_module_ready() checks if register offset is NULL.

int omap4_cminst_wait_module_ready(u8 part, u16 inst, s16 cdoffs,
				   u16 clkctrl_offs)
{
	int i = 0;

	if (!clkctrl_offs)
		return 0;

In case of AM33xx, not all clock domains have CLKSTCTRL at 0x00. An
example of this would be the RTC clock domain. In such cases the
module ready check is skipped which leads to a data abort during bootup
when RTC registers is accessed.

Since the actual base address is verified in
omap4_cminst_read_inst_reg(), this check here is not required at all
and hence can be removed.

Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
---
:100644 100644 7f9a464... 40545ff... M	arch/arm/mach-omap2/cminst44xx.c
 arch/arm/mach-omap2/cminst44xx.c |    3 ---
 1 file changed, 3 deletions(-)

Comments

Felipe Balbi Dec. 18, 2012, 12:31 p.m. UTC | #1
Hi,

On Tue, Dec 18, 2012 at 06:02:09PM +0530, Hebbar Gururaja wrote:
> From: "Hebbar, Gururaja" <gururaja.hebbar@ti.com>
> 
> omap4_cminst_wait_module_ready() checks if register offset is NULL.
> 
> int omap4_cminst_wait_module_ready(u8 part, u16 inst, s16 cdoffs,
> 				   u16 clkctrl_offs)
> {
> 	int i = 0;
> 
> 	if (!clkctrl_offs)
> 		return 0;
> 
> In case of AM33xx, not all clock domains have CLKSTCTRL at 0x00. An
> example of this would be the RTC clock domain. In such cases the
> module ready check is skipped which leads to a data abort during bootup
> when RTC registers is accessed.
> 
> Since the actual base address is verified in
> omap4_cminst_read_inst_reg(), this check here is not required at all
> and hence can be removed.
> 
> Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> ---
> :100644 100644 7f9a464... 40545ff... M	arch/arm/mach-omap2/cminst44xx.c
>  arch/arm/mach-omap2/cminst44xx.c |    3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
> index 7f9a464..40545ff 100644
> --- a/arch/arm/mach-omap2/cminst44xx.c
> +++ b/arch/arm/mach-omap2/cminst44xx.c
> @@ -271,9 +271,6 @@ int omap4_cminst_wait_module_ready(u8 part, u16 inst, s16 cdoffs,
>  {
>  	int i = 0;
>  
> -	if (!clkctrl_offs)
> -		return 0;

looks like commit log has the wrong argument. I believe you meant to say
that AM33xx has CLKSTCTRL exactly at 0x00 and that's why this check has
to be removed. No ?
Hebbar, Gururaja Dec. 18, 2012, 12:41 p.m. UTC | #2
On Tue, Dec 18, 2012 at 18:01:43, Balbi, Felipe wrote:
> Hi,
> 
> On Tue, Dec 18, 2012 at 06:02:09PM +0530, Hebbar Gururaja wrote:
> > From: "Hebbar, Gururaja" <gururaja.hebbar@ti.com>
> > 
> > omap4_cminst_wait_module_ready() checks if register offset is NULL.
> > 
> > int omap4_cminst_wait_module_ready(u8 part, u16 inst, s16 cdoffs,
> > 				   u16 clkctrl_offs)
> > {
> > 	int i = 0;
> > 
> > 	if (!clkctrl_offs)
> > 		return 0;
> > 
> > In case of AM33xx, not all clock domains have CLKSTCTRL at 0x00. An
> > example of this would be the RTC clock domain. In such cases the
> > module ready check is skipped which leads to a data abort during bootup
> > when RTC registers is accessed.
> > 
> > Since the actual base address is verified in
> > omap4_cminst_read_inst_reg(), this check here is not required at all
> > and hence can be removed.
> > 
> > Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> > ---
> > :100644 100644 7f9a464... 40545ff... M	arch/arm/mach-omap2/cminst44xx.c
> >  arch/arm/mach-omap2/cminst44xx.c |    3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
> > index 7f9a464..40545ff 100644
> > --- a/arch/arm/mach-omap2/cminst44xx.c
> > +++ b/arch/arm/mach-omap2/cminst44xx.c
> > @@ -271,9 +271,6 @@ int omap4_cminst_wait_module_ready(u8 part, u16 inst, s16 cdoffs,
> >  {
> >  	int i = 0;
> >  
> > -	if (!clkctrl_offs)
> > -		return 0;
> 
> looks like commit log has the wrong argument. I believe you meant to say
> that AM33xx has CLKSTCTRL exactly at 0x00 and that's why this check has
> to be removed. No ?

Correct. Thanks for the correction. V2 on the way.

> 
> -- 
> balbi
> 


Regards, 
Gururaja
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
index 7f9a464..40545ff 100644
--- a/arch/arm/mach-omap2/cminst44xx.c
+++ b/arch/arm/mach-omap2/cminst44xx.c
@@ -271,9 +271,6 @@  int omap4_cminst_wait_module_ready(u8 part, u16 inst, s16 cdoffs,
 {
 	int i = 0;
 
-	if (!clkctrl_offs)
-		return 0;
-
 	omap_test_timeout(_is_module_ready(part, inst, cdoffs, clkctrl_offs),
 			  MAX_MODULE_READY_TIME, i);