[PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Jon Hunter]

From: Jon Hunter <jon-hunter@ti.com>

When changing the L3 clock frequency, the CPU is executing from internal RAM
and the SDRC clock is disabled. During this time accesses made to external
DDR are stalled. If the ARM subsystem attempts to access the DDR while the
SDRC clock is disabled this will stall the CPU until the access to the SDRC
timeouts. A timeout on the SDRC should never occur. Once a timeout occurs all
the following accesses will be aborted and the DDR is no longer accessible.

Although the code being executed in the internal RAM does not directly access
the DDR, it was found that the branch prediction logic in the CPU may cause
the CPU to prefetch code from a DDR location while the SDRC clock is disabled.
This was causing an SDRC timeout which resulted in a system hang.

This patch fixes this problem by ensuring the branch prediction logic is
disabled while changing the L3 clock frequency. The branch prediction logic
is disabled by clearing the Z-bit in the ARM AUX CTRL register.

Disabling the branch prediction logic does not have any noticable impact
on the execution time of this code section. The hardware observability
signals were used to monitor the sdrc idle time with and without this
patch when operating at different CPU frequencies (150MHz, 500MHz and
600MHz) and the total sdrc idle time when changing frequenct was in
the range of 9-11us. This was measured on an omap3430 SDP running the
omapzoom p-android-omap-2.6.29 branch.

This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels.
The commits can be viewed here:
http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d
http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940

This patch has been rebased on the latest linux-omap tree and tested on
Kevin Hilman's pm branch.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/sram34xx.S |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index de99ba2..e87e730 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -129,8 +129,11 @@ ENTRY(omap3_sram_configure_core_dpll)
 	ldr	r4, [sp, #80]
 	str     r4, omap_sdrc_mr_1_val
 skip_cs1_params:
+	mrc p15, 0, r8, c1, c0, 0	@ read aux ctrl register
+	bic r10, r8, #0x800		@ clear Z-bit, disable branch prediction
+	mcr p15, 0, r10, c1, c0, 0	@ write aux ctrl register
 	dsb				@ flush buffered writes to interconnect
-
+	isb				@ prevent speculative exec past here
 	cmp	r3, #1			@ if increasing SDRC clk rate,
 	bleq	configure_sdrc		@ program the SDRC regs early (for RFR)
 	cmp	r1, #SDRC_UNLOCK_DLL	@ set the intended DLL state
@@ -148,6 +151,7 @@ skip_cs1_params:
 	beq	return_to_sdram		@ return to SDRAM code, otherwise,
 	bl	configure_sdrc		@ reprogram SDRC regs now
 return_to_sdram:
+	mcr p15, 0, r8, c1, c0, 0	@ restore aux ctrl register
 	isb				@ prevent speculative exec past here
 	mov 	r0, #0 			@ return value
 	ldmfd	sp!, {r1-r12, pc}	@ restore regs and return
-- 
1.7.0.4

Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Paul Walmsley]

Hi Jon,

I regret the delay:

On Wed, 21 Jul 2010, Jon Hunter wrote:

> From: Jon Hunter <jon-hunter@ti.com>
> 
> When changing the L3 clock frequency, the CPU is executing from internal RAM
> and the SDRC clock is disabled. During this time accesses made to external
> DDR are stalled. If the ARM subsystem attempts to access the DDR while the
> SDRC clock is disabled this will stall the CPU until the access to the SDRC
> timeouts. A timeout on the SDRC should never occur. Once a timeout occurs all
> the following accesses will be aborted and the DDR is no longer accessible.
> 
> Although the code being executed in the internal RAM does not directly access
> the DDR, it was found that the branch prediction logic in the CPU may cause
> the CPU to prefetch code from a DDR location while the SDRC clock is disabled.
> This was causing an SDRC timeout which resulted in a system hang.
> 
> This patch fixes this problem by ensuring the branch prediction logic is
> disabled while changing the L3 clock frequency. The branch prediction logic
> is disabled by clearing the Z-bit in the ARM AUX CTRL register.
> 
> Disabling the branch prediction logic does not have any noticable impact
> on the execution time of this code section. The hardware observability
> signals were used to monitor the sdrc idle time with and without this
> patch when operating at different CPU frequencies (150MHz, 500MHz and
> 600MHz) and the total sdrc idle time when changing frequenct was in
> the range of 9-11us. This was measured on an omap3430 SDP running the
> omapzoom p-android-omap-2.6.29 branch.
> 
> This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels.
> The commits can be viewed here:
> http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d
> http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940
> 
> This patch has been rebased on the latest linux-omap tree and tested on
> Kevin Hilman's pm branch.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

