[PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Anand Gadiyar]

We need to wait on the IDLEST bit after the clocks are enabled
before attempting to access any register.

Currently, the USBTLL i-clock ops uses the clkops_omap2_dflt_wait,
while the USBTLL f-clock ops uses clkops_omap2_dflt. If the
i-clock is enabled first, the clkops_omap2_dflt_wait is
short-circuited as the companion f-clock is not enabled.
This can cause a data abort if the IDLEST has not transitioned,
and we try to access a USBTLL register.

Since the USBTLL i-clock and f-clock could be enabled in any order,
this is a bug. Fix it by changing the clkops for the f-clock.

Signed-off-by: Anand Gadiyar <gadiyar@ti.com>
---

 arch/arm/mach-omap2/clock3xxx_data.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-omap-2.6/arch/arm/mach-omap2/clock3xxx_data.c
===================================================================
--- linux-omap-2.6.orig/arch/arm/mach-omap2/clock3xxx_data.c
+++ linux-omap-2.6/arch/arm/mach-omap2/clock3xxx_data.c
@@ -1484,7 +1484,7 @@ static struct clk ts_fck = {
 
 static struct clk usbtll_fck = {
 	.name		= "usbtll_fck",
-	.ops		= &clkops_omap2_dflt,
+	.ops		= &clkops_omap2_dflt_wait,
 	.parent		= &dpll5_m2_ck,
 	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, OMAP3430ES2_CM_FCLKEN3),
 	.enable_bit	= OMAP3430ES2_EN_USBTLL_SHIFT,

RE: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Gadiyar, Anand]

Gadiyar, Anand wrote:
> We need to wait on the IDLEST bit after the clocks are enabled
> before attempting to access any register.
> 
> Currently, the USBTLL i-clock ops uses the clkops_omap2_dflt_wait,
> while the USBTLL f-clock ops uses clkops_omap2_dflt. If the
> i-clock is enabled first, the clkops_omap2_dflt_wait is
> short-circuited as the companion f-clock is not enabled.
> This can cause a data abort if the IDLEST has not transitioned,
> and we try to access a USBTLL register.
> 
> Since the USBTLL i-clock and f-clock could be enabled in any order,
> this is a bug. Fix it by changing the clkops for the f-clock.
> 
> Signed-off-by: Anand Gadiyar <gadiyar@ti.com>

Ping?


> ---
> 
>  arch/arm/mach-omap2/clock3xxx_data.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-omap-2.6/arch/arm/mach-omap2/clock3xxx_data.c
> ===================================================================
> --- linux-omap-2.6.orig/arch/arm/mach-omap2/clock3xxx_data.c
> +++ linux-omap-2.6/arch/arm/mach-omap2/clock3xxx_data.c
> @@ -1484,7 +1484,7 @@ static struct clk ts_fck = {
>  
>  static struct clk usbtll_fck = {
>  	.name		= "usbtll_fck",
> -	.ops		= &clkops_omap2_dflt,
> +	.ops		= &clkops_omap2_dflt_wait,
>  	.parent		= &dpll5_m2_ck,
>  	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, OMAP3430ES2_CM_FCLKEN3),
>  	.enable_bit	= OMAP3430ES2_EN_USBTLL_SHIFT,
> 

RE: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Paul Walmsley]

On Sat, 19 Jun 2010, Gadiyar, Anand wrote:

> Gadiyar, Anand wrote:
> > We need to wait on the IDLEST bit after the clocks are enabled
> > before attempting to access any register.
> > 
> > Currently, the USBTLL i-clock ops uses the clkops_omap2_dflt_wait,
> > while the USBTLL f-clock ops uses clkops_omap2_dflt. If the
> > i-clock is enabled first, the clkops_omap2_dflt_wait is
> > short-circuited as the companion f-clock is not enabled.
> > This can cause a data abort if the IDLEST has not transitioned,
> > and we try to access a USBTLL register.
> > 
> > Since the USBTLL i-clock and f-clock could be enabled in any order,
> > this is a bug. Fix it by changing the clkops for the f-clock.
> > 
> > Signed-off-by: Anand Gadiyar <gadiyar@ti.com>

It looks fine to me.  I will queue it for a -rc branch. 

> Ping?

Generally it is helpful if you have a patch for a subsystem that I 
maintain, to cc me directly.


- Paul

> 
> 
> > ---
> > 
> >  arch/arm/mach-omap2/clock3xxx_data.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Index: linux-omap-2.6/arch/arm/mach-omap2/clock3xxx_data.c
> > ===================================================================
> > --- linux-omap-2.6.orig/arch/arm/mach-omap2/clock3xxx_data.c
> > +++ linux-omap-2.6/arch/arm/mach-omap2/clock3xxx_data.c
> > @@ -1484,7 +1484,7 @@ static struct clk ts_fck = {
> >  
> >  static struct clk usbtll_fck = {
> >  	.name		= "usbtll_fck",
> > -	.ops		= &clkops_omap2_dflt,
> > +	.ops		= &clkops_omap2_dflt_wait,
> >  	.parent		= &dpll5_m2_ck,
> >  	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, OMAP3430ES2_CM_FCLKEN3),
> >  	.enable_bit	= OMAP3430ES2_EN_USBTLL_SHIFT,
> > --
> 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
> 


- Paul

RE: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Gadiyar, Anand]

