| Message ID | 1243514587-5323-5-git-send-email-rnayak@ti.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Kevin Hilman |
| Headers | show |
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 -- 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
>-----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 > >-- 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
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 -- 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
> 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. -- 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
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 -- 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 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 -- 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
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)
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> --- 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(-)