Re: Single-stepping with UBC on SH7785

[Thomas Schwinge <thomas@codesourcery.com>]

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

Hi!

Ping.

On Tue, 14 Feb 2012 17:18:52 +0100, I wrote:
> Upgrading the kernel for it, we noticed that with ``recent'' kernels,
> single-stepping is broken for our SH7785-based board.  I tracked this
> down to commit 09a072947791088b88ae15111cf68fc5aaaf758d (2009-11-09),
> ``preliminary support for the SH-4A UBC to the hw-breakpoints API''.
> 
> After learning from the SH7785 manual how to use and program the UBC, it
> seemed obvious to me that what it implemented nowadays in
> arch/sh/kernel/cpu/sh4a/ubc.c for programming the UBC does not match how
> it used to be, so I changed that according to my interpretation of the
> manual, as well as the pre-09a072's
> arch/sh/kernel/process_32.c:__switch_to implementation.  This is the
> attached 0001-WIP-Restore-single-stepping-functionality-on-our-SH7.patch.
> Yet, it still wouldn't work.
> 
> 
> After several hours of grief, I came up with the additional
> 0001-Wire-the-clock-of-the-SH7785-s-UBC-as-expected-in-ub.patch -- and it
> worked!  (Meh, so simple...)
> 
> 
> ... and today I figured out that my first patch isn't even needed -- but
> I don't understand how the current ubc.c implementation gets away with
> not using the asid stuff, for example?  And shouldn't it respect the
> reserved value UBC_CRR_RES as well as UBC_CRR_INIT and UBC_CBR_INIT that
> I re-introduced?  Also the manual suggests a different order for
> programming the registers.
> 
> As soon as someone starts working on adding user-space controlled
> hardware breakpoint and/or watchpoint support, this will need further
> untangling/cleanup.
> 
> 
> Grüße,
>  Thomas
> 
> 
> From 2942691b300a14eb3751aa687165e1c00da3cedd Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge <thomas@codesourcery.com>
> Date: Tue, 14 Feb 2012 16:17:47 +0100
> Subject: [PATCH] [WIP] Restore single-stepping functionality on our
>  SH7785-based board.
> 
> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
> ---
>  arch/sh/kernel/cpu/sh4a/ubc.c |   67 ++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 66 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/sh/kernel/cpu/sh4a/ubc.c b/arch/sh/kernel/cpu/sh4a/ubc.c
> index efb2745..86c54cc 100644
> --- a/arch/sh/kernel/cpu/sh4a/ubc.c
> +++ b/arch/sh/kernel/cpu/sh4a/ubc.c
> @@ -14,6 +14,7 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <asm/hw_breakpoint.h>
> +#include <asm/mmu_context.h>
>  
>  #define UBC_CBR(idx)	(0xff200000 + (0x20 * idx))
>  #define UBC_CRR(idx)	(0xff200004 + (0x20 * idx))
> @@ -24,30 +25,83 @@
>  #define UBC_CBCR	0xff200620
>  
>  /* CRR */
> +#define UBC_CRR_RES	(1 << 13)
>  #define UBC_CRR_PCB	(1 << 1)
>  #define UBC_CRR_BIE	(1 << 0)
> +#define UBC_CRR_INIT	0x00002000
>  
>  /* CBR */
> +#define UBC_CBR_AIE	(1 << 30)
> +#define UBC_CBR_ID_INST	(1 << 4)
>  #define UBC_CBR_CE	(1 << 0)
> +#define UBC_CBR_INIT	0x20000000
> +
> +#define UBC_CBR_AIV_SET(asid)	(((asid) << UBC_CBR_AIV_SHIFT) & UBC_CBR_AIV_MASK)
> +#define UBC_CBR_AIV_SHIFT	16
> +#define UBC_CBR_AIV_MASK	0x00ff0000
>  
>  static struct sh_ubc sh4a_ubc;
>  
>  static void sh4a_ubc_enable(struct arch_hw_breakpoint *info, int idx)
>  {
> +#if 0
>  	__raw_writel(UBC_CBR_CE | info->len | info->type, UBC_CBR(idx));
> +#else
> +	int asid = 0;
> +	unsigned long tmp;
> +
> +	printk("0x%p\t%s for 0x%lx on channel %d\n", current, __FUNCTION__, info->address, idx);
> +
> +//#ifdef CONFIG_MMU
> +//	asid = cpu_asid(smp_processor_id(), current->mm);
> +	asid = get_asid();
> +//#endif
> +
> +	tmp = UBC_CBR_CE
> +		| UBC_CBR_ID_INST
> +		| UBC_CBR_AIE
> +		| UBC_CBR_AIV_SET(asid)
> +		/* | info->len */
> +		/* | info->type */ | (1 << 1);
> +	__raw_writel(tmp, UBC_CBR(idx));
> +	printk("  asid 0x%x, CBR: %lx", asid, tmp);
> +#endif
>  	__raw_writel(info->address, UBC_CAR(idx));
> +
> +	__raw_writel(0, UBC_CAMR(idx));
> +
> +//	__raw_writel(0, UBC_CBCR);
> +
> +	tmp = UBC_CRR_BIE
> +		| UBC_CRR_PCB
> +		| UBC_CRR_RES;
> +	__raw_writel(tmp, UBC_CRR(idx));
> +	printk(", CRR: %lx\n", tmp);
> +
> +	/* dummy read for write posting */
> +	(void) __raw_readl(UBC_CBR(idx));
> +	(void) __raw_readl(UBC_CRR(idx));
>  }
>  
>  static void sh4a_ubc_disable(struct arch_hw_breakpoint *info, int idx)
>  {
> +	printk("0x%p\t%s for 0x%lx on channel %d\n", current, __FUNCTION__, info->address, idx);
> +
> +#if 0
>  	__raw_writel(0, UBC_CBR(idx));
>  	__raw_writel(0, UBC_CAR(idx));
> +#else
> +	__raw_writel(UBC_CBR_INIT, UBC_CBR(idx));
> +	__raw_writel(UBC_CRR_INIT, UBC_CRR(idx));
> +#endif
>  }
>  
>  static void sh4a_ubc_enable_all(unsigned long mask)
>  {
>  	int i;
>  
> +	printk("0x%p\t%s with mask 0x%lx\n", current, __FUNCTION__, mask);
> +
>  	for (i = 0; i < sh4a_ubc.num_events; i++)
>  		if (mask & (1 << i))
>  			__raw_writel(__raw_readl(UBC_CBR(i)) | UBC_CBR_CE,
> @@ -58,6 +112,8 @@ static void sh4a_ubc_disable_all(void)
>  {
>  	int i;
>  
> +	printk("0x%p\t%s\n", current, __FUNCTION__);
> +
>  	for (i = 0; i < sh4a_ubc.num_events; i++)
>  		__raw_writel(__raw_readl(UBC_CBR(i)) & ~UBC_CBR_CE,
>  			     UBC_CBR(i));
> @@ -68,6 +124,8 @@ static unsigned long sh4a_ubc_active_mask(void)
>  	unsigned long active = 0;
>  	int i;
>  
> +	printk("0x%p\t%s\n", current, __FUNCTION__);
> +
>  	for (i = 0; i < sh4a_ubc.num_events; i++)
>  		if (__raw_readl(UBC_CBR(i)) & UBC_CBR_CE)
>  			active |= (1 << i);
> @@ -77,11 +135,15 @@ static unsigned long sh4a_ubc_active_mask(void)
>  
>  static unsigned long sh4a_ubc_triggered_mask(void)
>  {
> +	printk("0x%p\t%s\n", current, __FUNCTION__);
> +
>  	return __raw_readl(UBC_CCMFR);
>  }
>  
>  static void sh4a_ubc_clear_triggered_mask(unsigned long mask)
>  {
> +	printk("0x%p\t%s with mask 0x%lx\n", current, __FUNCTION__, mask);
> +
>  	__raw_writel(__raw_readl(UBC_CCMFR) & ~mask, UBC_CCMFR);
>  }
>  
> @@ -114,15 +176,18 @@ static int __init sh4a_ubc_init(void)
>  
>  	__raw_writel(0, UBC_CBCR);
>  
> +#if 0
>  	for (i = 0; i < sh4a_ubc.num_events; i++) {
>  		__raw_writel(0, UBC_CAMR(i));
>  		__raw_writel(0, UBC_CBR(i));
>  
> -		__raw_writel(UBC_CRR_BIE | UBC_CRR_PCB, UBC_CRR(i));
> +//		__raw_writel(UBC_CRR_BIE | UBC_CRR_PCB, UBC_CRR(i));
> +		__raw_writel(UBC_CRR_BIE | UBC_CRR_PCB | UBC_CRR_RES, UBC_CRR(i));
>  
>  		/* dummy read for write posting */
>  		(void)__raw_readl(UBC_CRR(i));
>  	}
> +#endif
>  
>  	clk_disable(ubc_iclk);
>  
> -- 
> 1.7.5.4
> 
> From c6696f9fcffcce6739449ea681a38c30e4799017 Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge <thomas@codesourcery.com>
> Date: Tue, 14 Feb 2012 16:19:49 +0100
> Subject: [PATCH] Wire the clock of the SH7785's UBC as expected in ubc.c.
> 
> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
> ---
>  arch/sh/kernel/cpu/sh4a/clock-sh7785.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7785.c b/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
> index e5b420c..2b31443 100644
> --- a/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
> +++ b/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
> @@ -156,7 +156,7 @@ static struct clk_lookup lookups[] = {
>  	CLKDEV_CON_ID("siof_fck", &mstp_clks[MSTP003]),
>  	CLKDEV_CON_ID("hspi_fck", &mstp_clks[MSTP002]),
>  	CLKDEV_CON_ID("hudi_fck", &mstp_clks[MSTP119]),
> -	CLKDEV_CON_ID("ubc_fck", &mstp_clks[MSTP117]),
> +	CLKDEV_CON_ID("ubc0", &mstp_clks[MSTP117]),
>  	CLKDEV_CON_ID("dmac_11_6_fck", &mstp_clks[MSTP105]),
>  	CLKDEV_CON_ID("dmac_5_0_fck", &mstp_clks[MSTP104]),
>  	CLKDEV_CON_ID("gdta_fck", &mstp_clks[MSTP100]),
> -- 
> 1.7.5.4
> 


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]