Really nice changelog.  I wish every patch had a description this good.  
Patch looks really good, too.  Queued for 2.6.37.


- Paul

Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Paul Walmsley]

Jon,

one other brief note:

On Wed, 21 Jul 2010, Jon Hunter wrote:

> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
> index de99ba2..e87e730 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -129,8 +129,11 @@ ENTRY(omap3_sram_configure_core_dpll)
>  	ldr	r4, [sp, #80]
>  	str     r4, omap_sdrc_mr_1_val
>  skip_cs1_params:
> +	mrc p15, 0, r8, c1, c0, 0	@ read aux ctrl register
> +	bic r10, r8, #0x800		@ clear Z-bit, disable branch prediction
> +	mcr p15, 0, r10, c1, c0, 0	@ write aux ctrl register

Please be careful with the whitespace between the opcode and the 
arguments - I will fix this in the current patch but it seems best to keep 
this consistent.


- Paul

Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [100915 12:07]:
> > 
> > This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels.
> > The commits can be viewed here:
> > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d
> > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940
> > 
> > This patch has been rebased on the latest linux-omap tree and tested on
> > Kevin Hilman's pm branch.

Sounds like a very important fix. But also a good example of how
messed up things are with the TI Linux kernel development.

Why has this fix it been sitting in some TI internal tree since
Fri, 12 Mar 2010?

This same bug has been patched in three different trees? But not in
the mainline kernel?

And this is happening all the time with the TI fixes.

Tony

Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Paul Walmsley]

On Wed, 15 Sep 2010, Tony Lindgren wrote:

> * Paul Walmsley <paul@pwsan.com> [100915 12:07]:
> > > 
> > > This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels.
> > > The commits can be viewed here:
> > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d
> > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940
> > > 
> > > This patch has been rebased on the latest linux-omap tree and tested on
> > > Kevin Hilman's pm branch.
> 
> Sounds like a very important fix. But also a good example of how
> messed up things are with the TI Linux kernel development.
> 
> Why has this fix it been sitting in some TI internal tree since
> Fri, 12 Mar 2010?

About two months of it is my fault, since it was posted to l-o on July 21.  
But all the time between 12 March and 21 July is up to TI to answer.
This really could have been a useful patch for certain vendors to have 
that are using CORE DVFS on their currently-shipping OMAP3 devices.

> This same bug has been patched in three different trees? But not in
> the mainline kernel?
> 
> And this is happening all the time with the TI fixes.

Yeah.


- Paul

Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Gadiyar, Anand]

On Thu, Sep 16, 2010 at 4:40 AM, Paul Walmsley <paul@pwsan.com> wrote:
> On Wed, 15 Sep 2010, Tony Lindgren wrote:
>
>> * Paul Walmsley <paul@pwsan.com> [100915 12:07]:
>> > >
>> > > This change has been commited to both TI's android 2.6.29 and 2.6.32 kernels.
>> > > The commits can be viewed here:
>> > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5679c7b1142f3cc2b9285181d53f6b40c4d0296d
>> > > http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=cf16e57823575d98e9d5165aa7a498ffb751c940
>> > >
>> > > This patch has been rebased on the latest linux-omap tree and tested on
>> > > Kevin Hilman's pm branch.
>>
>> Sounds like a very important fix. But also a good example of how
>> messed up things are with the TI Linux kernel development.
>>
>> Why has this fix it been sitting in some TI internal tree since
>> Fri, 12 Mar 2010?
>

Er, the Mar 12 commit date is misleading - it's likely a customer-internal
tree. It was committed to a TI internal tree 29 June/15 July, and sent to the
list within a week of the second revision.

The people who discovered this are on customer support - we spend
a significant amount of time solving bugs like these, but have very
little gap before the next big one strikes. We work tight production
schedules, getting products out and only just have enough time to
inform internal teams, but not enough to check if these fixes apply
to internal trees (mainline is far far away) and get them there.

