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