FW: [PATCH 1/5] OMAP1/2/3/4: DEBUG_LL: cleanup

Pandita, Vikram vikram.pandita at ti.com
Fri Sep 18 17:22:11 EDT 2009


>-----Original Message-----
>From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
>Sent: Thursday, September 17, 2009 2:48 PM
>To: Pandita, Vikram
>Cc: Kevin Hilman; linux-omap at vger.kernel.org; linux-arm-kernel at lists.arm.linux.org.uk
>Subject: Re: [PATCH 1/5] OMAP1/2/3/4: DEBUG_LL: cleanup
>
>On Fri, Sep 18, 2009 at 12:30:25AM +0530, Pandita, Vikram wrote:
>> There are review comments by Kevin [2] still getting fixed,
>> but a 'Russell-look' will surely help to the in-lined patch [3] .
>
>Still not good.

[posting to the right arm-list]

Russell 
I have considered all your comments for this patch and 
am working on a new approach.

I should be able to post V3 patch set in a day or so, 
which does not touch any of the common files.


>
>> +		mov	r9, r0
>> +
>> +		bl	get_uart_base		@ get uart phy address
>> +		adr     r2, __dummy
>> +		str	r0, [r2]		@save uart phy adder in memory
>> +		ldm	r2, {r13}^		@save phyadder in usermode reg
>> +
>> +		bl	get_uart_virt_base	@ get uart virtual address
>> +		adr     r2, __dummy
>> +		str	r0, [r2]		@save uart phy adder in memory
>> +		ldm	r2, {r14}^		@save phyadder in usermode reg
>> +
>> +		mov	r0, r9
>> +
>>  		bl	cache_clean_flush
>>  		add	pc, r5, r0		@ call relocation code
>>
>> @@ -303,6 +317,9 @@  LC0:		.word	LC0			@ r1
>>  LC1:		.word	reloc_end - reloc_start
>>  		.size	LC0, . - LC0
>>
>> +		.type   __dummy, #object
>> +__dummy:		.word	__dummy
>
>This appears to be in the text segment, and is written to.  That's not
>going to work if the text segment is stored in flash (and in fact might
>be dangerous in some cases.)
>
>> +
>>  #ifdef CONFIG_ARCH_RPC
>>  		.globl	params
>>  params:		ldr	r0, =params_phys
>> diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
>> index 17153b5..0758656 100644
>> --- a/arch/arm/boot/compressed/misc.c
>> +++ b/arch/arm/boot/compressed/misc.c
>> @@ -22,6 +22,15 @@  unsigned int __machine_arch_type;
>>  #include <linux/types.h>	/* for size_t */
>>  #include <linux/stddef.h>	/* for NULL */
>>  #include <asm/string.h>
>> +#include <asm/mach-types.h>
>> +/* TODO:
>> + * Include of this header is not working.
>> + * Gives Error: undefined reference to `omap_rev'
>> + * This header is needed for constant:
>> + * ZOOM2_EXT_QUART_VIRT = 0xFB000000
>> + * ZOOM2_EXT_QUART_PHYS = 0x10000000
>> + */
>> +/* #include <mach/io.h> */
>>
>>  #ifdef STANDALONE_DEBUG
>>  #define putstr printf
>> @@ -316,6 +325,103 @@  static void error(char *x)
>>
>>  #ifndef STANDALONE_DEBUG
>>
>> +u32  *omap_uart_debug_ll_phy_addr;
>> +
>> +static void find_debug_ll_uart_port(unsigned int arch_type)
>> +{
>> +	omap_uart_debug_ll_phy_addr = 0;
>> +
>> +	/* Add logic here for new platforms, using __macine_arch_type */
>> +
>> +	/* TODO: REVISIT -- Check Completeness
>> +	 * DEFINE PHY ADDRESS for EACH BOARD HERE: omap1/2/3/4 */
>> +#if defined(CONFIG_ARCH_OMAP1)
>> +	switch (arch_type) {
>> +	case MACH_TYPE_OMAP_PALMTT:
>> +	case MACH_TYPE_SX1:
>> +		/* UART2 */
>> +		omap_uart_debug_ll_phy_addr = (u32 *)0xfffb0800;
>> +	break;
>> +	default:
>> +			/* UART1 */
>> +		omap_uart_debug_ll_phy_addr = (u32 *)0xfffb0000;
>> +	break;
>> +	}
>> +#endif
>> +
>> +#if defined(CONFIG_ARCH_OMAP2)
>> +	switch (arch_type) {
>> +	case MACH_TYPE_NOKIA_N800:
>> +	case MACH_TYPE_NOKIA_N810:
>> +	case MACH_TYPE_NOKIA_N810_WIMAX:
>> +		/* UART3 */
>> +		omap_uart_debug_ll_phy_addr = (u32 *)0x4806e000;
>> +	break;
>> +	default:
>> +		/* UART1 */
>> +		omap_uart_debug_ll_phy_addr = (u32 *)0x4806a000;
>> +	break;
>> +	}
>> +#endif
>> +
>> +#if defined(CONFIG_ARCH_OMAP3)
>> +	switch (arch_type) {
>> +	case MACH_TYPE_OMAP_LDP:
>> +	case MACH_TYPE_OVERO:
>> +	case MACH_TYPE_OMAP3_PANDORA:
>> +	case MACH_TYPE_NOKIA_RX51:
>> +	case MACH_TYPE_OMAP3_BEAGLE:
>> +		/* UART3 */
>> +		omap_uart_debug_ll_phy_addr = (u32 *)0x49020000;
>> +	break;
>> +	case MACH_TYPE_OMAP_ZOOM2:
>> +		/* EXTERNEL UART */
>> +		omap_uart_debug_ll_phy_addr = (u32 *)0x10000000;
>> +	break;
>> +	default:
>> +		/* UART1 */
>> +		omap_uart_debug_ll_phy_addr = (u32 *)0x4806a000;
>> +	break;
>> +	}
>> +#endif
>> +
>> +#ifdef CONFIG_ARCH_OMAP4
>> +	switch (arch_type) {
>> +	/* OMAP3: UART1 */
>> +	case MACH_TYPE_OMAP_4430SDP:
>> +	default:
>> +		omap_uart_debug_ll_phy_addr = (u32 *)0x4806a000;
>> +		putstr("This is an OMAP4 ...\n");
>> +	break;
>> +	}
>> +#endif
>> +}
>> +
>> +ulg
>> +get_uart_base(void)
>> +{
>> +	return (ulg)omap_uart_debug_ll_phy_addr;
>> +}
>> +
>> +ulg
>> +get_uart_virt_base(void)
>> +{
>> +	ulg val = 0;
>> +
>> +#ifdef CONFIG_ARCH_OMAP1
>> +	/* omap1 */
>> +	val = 0xfef00000;
>> +#else
>> +	/* omap2/3/4... */
>> +	if (MACH_TYPE_OMAP_ZOOM2 == __machine_arch_type)
>> +		val = 0xFB000000; /* ZOOM2_EXT_QUART_VIRT */
>> +	else
>> +		val = 0xd8000000;
>> +#endif
>> +
>> +	return (ulg)(((val) >> 18) & 0xfffc);
>> +}
>> +
>
>Clearly, all this code should not be in here.  There's a perfectly good
>header file to contain the platform specifics which it can live in, and
>there's a perfectly good hook for it to latch into (arch_decomp_setup).
>
>> diff --git a/arch/arm/plat-omap/include/mach/debug-macro.S b/arch/arm/plat-omap/include/mach/debug-
>macro.S
>> index ac24050..ba80ca4 100644
>> --- a/arch/arm/plat-omap/include/mach/debug-macro.S
>> +++ b/arch/arm/plat-omap/include/mach/debug-macro.S
>> @@ -10,43 +10,42 @@
>>   * published by the Free Software Foundation.
>>   *
>>  */
>> +#include "io.h"
>> +
>> +		.align
>> +		.type   __phy_uart_addr, #object
>> +__phy_uart_addr:       .word   0xFF
>> +		.type   __virt_uart_addr, #object
>> +__virt_uart_addr:       .word   0xFF
>
>Again, this ends up in the text segment, and in the XIP case, the text
>segment is not writable (writes hit the flash chip.)
>
>> diff --git a/arch/arm/plat-omap/include/mach/uncompress.h b/arch/arm/plat-
>omap/include/mach/uncompress.h
>> index 0814c5f..9121d7a 100644
>> --- a/arch/arm/plat-omap/include/mach/uncompress.h
>> +++ b/arch/arm/plat-omap/include/mach/uncompress.h
>> @@ -21,7 +21,8 @@
>>  #include <linux/serial_reg.h>
>>  #include <mach/serial.h>
>>
>> -unsigned int system_rev;
>> +extern u32 *omap_uart_debug_ll_phy_addr;
>> +extern unsigned int __machine_arch_type;
>
>Just include asm/mach-types.h - there's no need to declare this here.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




More information about the linux-arm-kernel mailing list