(FWIW, Jon's been responsible for discovering and fixing more bugs
on OMAP than almost anyone else I know.)

For this specific patch, I'm not sure if other TI PM teams even
knew about the existence of this patch until end-June.



> About two months of it is my fault, since it was posted to l-o on July 21.
> But all the time between 12 March and 21 July is up to TI to answer.
> This really could have been a useful patch for certain vendors to have
> that are using CORE DVFS on their currently-shipping OMAP3 devices.

Sure, and I'm certain those other vendors have an equal number of critical
bug fixes sitting in their own trees, which they steadfastly refuse to
share with
other competing vendors until their own products are out. (I have specific
examples in mind, but don't want to start another flame war).

Grow up - when a bug is discovered in the field, people are not likely to
share with others in the interest of their own product timelines. While
this may overall be less beneficial for everyone, that is indeed how many
think and work.

>
>> This same bug has been patched in three different trees? But not in
>> the mainline kernel?
>>
>> And this is happening all the time with the TI fixes.
>
> Yeah.

OMAP4 has been demonstrably different - almost every single patch
reaches mainline almost as soon as it is discovered. We had new
silicon woken up with near-mainline code, and new boards supported
in mainline even before the number of boards were in the mid-double digits.

We're working very hard to keep things this way - give us a break. Stop
inciting flame wars. A little support and understanding on your part
would go a long way.

- Anand

RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Woodruff, Richard]

> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Paul Walmsley
> Sent: Wednesday, September 15, 2010 2:15 PM

> > This patch fixes this problem by ensuring the branch prediction logic is
> > disabled while changing the L3 clock frequency. The branch prediction logic
> > is disabled by clearing the Z-bit in the ARM AUX CTRL register.

Small correction, Z bit is in CR register. AUX CTRL figures in with the ASA feature.

> Really nice changelog.  I wish every patch had a description this good.
> Patch looks really good, too.  Queued for 2.6.37.

It is system specific if this change is required. It is probably safer to have it than not.

If the AUX CTRL register has the ASA bit/feature active to allow speculative accesses to propagate past the L2 boundary the Z bit should be cleared as in the patch.

However, if ASA bit is not activated then Z bit clearing should not be necessary as speculation will be squashed if there is no L2 hit (so no DDR request will be generated).

It is not recommended to enable ASA bit as it is known to cause some issues on EMU/HS devices. It was also projected as loosing more than it gained across some benchmarks.

Early boot loaders used to set the ASA.  It was removed long back.  Some kernels kept the value and opened up the lockup window.  I don't recall the linux-omap open kernel having the issue. Some vendor ones did over time.

Regards,
Richard W.

RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Shilimkar, Santosh]

