* [PATCH 06/06] OMAP3: PM: Update VDD1 OPP2 voltage level from 1.05 to 1.075
2009-05-28 12:43 ` [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access Rajendra Nayak
@ 2009-05-28 12:43 ` Rajendra Nayak
2009-06-04 23:24 ` [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access Kevin Hilman
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Rajendra Nayak @ 2009-05-28 12:43 UTC (permalink / raw)
To: linux-omap; +Cc: Rajendra Nayak
This patch updates the VDD1 OPP2 volatge according to the latest
Operating Condition Addendum from 1.05v to 1.075v
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
arch/arm/mach-omap2/omap3-opp.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/omap3-opp.h b/arch/arm/mach-omap2/omap3-opp.h
index 4bb165e..3c9a582 100644
--- a/arch/arm/mach-omap2/omap3-opp.h
+++ b/arch/arm/mach-omap2/omap3-opp.h
@@ -26,7 +26,7 @@ static struct omap_opp omap3_mpu_rate_table[] = {
/*OPP1*/
{S125M, VDD1_OPP1, 0x1E},
/*OPP2*/
- {S250M, VDD1_OPP2, 0x24},
+ {S250M, VDD1_OPP2, 0x26},
/*OPP3*/
{S500M, VDD1_OPP3, 0x30},
/*OPP4*/
@@ -50,7 +50,7 @@ static struct omap_opp omap3_dsp_rate_table[] = {
/*OPP1*/
{S90M, VDD1_OPP1, 0x1E},
/*OPP2*/
- {S180M, VDD1_OPP2, 0x24},
+ {S180M, VDD1_OPP2, 0x26},
/*OPP3*/
{S360M, VDD1_OPP3, 0x30},
/*OPP4*/
--
1.5.4.7
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access
2009-05-28 12:43 ` [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access Rajendra Nayak
2009-05-28 12:43 ` [PATCH 06/06] OMAP3: PM: Update VDD1 OPP2 voltage level from 1.05 to 1.075 Rajendra Nayak
@ 2009-06-04 23:24 ` Kevin Hilman
2009-06-05 13:11 ` Nayak, Rajendra
2009-06-05 21:26 ` Paul Walmsley
2009-06-05 22:05 ` Kevin Hilman
3 siblings, 1 reply; 14+ messages in thread
From: Kevin Hilman @ 2009-06-04 23:24 UTC (permalink / raw)
To: Rajendra Nayak; +Cc: linux-omap
Rajendra Nayak <rnayak@ti.com> writes:
> This patch implements locking using the semaphore in scratchpad
> memory preventing any concurrent access to scratchpad from OMAP
> and Baseband/Modem processor.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Any reason this function has to be written in asm? It's written along
with a bunch of other asm functions that are copied to and executed
from SRAM, but that doesn't appear to be the case here.
Looks like a simple C function would be clearer here.
Kevin
> ---
> arch/arm/mach-omap2/resource34xx.c | 6 +++++-
> arch/arm/mach-omap2/resource34xx.h | 2 ++
> arch/arm/mach-omap2/sleep34xx.S | 32 ++++++++++++++++++++++++++++++++
> 3 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/resource34xx.c b/arch/arm/mach-omap2/resource34xx.c
> index 82405b6..408d3ab 100644
> --- a/arch/arm/mach-omap2/resource34xx.c
> +++ b/arch/arm/mach-omap2/resource34xx.c
> @@ -236,6 +236,7 @@ static int program_opp_freq(int res, int target_level, int current_level)
> int ret = 0, l3_div;
> int *curr_opp;
>
> + lock_scratchpad_sem();
> if (res == VDD1_OPP) {
> curr_opp = &curr_vdd1_opp;
> clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
> @@ -253,11 +254,14 @@ static int program_opp_freq(int res, int target_level, int current_level)
> ret = clk_set_rate(dpll3_clk,
> l3_opps[target_level].rate * l3_div);
> }
> - if (ret)
> + if (ret) {
> + unlock_scratchpad_sem();
> return current_level;
> + }
> #ifdef CONFIG_PM
> omap3_save_scratchpad_contents();
> #endif
> + unlock_scratchpad_sem();
>
> *curr_opp = target_level;
> return target_level;
> diff --git a/arch/arm/mach-omap2/resource34xx.h b/arch/arm/mach-omap2/resource34xx.h
> index a160665..5b5618a 100644
> --- a/arch/arm/mach-omap2/resource34xx.h
> +++ b/arch/arm/mach-omap2/resource34xx.h
> @@ -29,6 +29,8 @@
> #include <mach/omap34xx.h>
>
> extern int sr_voltagescale_vcbypass(u32 t_opp, u32 c_opp, u8 t_vsel, u8 c_vsel);
> +extern void lock_scratchpad_sem();
> +extern void unlock_scratchpad_sem();
>
> /*
> * mpu_latency/core_latency are used to control the cpuidle C state.
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index 38aa3fd..aedcf94 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -39,6 +39,7 @@
> #define PM_PREPWSTST_MPU_V OMAP34XX_PRM_REGADDR(MPU_MOD, \
> OMAP3430_PM_PREPWSTST)
> #define CM_IDLEST1_CORE_V OMAP34XX_CM_REGADDR(CORE_MOD, CM_IDLEST1)
> +#define SDRC_SCRATCHPAD_SEM_V 0xd800291C
>
> /*
> * This is the physical address of the register as specified
> @@ -62,6 +63,37 @@
> #define SDRC_DLLA_STATUS_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_STATUS)
> #define SDRC_DLLA_CTRL_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
>
> + .text
> +/* Function to aquire the semaphore in scratchpad */
> +ENTRY(lock_scratchpad_sem)
> + stmfd sp!, {lr} @ save registers on stack
> +wait_sem:
> + mov r0,#1
> + ldr r1, sdrc_scratchpad_sem
> +wait_loop:
> + ldr r2, [r1] @ load the lock value
> + cmp r2, r0 @ is the lock free ?
> + beq wait_loop @ not free...
> + swp r2, r0, [r1] @ semaphore free so lock it and proceed
> + cmp r2, r0 @ did we succeed ?
> + beq wait_sem @ no - try again
> + ldmfd sp!, {pc} @ restore regs and return
> +sdrc_scratchpad_sem:
> + .word SDRC_SCRATCHPAD_SEM_V
> +ENTRY(lock_scratchpad_sem_sz)
> + .word . - lock_scratchpad_sem
> +
> + .text
> +/* Function to release the scratchpad semaphore */
> +ENTRY(unlock_scratchpad_sem)
> + stmfd sp!, {lr} @ save registers on stack
> + ldr r3, sdrc_scratchpad_sem
> + mov r2,#0
> + str r2,[r3]
> + ldmfd sp!, {pc} @ restore regs and return
> +ENTRY(unlock_scratchpad_sem_sz)
> + .word . - unlock_scratchpad_sem
> +
> .text
> /* Function call to get the restore pointer for resume from OFF */
> ENTRY(get_restore_pointer)
> --
> 1.5.4.7
>
> --
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access
2009-06-04 23:24 ` [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access Kevin Hilman
@ 2009-06-05 13:11 ` Nayak, Rajendra
0 siblings, 0 replies; 14+ messages in thread
From: Nayak, Rajendra @ 2009-06-05 13:11 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-omap@vger.kernel.org
>-----Original Message-----
>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>Sent: Friday, June 05, 2009 4:54 AM
>To: Nayak, Rajendra
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 05/06] OMAP3: PM: Implement locking for
>any scratchpad access
>
>Rajendra Nayak <rnayak@ti.com> writes:
>
>> This patch implements locking using the semaphore in scratchpad
>> memory preventing any concurrent access to scratchpad from OMAP
>> and Baseband/Modem processor.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>
>Any reason this function has to be written in asm? It's written along
>with a bunch of other asm functions that are copied to and executed
>from SRAM, but that doesn't appear to be the case here.
>
>Looks like a simple C function would be clearer here.
Kevin,
The idea was to atomically read and write into the semaphore memory
location in scratchpad using a swp instruction.
I am not too sure how to do this in C.
regards,
Rajendra
>
>Kevin
>
>> ---
>> arch/arm/mach-omap2/resource34xx.c | 6 +++++-
>> arch/arm/mach-omap2/resource34xx.h | 2 ++
>> arch/arm/mach-omap2/sleep34xx.S | 32
>++++++++++++++++++++++++++++++++
>> 3 files changed, 39 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/resource34xx.c
>b/arch/arm/mach-omap2/resource34xx.c
>> index 82405b6..408d3ab 100644
>> --- a/arch/arm/mach-omap2/resource34xx.c
>> +++ b/arch/arm/mach-omap2/resource34xx.c
>> @@ -236,6 +236,7 @@ static int program_opp_freq(int res, int
>target_level, int current_level)
>> int ret = 0, l3_div;
>> int *curr_opp;
>>
>> + lock_scratchpad_sem();
>> if (res == VDD1_OPP) {
>> curr_opp = &curr_vdd1_opp;
>> clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
>> @@ -253,11 +254,14 @@ static int program_opp_freq(int res,
>int target_level, int current_level)
>> ret = clk_set_rate(dpll3_clk,
>> l3_opps[target_level].rate * l3_div);
>> }
>> - if (ret)
>> + if (ret) {
>> + unlock_scratchpad_sem();
>> return current_level;
>> + }
>> #ifdef CONFIG_PM
>> omap3_save_scratchpad_contents();
>> #endif
>> + unlock_scratchpad_sem();
>>
>> *curr_opp = target_level;
>> return target_level;
>> diff --git a/arch/arm/mach-omap2/resource34xx.h
>b/arch/arm/mach-omap2/resource34xx.h
>> index a160665..5b5618a 100644
>> --- a/arch/arm/mach-omap2/resource34xx.h
>> +++ b/arch/arm/mach-omap2/resource34xx.h
>> @@ -29,6 +29,8 @@
>> #include <mach/omap34xx.h>
>>
>> extern int sr_voltagescale_vcbypass(u32 t_opp, u32 c_opp,
>u8 t_vsel, u8 c_vsel);
>> +extern void lock_scratchpad_sem();
>> +extern void unlock_scratchpad_sem();
>>
>> /*
>> * mpu_latency/core_latency are used to control the cpuidle C state.
>> diff --git a/arch/arm/mach-omap2/sleep34xx.S
>b/arch/arm/mach-omap2/sleep34xx.S
>> index 38aa3fd..aedcf94 100644
>> --- a/arch/arm/mach-omap2/sleep34xx.S
>> +++ b/arch/arm/mach-omap2/sleep34xx.S
>> @@ -39,6 +39,7 @@
>> #define PM_PREPWSTST_MPU_V OMAP34XX_PRM_REGADDR(MPU_MOD, \
>> OMAP3430_PM_PREPWSTST)
>> #define CM_IDLEST1_CORE_V OMAP34XX_CM_REGADDR(CORE_MOD,
>CM_IDLEST1)
>> +#define SDRC_SCRATCHPAD_SEM_V 0xd800291C
>>
>> /*
>> * This is the physical address of the register as specified
>> @@ -62,6 +63,37 @@
>> #define SDRC_DLLA_STATUS_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_STATUS)
>> #define SDRC_DLLA_CTRL_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
>>
>> + .text
>> +/* Function to aquire the semaphore in scratchpad */
>> +ENTRY(lock_scratchpad_sem)
>> + stmfd sp!, {lr} @ save registers on stack
>> +wait_sem:
>> + mov r0,#1
>> + ldr r1, sdrc_scratchpad_sem
>> +wait_loop:
>> + ldr r2, [r1] @ load the lock value
>> + cmp r2, r0 @ is the lock free ?
>> + beq wait_loop @ not free...
>> + swp r2, r0, [r1] @ semaphore free so lock it and proceed
>> + cmp r2, r0 @ did we succeed ?
>> + beq wait_sem @ no - try again
>> + ldmfd sp!, {pc} @ restore regs and return
>> +sdrc_scratchpad_sem:
>> + .word SDRC_SCRATCHPAD_SEM_V
>> +ENTRY(lock_scratchpad_sem_sz)
>> + .word . - lock_scratchpad_sem
>> +
>> + .text
>> +/* Function to release the scratchpad semaphore */
>> +ENTRY(unlock_scratchpad_sem)
>> + stmfd sp!, {lr} @ save registers on stack
>> + ldr r3, sdrc_scratchpad_sem
>> + mov r2,#0
>> + str r2,[r3]
>> + ldmfd sp!, {pc} @ restore regs and return
>> +ENTRY(unlock_scratchpad_sem_sz)
>> + .word . - unlock_scratchpad_sem
>> +
>> .text
>> /* Function call to get the restore pointer for resume from OFF */
>> ENTRY(get_restore_pointer)
>> --
>> 1.5.4.7
>>
>> --
>> 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
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access
2009-05-28 12:43 ` [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access Rajendra Nayak
2009-05-28 12:43 ` [PATCH 06/06] OMAP3: PM: Update VDD1 OPP2 voltage level from 1.05 to 1.075 Rajendra Nayak
2009-06-04 23:24 ` [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access Kevin Hilman
@ 2009-06-05 21:26 ` Paul Walmsley
2009-06-05 21:54 ` Woodruff, Richard
2009-06-05 22:05 ` Kevin Hilman
3 siblings, 1 reply; 14+ messages in thread
From: Paul Walmsley @ 2009-06-05 21:26 UTC (permalink / raw)
To: Rajendra Nayak; +Cc: linux-omap
Hi Rajendra,
On Thu, 28 May 2009, Rajendra Nayak wrote:
> This patch implements locking using the semaphore in scratchpad
> memory preventing any concurrent access to scratchpad from OMAP
> and Baseband/Modem processor.
This patch might be okay for the target use case. But there might still
be a race window with this patch, where the modem could steal the MPU's
lock or vice versa.
That swp tries to guarantee atomicity through exclusive memory accesses,
but the AXI2OCP bridge section of the TRM claims that the bridge
translates "exclusive accesses to non-exclusive read/write in the bridge"
(section 3.2.3.2). That seems to suggest that the memory accesses will be
non-atomic and that therefore a race window exists.
The easiest way to close it up tight might be to use the ICR, and have the
modem request permission to write to the scratchpad RAM from the MPU, or
vice versa. I realize that "easiest" is definitely relative in this
case...
- Paul
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
> arch/arm/mach-omap2/resource34xx.c | 6 +++++-
> arch/arm/mach-omap2/resource34xx.h | 2 ++
> arch/arm/mach-omap2/sleep34xx.S | 32 ++++++++++++++++++++++++++++++++
> 3 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/resource34xx.c b/arch/arm/mach-omap2/resource34xx.c
> index 82405b6..408d3ab 100644
> --- a/arch/arm/mach-omap2/resource34xx.c
> +++ b/arch/arm/mach-omap2/resource34xx.c
> @@ -236,6 +236,7 @@ static int program_opp_freq(int res, int target_level, int current_level)
> int ret = 0, l3_div;
> int *curr_opp;
>
> + lock_scratchpad_sem();
> if (res == VDD1_OPP) {
> curr_opp = &curr_vdd1_opp;
> clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
> @@ -253,11 +254,14 @@ static int program_opp_freq(int res, int target_level, int current_level)
> ret = clk_set_rate(dpll3_clk,
> l3_opps[target_level].rate * l3_div);
> }
> - if (ret)
> + if (ret) {
> + unlock_scratchpad_sem();
> return current_level;
> + }
> #ifdef CONFIG_PM
> omap3_save_scratchpad_contents();
> #endif
> + unlock_scratchpad_sem();
>
> *curr_opp = target_level;
> return target_level;
> diff --git a/arch/arm/mach-omap2/resource34xx.h b/arch/arm/mach-omap2/resource34xx.h
> index a160665..5b5618a 100644
> --- a/arch/arm/mach-omap2/resource34xx.h
> +++ b/arch/arm/mach-omap2/resource34xx.h
> @@ -29,6 +29,8 @@
> #include <mach/omap34xx.h>
>
> extern int sr_voltagescale_vcbypass(u32 t_opp, u32 c_opp, u8 t_vsel, u8 c_vsel);
> +extern void lock_scratchpad_sem();
> +extern void unlock_scratchpad_sem();
>
> /*
> * mpu_latency/core_latency are used to control the cpuidle C state.
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index 38aa3fd..aedcf94 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -39,6 +39,7 @@
> #define PM_PREPWSTST_MPU_V OMAP34XX_PRM_REGADDR(MPU_MOD, \
> OMAP3430_PM_PREPWSTST)
> #define CM_IDLEST1_CORE_V OMAP34XX_CM_REGADDR(CORE_MOD, CM_IDLEST1)
> +#define SDRC_SCRATCHPAD_SEM_V 0xd800291C
>
> /*
> * This is the physical address of the register as specified
> @@ -62,6 +63,37 @@
> #define SDRC_DLLA_STATUS_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_STATUS)
> #define SDRC_DLLA_CTRL_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
>
> + .text
> +/* Function to aquire the semaphore in scratchpad */
> +ENTRY(lock_scratchpad_sem)
> + stmfd sp!, {lr} @ save registers on stack
> +wait_sem:
> + mov r0,#1
> + ldr r1, sdrc_scratchpad_sem
> +wait_loop:
> + ldr r2, [r1] @ load the lock value
> + cmp r2, r0 @ is the lock free ?
> + beq wait_loop @ not free...
> + swp r2, r0, [r1] @ semaphore free so lock it and proceed
> + cmp r2, r0 @ did we succeed ?
> + beq wait_sem @ no - try again
> + ldmfd sp!, {pc} @ restore regs and return
> +sdrc_scratchpad_sem:
> + .word SDRC_SCRATCHPAD_SEM_V
> +ENTRY(lock_scratchpad_sem_sz)
> + .word . - lock_scratchpad_sem
> +
> + .text
> +/* Function to release the scratchpad semaphore */
> +ENTRY(unlock_scratchpad_sem)
> + stmfd sp!, {lr} @ save registers on stack
> + ldr r3, sdrc_scratchpad_sem
> + mov r2,#0
> + str r2,[r3]
> + ldmfd sp!, {pc} @ restore regs and return
> +ENTRY(unlock_scratchpad_sem_sz)
> + .word . - unlock_scratchpad_sem
> +
> .text
> /* Function call to get the restore pointer for resume from OFF */
> ENTRY(get_restore_pointer)
> --
> 1.5.4.7
>
> --
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access
2009-06-05 21:26 ` Paul Walmsley
@ 2009-06-05 21:54 ` Woodruff, Richard
2009-06-05 22:21 ` Paul Walmsley
0 siblings, 1 reply; 14+ messages in thread
From: Woodruff, Richard @ 2009-06-05 21:54 UTC (permalink / raw)
To: Paul Walmsley, Nayak, Rajendra; +Cc: linux-omap@vger.kernel.org
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Paul Walmsley
> Sent: Friday, June 05, 2009 4:26 PM
> On Thu, 28 May 2009, Rajendra Nayak wrote:
>
> > This patch implements locking using the semaphore in scratchpad
> > memory preventing any concurrent access to scratchpad from OMAP
> > and Baseband/Modem processor.
>
> This patch might be okay for the target use case. But there might still
> be a race window with this patch, where the modem could steal the MPU's
> lock or vice versa.
>
> That swp tries to guarantee atomicity through exclusive memory accesses,
> but the AXI2OCP bridge section of the TRM claims that the bridge
> translates "exclusive accesses to non-exclusive read/write in the bridge"
> (section 3.2.3.2). That seems to suggest that the memory accesses will be
> non-atomic and that therefore a race window exists.
This is an incorrect interpretation. A race does not exist because of this point.
The AXI2OCP does translate external excusive operations this way (STREX/LWREX). This is not the same for SWP. SWP generates a global bus lock.
Exclusive operations do have a reservation in the L2 block. So they are good until they go to external memory. On OMAP these work as you expect with respect to the ARM, but won't work between ARM and DSP or other master.
SWP on the other hand still works. SWP is a relative pig compared to STREX/LWREX as it locks the whole L3, compared to a small key address, but, it's a lot lighter than a mailbox.
Regards,
Richard W.
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access
2009-06-05 21:54 ` Woodruff, Richard
@ 2009-06-05 22:21 ` Paul Walmsley
0 siblings, 0 replies; 14+ messages in thread
From: Paul Walmsley @ 2009-06-05 22:21 UTC (permalink / raw)
To: Woodruff, Richard; +Cc: Nayak, Rajendra, linux-omap@vger.kernel.org
On Fri, 5 Jun 2009, Woodruff, Richard wrote:
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Paul Walmsley
> > Sent: Friday, June 05, 2009 4:26 PM
>
> > On Thu, 28 May 2009, Rajendra Nayak wrote:
> >
> > > This patch implements locking using the semaphore in scratchpad
> > > memory preventing any concurrent access to scratchpad from OMAP
> > > and Baseband/Modem processor.
> >
> > This patch might be okay for the target use case. But there might still
> > be a race window with this patch, where the modem could steal the MPU's
> > lock or vice versa.
> >
> > That swp tries to guarantee atomicity through exclusive memory accesses,
> > but the AXI2OCP bridge section of the TRM claims that the bridge
> > translates "exclusive accesses to non-exclusive read/write in the bridge"
> > (section 3.2.3.2). That seems to suggest that the memory accesses will be
> > non-atomic and that therefore a race window exists.
>
> This is an incorrect interpretation. A race does not exist because of this point.
>
> The AXI2OCP does translate external excusive operations this way
> (STREX/LWREX). This is not the same for SWP. SWP generates a global
> bus lock.
>
> Exclusive operations do have a reservation in the L2 block. So they are
> good until they go to external memory. On OMAP these work as you expect
> with respect to the ARM, but won't work between ARM and DSP or other
> master.
>
> SWP on the other hand still works. SWP is a relative pig compared to
> STREX/LWREX as it locks the whole L3, compared to a small key address,
> but, it's a lot lighter than a mailbox.
OK, thanks for the explanation, Richard. Sounds like it's okay then,
- Paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access
2009-05-28 12:43 ` [PATCH 05/06] OMAP3: PM: Implement locking for any scratchpad access Rajendra Nayak
` (2 preceding siblings ...)
2009-06-05 21:26 ` Paul Walmsley
@ 2009-06-05 22:05 ` Kevin Hilman
3 siblings, 0 replies; 14+ messages in thread
From: Kevin Hilman @ 2009-06-05 22:05 UTC (permalink / raw)
To: Rajendra Nayak; +Cc: linux-omap
Rajendra Nayak <rnayak@ti.com> writes:
> This patch implements locking using the semaphore in scratchpad
> memory preventing any concurrent access to scratchpad from OMAP
> and Baseband/Modem processor.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Applying to PM brach after the clarifications.
Thanks,
Kevin
> ---
> arch/arm/mach-omap2/resource34xx.c | 6 +++++-
> arch/arm/mach-omap2/resource34xx.h | 2 ++
> arch/arm/mach-omap2/sleep34xx.S | 32 ++++++++++++++++++++++++++++++++
> 3 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/resource34xx.c b/arch/arm/mach-omap2/resource34xx.c
> index 82405b6..408d3ab 100644
> --- a/arch/arm/mach-omap2/resource34xx.c
> +++ b/arch/arm/mach-omap2/resource34xx.c
> @@ -236,6 +236,7 @@ static int program_opp_freq(int res, int target_level, int current_level)
> int ret = 0, l3_div;
> int *curr_opp;
>
> + lock_scratchpad_sem();
> if (res == VDD1_OPP) {
> curr_opp = &curr_vdd1_opp;
> clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
> @@ -253,11 +254,14 @@ static int program_opp_freq(int res, int target_level, int current_level)
> ret = clk_set_rate(dpll3_clk,
> l3_opps[target_level].rate * l3_div);
> }
> - if (ret)
> + if (ret) {
> + unlock_scratchpad_sem();
> return current_level;
> + }
> #ifdef CONFIG_PM
> omap3_save_scratchpad_contents();
> #endif
> + unlock_scratchpad_sem();
>
> *curr_opp = target_level;
> return target_level;
> diff --git a/arch/arm/mach-omap2/resource34xx.h b/arch/arm/mach-omap2/resource34xx.h
> index a160665..5b5618a 100644
> --- a/arch/arm/mach-omap2/resource34xx.h
> +++ b/arch/arm/mach-omap2/resource34xx.h
> @@ -29,6 +29,8 @@
> #include <mach/omap34xx.h>
>
> extern int sr_voltagescale_vcbypass(u32 t_opp, u32 c_opp, u8 t_vsel, u8 c_vsel);
> +extern void lock_scratchpad_sem();
> +extern void unlock_scratchpad_sem();
>
> /*
> * mpu_latency/core_latency are used to control the cpuidle C state.
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index 38aa3fd..aedcf94 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -39,6 +39,7 @@
> #define PM_PREPWSTST_MPU_V OMAP34XX_PRM_REGADDR(MPU_MOD, \
> OMAP3430_PM_PREPWSTST)
> #define CM_IDLEST1_CORE_V OMAP34XX_CM_REGADDR(CORE_MOD, CM_IDLEST1)
> +#define SDRC_SCRATCHPAD_SEM_V 0xd800291C
>
> /*
> * This is the physical address of the register as specified
> @@ -62,6 +63,37 @@
> #define SDRC_DLLA_STATUS_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_STATUS)
> #define SDRC_DLLA_CTRL_V OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
>
> + .text
> +/* Function to aquire the semaphore in scratchpad */
> +ENTRY(lock_scratchpad_sem)
> + stmfd sp!, {lr} @ save registers on stack
> +wait_sem:
> + mov r0,#1
> + ldr r1, sdrc_scratchpad_sem
> +wait_loop:
> + ldr r2, [r1] @ load the lock value
> + cmp r2, r0 @ is the lock free ?
> + beq wait_loop @ not free...
> + swp r2, r0, [r1] @ semaphore free so lock it and proceed
> + cmp r2, r0 @ did we succeed ?
> + beq wait_sem @ no - try again
> + ldmfd sp!, {pc} @ restore regs and return
> +sdrc_scratchpad_sem:
> + .word SDRC_SCRATCHPAD_SEM_V
> +ENTRY(lock_scratchpad_sem_sz)
> + .word . - lock_scratchpad_sem
> +
> + .text
> +/* Function to release the scratchpad semaphore */
> +ENTRY(unlock_scratchpad_sem)
> + stmfd sp!, {lr} @ save registers on stack
> + ldr r3, sdrc_scratchpad_sem
> + mov r2,#0
> + str r2,[r3]
> + ldmfd sp!, {pc} @ restore regs and return
> +ENTRY(unlock_scratchpad_sem_sz)
> + .word . - unlock_scratchpad_sem
> +
> .text
> /* Function call to get the restore pointer for resume from OFF */
> ENTRY(get_restore_pointer)
> --
> 1.5.4.7
>
> --
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread