[PATCH 1/2] ARM: Add inituart macro to initialize DEBUG_LL serial port based on the machine ID

[Nicolas Pitre]

On Thu, 3 Feb 2011, Tony Lindgren wrote:

> Allow machine specific init of the DEBUG_LL serial port. This is needed
> to debug kernels built with support for multiple machines compiled in
> without recompiling the kernel.
> 
> As some SoCs need to use variables to store the port address and LSR
> register configuration, we must call inituart again after clearing
> the BSS.
> 
> If variables are being used for for inituart and addruart, this setup
> won't work properly for debugging the uncompress code without patching
> arch/arm/boot/compressed/head.S for the machine in question.
> 
> For adding DEBUG_LL support for new machines, only two lines are needed
> in debug-macro.S:
> 
> 	.macro	inituart, id, a, v
> 	...
> 	ldr	\v, =MACH_TYPE_SOMETHING1
> 	cmp	\id, \v
> 	ldrne	\v, =MACH_TYPE_SOMETHING2
> 	cmpne	\id, \v
> 	ldrne	\v, =MACH_TYPE_SOMETHING3
> 	cmpne	\id, \v
> 	...
> 	.endm
> 
> Note that this patch depends on a patch to add empty inituart macro for
> each debug-macro.S file.
> 
> Signed-off-by: Tony Lindgren <tony at atomide.com>
> ---
>  arch/arm/kernel/head-common.S |    7 +++++++
>  arch/arm/kernel/head.S        |    5 +++++
>  2 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index c84b57d..0b66d6b 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -63,6 +63,9 @@ ENDPROC(__vet_atags)
>   *  r1  = machine ID
>   *  r2  = atags pointer
>   *  r9  = processor ID
> + *
> + * Note that if the machine uses BSS to store the debug UART configuration,
> + * debug_ll code will not work after clear BSS until inituart is called again.
>   */

Please make sure that inituart always uses .data to store its needed 
configuration instead.

>  	__INIT
>  __mmap_switched:
> @@ -80,6 +83,10 @@ __mmap_switched:
>  	strcc	fp, [r6],#4
>  	bcc	1b
>  
> +#if defined(CONFIG_DEBUG_LL) && !defined(CONFIG_DEBUG_ICEDCC)
> +	inituart r1, r4, r5
> +#endif
> +
>   ARM(	ldmia	r3, {r4, r5, r6, r7, sp})
>   THUMB(	ldmia	r3, {r4, r5, r6, r7}	)
>   THUMB(	ldr	sp, [r3, #16]		)
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 591a2ea..be64264 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -75,6 +75,11 @@
>  ENTRY(stext)
>  	setmode	PSR_F_BIT | PSR_I_BIT | SVC_MODE, r9 @ ensure svc mode
>  						@ and irqs disabled
> +
> +#if defined(CONFIG_DEBUG_LL) && !defined(CONFIG_DEBUG_ICEDCC)
> +	inituart r1, r3, r4
> +#endif
> +

Here you have two calls, one which is performed when the MMU is off, and 
the other one (after .bss is cleared) when the MMU is on.  Did you 
consider the implications of accessing that .bss variable in those two 
circumstances?  This is also true for when printascii is called.

Ensuring that .data is used instead of .bss would eliminate one of those 
calls which is a good thing to do.  But extra cleverness is needed to 
retrieve the config data in a position independent way.


Nicolas