Paul Walmsley wrote:
> Generally it is helpful if you have a patch for a subsystem that I 
> maintain, to cc me directly.

Will do. Thanks.

- Anand

RE: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Paul Walmsley]

On Mon, 21 Jun 2010, Paul Walmsley wrote:

> On Sat, 19 Jun 2010, Gadiyar, Anand wrote:
> 
> > Gadiyar, Anand wrote:
> > > We need to wait on the IDLEST bit after the clocks are enabled
> > > before attempting to access any register.
> > > 
> > > Currently, the USBTLL i-clock ops uses the clkops_omap2_dflt_wait,
> > > while the USBTLL f-clock ops uses clkops_omap2_dflt. If the
> > > i-clock is enabled first, the clkops_omap2_dflt_wait is
> > > short-circuited as the companion f-clock is not enabled.
> > > This can cause a data abort if the IDLEST has not transitioned,
> > > and we try to access a USBTLL register.
> > > 
> > > Since the USBTLL i-clock and f-clock could be enabled in any order,
> > > this is a bug. Fix it by changing the clkops for the f-clock.
> > > 
> > > Signed-off-by: Anand Gadiyar <gadiyar@ti.com>
> 
> It looks fine to me.  I will queue it for a -rc branch. 

Will requeue this for 2.6.36 merge window since it is not a regression, 
and it seems that Linus wants regression fixes for the -rc series...


- Paul

RE: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Gadiyar, Anand]

Paul Walmsley wrote:
> On Mon, 21 Jun 2010, Paul Walmsley wrote:
> > On Sat, 19 Jun 2010, Gadiyar, Anand wrote:
> > > Gadiyar, Anand wrote:
> > > > We need to wait on the IDLEST bit after the clocks are enabled
> > > > before attempting to access any register.
> > > > 
> > > > Currently, the USBTLL i-clock ops uses the clkops_omap2_dflt_wait,
> > > > while the USBTLL f-clock ops uses clkops_omap2_dflt. If the
> > > > i-clock is enabled first, the clkops_omap2_dflt_wait is
> > > > short-circuited as the companion f-clock is not enabled.
> > > > This can cause a data abort if the IDLEST has not transitioned,
> > > > and we try to access a USBTLL register.
> > > > 
> > > > Since the USBTLL i-clock and f-clock could be enabled in any order,
> > > > this is a bug. Fix it by changing the clkops for the f-clock.
> > > > 
> > > > Signed-off-by: Anand Gadiyar <gadiyar@ti.com>
> > 
> > It looks fine to me.  I will queue it for a -rc branch. 
> 
> Will requeue this for 2.6.36 merge window since it is not a regression, 
> and it seems that Linus wants regression fixes for the -rc series...
> 

I received a bug-report last Friday from some user of the recently-merged
ohci-omap3 driver about a data-abort caused at driver load. This patch
fixes that issue.

This wasn't known to me when I originally submitted the patch, but looks
like this is a problem caused in a driver that went in in the last merge
window. So maybe this patch can go in in the -rc series. ;)

It's your call. I'm not particular.

- Anand

RE: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Paul Walmsley]

Hi Tony,

what do you think on this one?  -rc or .36?


- Paul

On Thu, 24 Jun 2010, Gadiyar, Anand wrote:

> Paul Walmsley wrote:
> > On Mon, 21 Jun 2010, Paul Walmsley wrote:
> > > On Sat, 19 Jun 2010, Gadiyar, Anand wrote:
> > > > Gadiyar, Anand wrote:
> > > > > We need to wait on the IDLEST bit after the clocks are enabled
> > > > > before attempting to access any register.
> > > > > 
> > > > > Currently, the USBTLL i-clock ops uses the clkops_omap2_dflt_wait,
> > > > > while the USBTLL f-clock ops uses clkops_omap2_dflt. If the
> > > > > i-clock is enabled first, the clkops_omap2_dflt_wait is
> > > > > short-circuited as the companion f-clock is not enabled.
> > > > > This can cause a data abort if the IDLEST has not transitioned,
> > > > > and we try to access a USBTLL register.
> > > > > 
> > > > > Since the USBTLL i-clock and f-clock could be enabled in any order,
> > > > > this is a bug. Fix it by changing the clkops for the f-clock.
> > > > > 
> > > > > Signed-off-by: Anand Gadiyar <gadiyar@ti.com>
> > > 
> > > It looks fine to me.  I will queue it for a -rc branch. 
> > 
> > Will requeue this for 2.6.36 merge window since it is not a regression, 
> > and it seems that Linus wants regression fixes for the -rc series...
> > 
> 
> I received a bug-report last Friday from some user of the recently-merged
> ohci-omap3 driver about a data-abort caused at driver load. This patch
> fixes that issue.
> 
> This wasn't known to me when I originally submitted the patch, but looks
> like this is a problem caused in a driver that went in in the last merge
> window. So maybe this patch can go in in the -rc series. ;)
> 
> It's your call. I'm not particular.