[-- Attachment #1: Type: text/plain, Size: 5115 bytes --]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Woodruff, Richard
> Sent: Thursday, September 16, 2010 11:36 AM
> To: Paul Walmsley; Hunter, Jon
> Cc: linux-omap; khilman@deeprootsystems.com; tony@atomide.com
> Subject: RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing
> frequency
> 
> 
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Paul Walmsley
> > Sent: Wednesday, September 15, 2010 2:15 PM
> 
> > > This patch fixes this problem by ensuring the branch prediction logic
> is
> > > disabled while changing the L3 clock frequency. The branch prediction
> logic
> > > is disabled by clearing the Z-bit in the ARM AUX CTRL register.
> 
> Small correction, Z bit is in CR register. AUX CTRL figures in with the
> ASA feature.
> 
> > Really nice changelog.  I wish every patch had a description this good.
> > Patch looks really good, too.  Queued for 2.6.37.
> 
> It is system specific if this change is required. It is probably safer to
> have it than not.
> 
> If the AUX CTRL register has the ASA bit/feature active to allow
> speculative accesses to propagate past the L2 boundary the Z bit should be
> cleared as in the patch.
> 
> However, if ASA bit is not activated then Z bit clearing should not be
> necessary as speculation will be squashed if there is no L2 hit (so no DDR
> request will be generated).
> 
> It is not recommended to enable ASA bit as it is known to cause some
> issues on EMU/HS devices. It was also projected as loosing more than it
> gained across some benchmarks.
> 
> Early boot loaders used to set the ASA.  It was removed long back.  Some
> kernels kept the value and opened up the lockup window.  I don't recall
> the linux-omap open kernel having the issue. Some vendor ones did over
> time.
> 
The code seems to be correct but just the description has typo. The code
is using control register. I just corrected the description and white
space issue. Here is updated patch. 

Paul,
You can use this version if you like

>From fd4250671a1ae8deb718ac3688ea8971df7524cf Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Thu, 16 Sep 2010 12:03:23 +0530
Subject: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

When changing the L3 clock frequency, the CPU is executing from internal RAM
and the SDRC clock is disabled. During this time accesses made to external
DDR are stalled. If the ARM subsystem attempts to access the DDR while the
SDRC clock is disabled this will stall the CPU until the access to the SDRC
timeouts. A timeout on the SDRC should never occur. Once a timeout occurs all
the following accesses will be aborted and the DDR is no longer accessible.

Although the code being executed in the internal RAM does not directly access
the DDR, it was found that the branch prediction logic in the CPU may cause
the CPU to prefetch code from a DDR location while the SDRC clock is disabled.
This was causing an SDRC timeout which resulted in a system hang.

This patch fixes this problem by ensuring the branch prediction logic is
disabled while changing the L3 clock frequency. The branch prediction logic
is disabled by clearing the Z-bit in the ARM CTRL register.

Disabling the branch prediction logic does not have any noticable impact
on the execution time of this code section. The hardware observability
signals were used to monitor the sdrc idle time with and without this
patch when operating at different CPU frequencies (150MHz, 500MHz and
600MHz) and the total sdrc idle time when changing frequenct was in
the range of 9-11us. This was measured on an omap3430 SDP running the
omapzoom p-android-omap-2.6.29 branch.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/sram34xx.S |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index de99ba2..3637274 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -129,8 +129,11 @@ ENTRY(omap3_sram_configure_core_dpll)
 	ldr	r4, [sp, #80]
 	str     r4, omap_sdrc_mr_1_val
 skip_cs1_params:
+	mrc	p15, 0, r8, c1, c0, 0	@ read ctrl register
+	bic	r10, r8, #0x800		@ clear Z-bit, disable branch prediction
+	mcr	p15, 0, r10, c1, c0, 0	@ write ctrl register
 	dsb				@ flush buffered writes to interconnect
-
+	isb				@ prevent speculative exec past here
 	cmp	r3, #1			@ if increasing SDRC clk rate,
 	bleq	configure_sdrc		@ program the SDRC regs early (for RFR)
 	cmp	r1, #SDRC_UNLOCK_DLL	@ set the intended DLL state
@@ -148,6 +151,7 @@ skip_cs1_params:
 	beq	return_to_sdram		@ return to SDRAM code, otherwise,
 	bl	configure_sdrc		@ reprogram SDRC regs now
 return_to_sdram:
+	mcr	p15, 0, r8, c1, c0, 0	@ restore ctrl register
 	isb				@ prevent speculative exec past here
 	mov 	r0, #0 			@ return value
 	ldmfd	sp!, {r1-r12, pc}	@ restore regs and return
-- 
1.6.0.4



[-- Attachment #2: 0001-omap3-Prevent-SDRC-deadlock-when-L3-is-changing-fre.patch --]
[-- Type: application/octet-stream, Size: 2892 bytes --]

From fd4250671a1ae8deb718ac3688ea8971df7524cf Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Thu, 16 Sep 2010 12:03:23 +0530
Subject: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

When changing the L3 clock frequency, the CPU is executing from internal RAM
and the SDRC clock is disabled. During this time accesses made to external
DDR are stalled. If the ARM subsystem attempts to access the DDR while the
SDRC clock is disabled this will stall the CPU until the access to the SDRC
timeouts. A timeout on the SDRC should never occur. Once a timeout occurs all
the following accesses will be aborted and the DDR is no longer accessible.

Although the code being executed in the internal RAM does not directly access
the DDR, it was found that the branch prediction logic in the CPU may cause
the CPU to prefetch code from a DDR location while the SDRC clock is disabled.
This was causing an SDRC timeout which resulted in a system hang.

This patch fixes this problem by ensuring the branch prediction logic is
disabled while changing the L3 clock frequency. The branch prediction logic
is disabled by clearing the Z-bit in the ARM CTRL register.

Disabling the branch prediction logic does not have any noticable impact
on the execution time of this code section. The hardware observability
signals were used to monitor the sdrc idle time with and without this
patch when operating at different CPU frequencies (150MHz, 500MHz and
600MHz) and the total sdrc idle time when changing frequenct was in
the range of 9-11us. This was measured on an omap3430 SDP running the
omapzoom p-android-omap-2.6.29 branch.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/sram34xx.S |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index de99ba2..3637274 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -129,8 +129,11 @@ ENTRY(omap3_sram_configure_core_dpll)
 	ldr	r4, [sp, #80]
 	str     r4, omap_sdrc_mr_1_val
 skip_cs1_params:
+	mrc	p15, 0, r8, c1, c0, 0	@ read ctrl register
+	bic	r10, r8, #0x800		@ clear Z-bit, disable branch prediction
+	mcr	p15, 0, r10, c1, c0, 0	@ write ctrl register
 	dsb				@ flush buffered writes to interconnect
-
+	isb				@ prevent speculative exec past here
 	cmp	r3, #1			@ if increasing SDRC clk rate,
 	bleq	configure_sdrc		@ program the SDRC regs early (for RFR)
 	cmp	r1, #SDRC_UNLOCK_DLL	@ set the intended DLL state
@@ -148,6 +151,7 @@ skip_cs1_params:
 	beq	return_to_sdram		@ return to SDRAM code, otherwise,
 	bl	configure_sdrc		@ reprogram SDRC regs now
 return_to_sdram:
+	mcr	p15, 0, r8, c1, c0, 0	@ restore ctrl register
 	isb				@ prevent speculative exec past here
 	mov 	r0, #0 			@ return value
 	ldmfd	sp!, {r1-r12, pc}	@ restore regs and return
-- 
1.6.0.4

Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Tony Lindgren]

* Gadiyar, Anand <gadiyar@ti.com> [100915 21:51]:
> 
> > About two months of it is my fault, since it was posted to l-o on July 21.
> > But all the time between 12 March and 21 July is up to TI to answer.
> > This really could have been a useful patch for certain vendors to have
> > that are using CORE DVFS on their currently-shipping OMAP3 devices.
> 
> Sure, and I'm certain those other vendors have an equal number of critical
> bug fixes sitting in their own trees, which they steadfastly refuse to
> share with
> other competing vendors until their own products are out. (I have specific
> examples in mind, but don't want to start another flame war).
> 
> Grow up - when a bug is discovered in the field, people are not likely to
> share with others in the interest of their own product timelines. While
> this may overall be less beneficial for everyone, that is indeed how many
> think and work.

I don't buy this. But maybe some of TI's customers can comment on this?

AFAIK everybody wants to avoid the duplicate effort of finding and fixing
these bugs.

TI is the only party who is even aware of all the bugs fixed in various
places. And it's TI who should follow up that the bugfixes get posted
in a timely manner.

Regards,

Tony

Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Gadiyar, Anand]

On Fri, Sep 17, 2010 at 12:27 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Gadiyar, Anand <gadiyar@ti.com> [100915 21:51]:
>>
>> > About two months of it is my fault, since it was posted to l-o on July 21.
>> > But all the time between 12 March and 21 July is up to TI to answer.
>> > This really could have been a useful patch for certain vendors to have
>> > that are using CORE DVFS on their currently-shipping OMAP3 devices.
>>
>> Sure, and I'm certain those other vendors have an equal number of critical
>> bug fixes sitting in their own trees, which they steadfastly refuse to
>> share with
>> other competing vendors until their own products are out. (I have specific
>> examples in mind, but don't want to start another flame war).
>>
>> Grow up - when a bug is discovered in the field, people are not likely to
>> share with others in the interest of their own product timelines. While
>> this may overall be less beneficial for everyone, that is indeed how many
>> think and work.
>
> I don't buy this. But maybe some of TI's customers can comment on this?
>
> AFAIK everybody wants to avoid the duplicate effort of finding and fixing
> these bugs.
>
> TI is the only party who is even aware of all the bugs fixed in various
> places. And it's TI who should follow up that the bugfixes get posted
> in a timely manner.
>

We're committed 100% to making sure mainline is working well
on OMAPs. This means that any bugs we discover will be reported,
and any fixes we have will make their way upstream as soon as possible.

I'm not going to go and fight what's happened in the past. But for the
future, we're taking steps to make sure mainline works well.

For the past - OMAP3 and before - we'll do our best to scrub all internal
trees for missing bugs/features/what not and hopefully this will
go upstream as we discover those.

Sounds fair?

- Anand

Re: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Tony Lindgren]

* Gadiyar, Anand <gadiyar@ti.com> [100916 13:16]:
> On Fri, Sep 17, 2010 at 12:27 AM, Tony Lindgren <tony@atomide.com> wrote:
> 
> For the past - OMAP3 and before - we'll do our best to scrub all internal
> trees for missing bugs/features/what not and hopefully this will
> go upstream as we discover those.
> 
> Sounds fair?

Sounds fair. But it might be worth checking that you guys have a system
in place for collecting omap specific fixes and promptly merging them
upstream to the mainline tree.

Regards,

Tony

RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Woodruff, Richard]

> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Tony Lindgren
> Sent: Thursday, September 16, 2010 3:50 PM

> Sounds fair. But it might be worth checking that you guys have a system
> in place for collecting omap specific fixes and promptly merging them
> upstream to the mainline tree.

Many times there will be lag.  In many cases the customer has written significant code.  Until they publish this code or publicly post it for review its not something TI should not repost as is.

I think many of the engineers do see the benefit of sharing. However, if they don't have a policy in place which allows them to post until after the production is released it won't happen early.

Anymore, I like to advise a customer's team that it is ok to post patches for RFC even if they don't have time to follow up or it is against an 'old' kernel.

Once this is done TI and others are free to make use of the info to better the common tree.  The customer who found it will re-use the tree likely and get this plus other benefits.

Its best to get this policy set at project start. Asking near launch may very well be denied.

Regards,
Richard W.

RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing frequency

[Paul Walmsley]

Hi Richard, Santosh,

On Thu, 16 Sep 2010, Shilimkar, Santosh wrote:

> > -----Original Message-----
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Woodruff, Richard
> > Sent: Thursday, September 16, 2010 11:36 AM
> > To: Paul Walmsley; Hunter, Jon
> > Cc: linux-omap; khilman@deeprootsystems.com; tony@atomide.com
> > Subject: RE: [PATCH] omap3: Prevent SDRC deadlock when L3 is changing
> > frequency
> > 
> > 
> > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > > owner@vger.kernel.org] On Behalf Of Paul Walmsley
> > > Sent: Wednesday, September 15, 2010 2:15 PM
> > 
> > > > This patch fixes this problem by ensuring the branch prediction logic
> > is
> > > > disabled while changing the L3 clock frequency. The branch prediction
> > logic
> > > > is disabled by clearing the Z-bit in the ARM AUX CTRL register.
> > 
> > Small correction, Z bit is in CR register. AUX CTRL figures in with the
> > ASA feature.
> > 
> > > Really nice changelog.  I wish every patch had a description this good.
> > > Patch looks really good, too.  Queued for 2.6.37.
> > 
> > It is system specific if this change is required. It is probably safer to
> > have it than not.
> > 
> > If the AUX CTRL register has the ASA bit/feature active to allow
> > speculative accesses to propagate past the L2 boundary the Z bit should be
> > cleared as in the patch.
> > 
> > However, if ASA bit is not activated then Z bit clearing should not be
> > necessary as speculation will be squashed if there is no L2 hit (so no DDR
> > request will be generated).
> > 
> > It is not recommended to enable ASA bit as it is known to cause some
> > issues on EMU/HS devices. It was also projected as loosing more than it
> > gained across some benchmarks.
> > 
> > Early boot loaders used to set the ASA.  It was removed long back.  Some
> > kernels kept the value and opened up the lockup window.  I don't recall
> > the linux-omap open kernel having the issue. Some vendor ones did over
> > time.
> > 
> The code seems to be correct but just the description has typo. The code
> is using control register. I just corrected the description and white
> space issue. Here is updated patch. 
> 
> Paul,
> You can use this version if you like

Thanks for the fixes, will update the patch..


- Paul