| Message ID | 1350487901-3108-2-git-send-email-will.deacon@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/17/12 08:31, Will Deacon wrote: > The OS save and restore register are optional in debug architecture v7, > so check the status register before attempting to clear the OS lock. > > Signed-off-by: Will Deacon <will.deacon@arm.com> Tested-by: Stephen Boyd <sboyd@codeaurora.org> > --- > arch/arm/kernel/hw_breakpoint.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c > index 281bf33..ec16ada 100644 > --- a/arch/arm/kernel/hw_breakpoint.c > +++ b/arch/arm/kernel/hw_breakpoint.c > @@ -929,6 +929,13 @@ static void reset_ctrl_regs(void *unused) > asm volatile("mrc p14, 0, %0, c1, c5, 4" : "=r" (dbg_power)); > if ((dbg_power & 0x1) == 0) > err = -EPERM; > + > + /* > + * Check whether we implement OS save and restore. > + */ > + asm volatile("mrc p14, 0, %0, c1, c1, 4" : "=r" (dbg_power)); minor nit for this series. dbg_power has become a catch-all variable in this code. It would be nice if we named the variables used to hold the read register the same as the register or if we made the name of the variable generic like 'val'.
On Wed, Oct 24, 2012 at 11:53:29PM +0100, Stephen Boyd wrote: > On 10/17/12 08:31, Will Deacon wrote: > > The OS save and restore register are optional in debug architecture v7, > > so check the status register before attempting to clear the OS lock. > > > > Signed-off-by: Will Deacon <will.deacon@arm.com> > > Tested-by: Stephen Boyd <sboyd@codeaurora.org> > > > --- > > arch/arm/kernel/hw_breakpoint.c | 10 +++++++++- > > 1 files changed, 9 insertions(+), 1 deletions(-) > > > > diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c > > index 281bf33..ec16ada 100644 > > --- a/arch/arm/kernel/hw_breakpoint.c > > +++ b/arch/arm/kernel/hw_breakpoint.c > > @@ -929,6 +929,13 @@ static void reset_ctrl_regs(void *unused) > > asm volatile("mrc p14, 0, %0, c1, c5, 4" : "=r" (dbg_power)); > > if ((dbg_power & 0x1) == 0) > > err = -EPERM; > > + > > + /* > > + * Check whether we implement OS save and restore. > > + */ > > + asm volatile("mrc p14, 0, %0, c1, c1, 4" : "=r" (dbg_power)); > > minor nit for this series. dbg_power has become a catch-all variable in > this code. It would be nice if we named the variables used to hold the > read register the same as the register or if we made the name of the > variable generic like 'val'. Thanks for your feedback and tested-bys on this series Stephen, I really appreciate it. I'll send out a v2 and update the branch I'm sending to -next. Cheers, Will
diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c index 281bf33..ec16ada 100644 --- a/arch/arm/kernel/hw_breakpoint.c +++ b/arch/arm/kernel/hw_breakpoint.c @@ -929,6 +929,13 @@ static void reset_ctrl_regs(void *unused) asm volatile("mrc p14, 0, %0, c1, c5, 4" : "=r" (dbg_power)); if ((dbg_power & 0x1) == 0) err = -EPERM; + + /* + * Check whether we implement OS save and restore. + */ + asm volatile("mrc p14, 0, %0, c1, c1, 4" : "=r" (dbg_power)); + if ((dbg_power & 0x9) == 0) + goto clear_vcr; break; case ARM_DEBUG_ARCH_V7_1: /* @@ -947,7 +954,7 @@ static void reset_ctrl_regs(void *unused) } /* - * Unconditionally clear the lock by writing a value + * Unconditionally clear the OS lock by writing a value * other than 0xC5ACCE55 to the access register. */ asm volatile("mcr p14, 0, %0, c1, c0, 4" : : "r" (0)); @@ -957,6 +964,7 @@ static void reset_ctrl_regs(void *unused) * Clear any configured vector-catch events before * enabling monitor mode. */ +clear_vcr: asm volatile("mcr p14, 0, %0, c0, c7, 0" : : "r" (0)); isb();
The OS save and restore register are optional in debug architecture v7, so check the status register before attempting to clear the OS lock. Signed-off-by: Will Deacon <will.deacon@arm.com> --- arch/arm/kernel/hw_breakpoint.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)