- Paul

Re: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [100624 09:22]:
> Hi Tony,
> 
> what do you think on this one?  -rc or .36?

If we want to try to get it into the -rc cycle, the description
should mention what it fixes and show the error. Otherwise it might
be hard to argue this is a real fix or a regression.

Regards,

Tony

 
> 
> - Paul
> 
> On Thu, 24 Jun 2010, Gadiyar, Anand wrote:
> 
> > Paul Walmsley wrote:
> > > On Mon, 21 Jun 2010, Paul Walmsley wrote:
> > > > On Sat, 19 Jun 2010, Gadiyar, Anand wrote:
> > > > > Gadiyar, Anand wrote:
> > > > > > We need to wait on the IDLEST bit after the clocks are enabled
> > > > > > before attempting to access any register.
> > > > > > 
> > > > > > Currently, the USBTLL i-clock ops uses the clkops_omap2_dflt_wait,
> > > > > > while the USBTLL f-clock ops uses clkops_omap2_dflt. If the
> > > > > > i-clock is enabled first, the clkops_omap2_dflt_wait is
> > > > > > short-circuited as the companion f-clock is not enabled.
> > > > > > This can cause a data abort if the IDLEST has not transitioned,
> > > > > > and we try to access a USBTLL register.
> > > > > > 
> > > > > > Since the USBTLL i-clock and f-clock could be enabled in any order,
> > > > > > this is a bug. Fix it by changing the clkops for the f-clock.
> > > > > > 
> > > > > > Signed-off-by: Anand Gadiyar <gadiyar@ti.com>
> > > > 
> > > > It looks fine to me.  I will queue it for a -rc branch. 
> > > 
> > > Will requeue this for 2.6.36 merge window since it is not a regression, 
> > > and it seems that Linus wants regression fixes for the -rc series...
> > > 
> > 
> > I received a bug-report last Friday from some user of the recently-merged
> > ohci-omap3 driver about a data-abort caused at driver load. This patch
> > fixes that issue.
> > 
> > This wasn't known to me when I originally submitted the patch, but looks
> > like this is a problem caused in a driver that went in in the last merge
> > window. So maybe this patch can go in in the -rc series. ;)
> > 
> > It's your call. I'm not particular.
> 
> 
> 
> - Paul

Re: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Paul Walmsley]

On Thu, 1 Jul 2010, Tony Lindgren wrote:

> * Paul Walmsley <paul@pwsan.com> [100624 09:22]:
> > Hi Tony,
> > 
> > what do you think on this one?  -rc or .36?
> 
> If we want to try to get it into the -rc cycle, the description
> should mention what it fixes and show the error. Otherwise it might
> be hard to argue this is a real fix or a regression.

Thanks, I'll queue it for .36.

- Paul

Re: [PATCH] OMAP3: wait on IDLEST after enabling USBTLL fclk

[Kevin Hilman]

Anand Gadiyar <gadiyar@ti.com> writes:

> We need to wait on the IDLEST bit after the clocks are enabled
> before attempting to access any register.
>
> Currently, the USBTLL i-clock ops uses the clkops_omap2_dflt_wait,
> while the USBTLL f-clock ops uses clkops_omap2_dflt. If the
> i-clock is enabled first, the clkops_omap2_dflt_wait is
> short-circuited as the companion f-clock is not enabled.
> This can cause a data abort if the IDLEST has not transitioned,
> and we try to access a USBTLL register.
>
> Since the USBTLL i-clock and f-clock could be enabled in any order,
> this is a bug. Fix it by changing the clkops for the f-clock.
>
> Signed-off-by: Anand Gadiyar <gadiyar@ti.com>

Acked-by: Kevin Hilman <khilman@deeprootsystems.com>

> ---
>
>  arch/arm/mach-omap2/clock3xxx_data.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-omap-2.6/arch/arm/mach-omap2/clock3xxx_data.c
> ===================================================================
> --- linux-omap-2.6.orig/arch/arm/mach-omap2/clock3xxx_data.c
> +++ linux-omap-2.6/arch/arm/mach-omap2/clock3xxx_data.c
> @@ -1484,7 +1484,7 @@ static struct clk ts_fck = {
>  
>  static struct clk usbtll_fck = {
>  	.name		= "usbtll_fck",
> -	.ops		= &clkops_omap2_dflt,
> +	.ops		= &clkops_omap2_dflt_wait,
>  	.parent		= &dpll5_m2_ck,
>  	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, OMAP3430ES2_CM_FCLKEN3),
>  	.enable_bit	= OMAP3430ES2_EN_USBTLL_SHIFT,
> --
> 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