| Message ID | 1355835378-23437-1-git-send-email-gururaja.hebbar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 18, 2012 at 06:26:18PM +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, CLKCTRL register offset for different clock domains > are not uniformly placed. An example of this would be the RTC clock > domain with CLKCTRL offset at 0x00. > In such cases the module ready check is skipped which leads to a data > abort during boot-up 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> Reviewed-by: Felipe Balbi <balbi@ti.com> > --- > Changes in v2: > - update commit message to reflect the actual cause. Previous > message conveyed a wrong/opposite message. > > :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; > - > omap_test_timeout(_is_module_ready(part, inst, cdoffs, clkctrl_offs), > MAX_MODULE_READY_TIME, i); > > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 18 Dec 2012, 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, CLKCTRL register offset for different clock domains > are not uniformly placed. An example of this would be the RTC clock > domain with CLKCTRL offset at 0x00. > In such cases the module ready check is skipped which leads to a data > abort during boot-up when RTC registers is accessed. We've got a few hwmods on OMAP44xx that don't have clkctrl_offs registers listed in the hwmod data, and which are not marked with HWMOD_NO_IDLEST. What's going to happen in those cases? - Paul
On Wed, Dec 19, 2012 at 07:43:50, Paul Walmsley wrote: > On Tue, 18 Dec 2012, 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, CLKCTRL register offset for different clock domains > > are not uniformly placed. An example of this would be the RTC clock > > domain with CLKCTRL offset at 0x00. > > In such cases the module ready check is skipped which leads to a data > > abort during boot-up when RTC registers is accessed. > > We've got a few hwmods on OMAP44xx that don't have clkctrl_offs registers > listed in the hwmod data, and which are not marked with HWMOD_NO_IDLEST. > What's going to happen in those cases? Hmm. This is a special case to me. Let me go back and do some review. Will be back with some more details. > > > - Paul > Regards, Gururaja
On Thu, 20 Dec 2012, Hebbar, Gururaja wrote: > On Wed, Dec 19, 2012 at 07:43:50, Paul Walmsley wrote: > > > We've got a few hwmods on OMAP44xx that don't have clkctrl_offs registers > > listed in the hwmod data, and which are not marked with HWMOD_NO_IDLEST. > > What's going to happen in those cases? > > Hmm. This is a special case to me. Let me go back and do some review. > Will be back with some more details. Probably what needs to happen is that those hwmods need to be marked with HWMOD_NO_IDLEST, in a separate patch before yours. As far as I can tell, that's how they should have been marked initially. Then your patch should be fine. - Paul
On Thu, Dec 20, 2012 at 13:05:27, Paul Walmsley wrote: > On Thu, 20 Dec 2012, Hebbar, Gururaja wrote: > > > On Wed, Dec 19, 2012 at 07:43:50, Paul Walmsley wrote: > > > > > We've got a few hwmods on OMAP44xx that don't have clkctrl_offs registers > > > listed in the hwmod data, and which are not marked with HWMOD_NO_IDLEST. > > > What's going to happen in those cases? > > > > Hmm. This is a special case to me. Let me go back and do some review. > > Will be back with some more details. > > Probably what needs to happen is that those hwmods need to be marked with > HWMOD_NO_IDLEST, in a separate patch before yours. As far as I can tell, > that's how they should have been marked initially. Then your patch should > be fine. Looking at latest kernel v3.8-rc5, there is separate cm33xx.c file which handles module ready checking for am33xx platform. So I will update this patch to work on this file instead of touching omap4 related file (cminst44xx.c) > > - Paul > Regards, Gururaja
On Wed, 30 Jan 2013, Hebbar, Gururaja wrote: > On Thu, Dec 20, 2012 at 13:05:27, Paul Walmsley wrote: > > On Thu, 20 Dec 2012, Hebbar, Gururaja wrote: > > > > > On Wed, Dec 19, 2012 at 07:43:50, Paul Walmsley wrote: > > > > > > > We've got a few hwmods on OMAP44xx that don't have clkctrl_offs registers > > > > listed in the hwmod data, and which are not marked with HWMOD_NO_IDLEST. > > > > What's going to happen in those cases? > > > > > > Hmm. This is a special case to me. Let me go back and do some review. > > > Will be back with some more details. > > > > Probably what needs to happen is that those hwmods need to be marked with > > HWMOD_NO_IDLEST, in a separate patch before yours. As far as I can tell, > > that's how they should have been marked initially. Then your patch should > > be fine. > > Looking at latest kernel v3.8-rc5, there is separate cm33xx.c file which > handles module ready checking for am33xx platform. So I will update this > patch to work on this file instead of touching omap4 related file > (cminst44xx.c) The same problem still exists. According to the TRM, there's at least one AM335x CLKCTRL register that doesn't have IDLEST bits. So to avoid reading reserved bits, either this patch or a preceding patch should mark those hwmods with HWMOD_NO_IDLEST. - Paul
On Wed, Jan 30, 2013 at 22:09:51, Paul Walmsley wrote: > On Wed, 30 Jan 2013, Hebbar, Gururaja wrote: > > > On Thu, Dec 20, 2012 at 13:05:27, Paul Walmsley wrote: > > > On Thu, 20 Dec 2012, Hebbar, Gururaja wrote: > > > > > > > On Wed, Dec 19, 2012 at 07:43:50, Paul Walmsley wrote: > > > > > > > > > We've got a few hwmods on OMAP44xx that don't have clkctrl_offs registers > > > > > listed in the hwmod data, and which are not marked with HWMOD_NO_IDLEST. > > > > > What's going to happen in those cases? > > > > > > > > Hmm. This is a special case to me. Let me go back and do some review. > > > > Will be back with some more details. > > > > > > Probably what needs to happen is that those hwmods need to be marked with > > > HWMOD_NO_IDLEST, in a separate patch before yours. As far as I can tell, > > > that's how they should have been marked initially. Then your patch should > > > be fine. > > > > Looking at latest kernel v3.8-rc5, there is separate cm33xx.c file which > > handles module ready checking for am33xx platform. So I will update this > > patch to work on this file instead of touching omap4 related file > > (cminst44xx.c) > > The same problem still exists. According to the TRM, there's at least one > AM335x CLKCTRL register that doesn't have IDLEST bits. So to avoid > reading reserved bits, either this patch or a preceding patch should > mark those hwmods with HWMOD_NO_IDLEST. Ok. I will look into this now. > > > - Paul > Regards, Gururaja
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);