Commit 0c8219f0... breaks omap7xx support

Commit 0c8219f0... breaks omap7xx support

[Cory Maccarrone]

All,

I've finally gotten some time to test the latest linux-omap code, and
found the following commit:

    4f2c49fedf... omap: Clean the serial port defines

breaks current omap7xx support.  To get linux-omap master to boot, I
had to revert that, and the three two that follow it:

    0c8219f03... omap: Make uncompress code and DEBUG_LL code generic
    61b603b4e...omap: Remove old DEBUG_LL serial port options

in order to boot with DEBUG_LL enabled.  Disabled, I only needed to
revert the last two.

I don't know enough about that part of the kernel to debug it, so all
I can offer is a test result.  For reference, this was on
board-htcherald using the htcherald_defconfig settings.

- Cory

Re: Commit 0c8219f0... breaks omap7xx support

[Tony Lindgren]

* Cory Maccarrone <darkstar6262@gmail.com> [100306 20:53]:
> All,
> 
> I've finally gotten some time to test the latest linux-omap code, and
> found the following commit:
> 
>     4f2c49fedf... omap: Clean the serial port defines
> 
> breaks current omap7xx support.  To get linux-omap master to boot, I
> had to revert that, and the three two that follow it:
> 
>     0c8219f03... omap: Make uncompress code and DEBUG_LL code generic
>     61b603b4e...omap: Remove old DEBUG_LL serial port options
> 
> in order to boot with DEBUG_LL enabled.  Disabled, I only needed to
> revert the last two.

Hmm OK thanks for bisecting this. The first patch seems to have
a bug for uncompress.h on non omap3 platforms, see the attached
patch.

Are you sure you have CONFIG_EARLY_PRINTK and earlyprintk also
in your cmdline?

> I don't know enough about that part of the kernel to debug it, so all
> I can offer is a test result.  For reference, this was on
> board-htcherald using the htcherald_defconfig settings.

Can you try the following:

- Use the current mainline kernel head
- make sure you have DEBUG_LL and EARLY_PRINTK enabled in your .config
- check that you have earlyprintk also in the kernel cmdline
- no other serial patches applied

If no serial output, please temporarily hardcode the values
for your 7xx board in mach-omap1/include/mach/debug-macro.S for
omap_uart_phys and omap_uart_virt. Maybe that will give some
more clues.

If you don't have any serial output with that, then try this:

- reset some branch to 4f2c49fedf
- make sure you have DEBUG_LL and EARLY_PRINTK enabled in your .config
- check that you have earlyprintk also in the kernel cmdline
- apply the following patch for the uncompress code

That way you might be able to track down further where things
break with the patches following 4f2c49fedf.

Regards,

Tony

--- a/arch/arm/plat-omap/include/plat/uncompress.h
+++ b/arch/arm/plat-omap/include/plat/uncompress.h
@@ -42,11 +42,11 @@ static void putc(int c)
 #ifdef CONFIG_ARCH_OMAP
 	/* Will get removed in the next patch, set to OMAP3 to compile */
 #ifdef	CONFIG_OMAP_LL_DEBUG_UART3
-	uart = (volatile u8 *)(OMAP3_UART3_BASE);
+	uart = (volatile u8 *)(OMAP1_UART3_BASE);
 #elif defined(CONFIG_OMAP_LL_DEBUG_UART2)
-	uart = (volatile u8 *)(OMAP3_UART2_BASE);
+	uart = (volatile u8 *)(OMAP1_UART2_BASE);
 #elif defined(CONFIG_OMAP_LL_DEBUG_UART1)
-	uart = (volatile u8 *)(OMAP3_UART1_BASE);
+	uart = (volatile u8 *)(OMAP1_UART1_BASE);
 #elif defined(CONFIG_OMAP_LL_DEBUG_NONE)
 	return;
 #else

Re: Commit 0c8219f0... breaks omap7xx support

[Cory Maccarrone]

On Mon, Mar 8, 2010 at 4:48 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Cory Maccarrone <darkstar6262@gmail.com> [100306 20:53]:
>> All,
>>
>> I've finally gotten some time to test the latest linux-omap code, and
>> found the following commit:
>>
>>     4f2c49fedf... omap: Clean the serial port defines
>>
>> breaks current omap7xx support.  To get linux-omap master to boot, I
>> had to revert that, and the three two that follow it:
>>
>>     0c8219f03... omap: Make uncompress code and DEBUG_LL code generic
>>     61b603b4e...omap: Remove old DEBUG_LL serial port options
>>
>> in order to boot with DEBUG_LL enabled.  Disabled, I only needed to
>> revert the last two.
>
> Hmm OK thanks for bisecting this. The first patch seems to have
> a bug for uncompress.h on non omap3 platforms, see the attached
> patch.
>
> Are you sure you have CONFIG_EARLY_PRINTK and earlyprintk also
> in your cmdline?
>

I am now :)  It crashes equally with those enabled.

>> I don't know enough about that part of the kernel to debug it, so all
>> I can offer is a test result.  For reference, this was on
>> board-htcherald using the htcherald_defconfig settings.
>
> Can you try the following:
>
> - Use the current mainline kernel head
> - make sure you have DEBUG_LL and EARLY_PRINTK enabled in your .config
> - check that you have earlyprintk also in the kernel cmdline
> - no other serial patches applied
>

No output, kernel crashes, watchdog reboots the device with this setup.

> If no serial output, please temporarily hardcode the values
> for your 7xx board in mach-omap1/include/mach/debug-macro.S for
> omap_uart_phys and omap_uart_virt. Maybe that will give some
> more clues.
>

Same result as above with the hardcoding.

> If you don't have any serial output with that, then try this:
>
> - reset some branch to 4f2c49fedf
> - make sure you have DEBUG_LL and EARLY_PRINTK enabled in your .config
> - check that you have earlyprintk also in the kernel cmdline
> - apply the following patch for the uncompress code
>

This worked.  I was able to boot in with DEBUG_LL enabled and
everything appears to be working properly.

> That way you might be able to track down further where things
> break with the patches following 4f2c49fedf.
>
> Regards,
>
> Tony
>
> --- a/arch/arm/plat-omap/include/plat/uncompress.h
> +++ b/arch/arm/plat-omap/include/plat/uncompress.h
> @@ -42,11 +42,11 @@ static void putc(int c)
>  #ifdef CONFIG_ARCH_OMAP
>        /* Will get removed in the next patch, set to OMAP3 to compile */
>  #ifdef CONFIG_OMAP_LL_DEBUG_UART3
> -       uart = (volatile u8 *)(OMAP3_UART3_BASE);
> +       uart = (volatile u8 *)(OMAP1_UART3_BASE);
>  #elif defined(CONFIG_OMAP_LL_DEBUG_UART2)
> -       uart = (volatile u8 *)(OMAP3_UART2_BASE);
> +       uart = (volatile u8 *)(OMAP1_UART2_BASE);
>  #elif defined(CONFIG_OMAP_LL_DEBUG_UART1)
> -       uart = (volatile u8 *)(OMAP3_UART1_BASE);
> +       uart = (volatile u8 *)(OMAP1_UART1_BASE);
>  #elif defined(CONFIG_OMAP_LL_DEBUG_NONE)
>        return;
>  #else
>

Any other hints?  I'm going to start looking at the changes, I'm
beginning to understand them more.

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

Re: Commit 0c8219f0... breaks omap7xx support

[Tony Lindgren]

* Cory Maccarrone <darkstar6262@gmail.com> [100308 17:49]:
> On Mon, Mar 8, 2010 at 4:48 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Cory Maccarrone <darkstar6262@gmail.com> [100306 20:53]:
> 
> > If you don't have any serial output with that, then try this:
> >
> > - reset some branch to 4f2c49fedf
> > - make sure you have DEBUG_LL and EARLY_PRINTK enabled in your .config
> > - check that you have earlyprintk also in the kernel cmdline
> > - apply the following patch for the uncompress code
> >
> 
> This worked.  I was able to boot in with DEBUG_LL enabled and
> everything appears to be working properly.

OK, then that means the debug-macro.S changes for busyuart macro
are OK.
 
> Any other hints?  I'm going to start looking at the changes, I'm
> beginning to understand them more.

Well the next patch gets rid of the hardcoded port addresses.
The debug port number is stored in uncompress.h based on the
machine ID.

Maybe try this next: Reset your test branch to commit
0c8219f0302d0d27fda52c790d38406801e547ec, then take a look at
arch/arm/plat-omap/include/plat/uncompress.h in a text editor.
Then see if there's an entry for your board in __arch_decomp_setup.

Is the DEBUG_LL_OMAP7XX(1, herald) correct? Is port 1 the right
UART to use?

If there's no entry, then you need to add a DEBUG_LL_OMAP7XX line
suitable for your board.

If you see something like "Uncompressing Linux..." then your know
the uncompress.h part is working. Once you get that working, then
it's time to look at the debug-macro.S again, see the addruart
macro that reads the debug port number from UART1 scratchpad
register.

Regards,

Tony

Re: Commit 0c8219f0... breaks omap7xx support

[Cory Maccarrone]

On Mon, Mar 8, 2010 at 6:46 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Cory Maccarrone <darkstar6262@gmail.com> [100308 17:49]:
>> On Mon, Mar 8, 2010 at 4:48 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Cory Maccarrone <darkstar6262@gmail.com> [100306 20:53]:
>>
>> > If you don't have any serial output with that, then try this:
>> >
>> > - reset some branch to 4f2c49fedf
>> > - make sure you have DEBUG_LL and EARLY_PRINTK enabled in your .config
>> > - check that you have earlyprintk also in the kernel cmdline
>> > - apply the following patch for the uncompress code
>> >
>>
>> This worked.  I was able to boot in with DEBUG_LL enabled and
>> everything appears to be working properly.
>
> OK, then that means the debug-macro.S changes for busyuart macro
> are OK.
>
>> Any other hints?  I'm going to start looking at the changes, I'm
>> beginning to understand them more.
>
> Well the next patch gets rid of the hardcoded port addresses.
> The debug port number is stored in uncompress.h based on the
> machine ID.
>
> Maybe try this next: Reset your test branch to commit
> 0c8219f0302d0d27fda52c790d38406801e547ec, then take a look at
> arch/arm/plat-omap/include/plat/uncompress.h in a text editor.
> Then see if there's an entry for your board in __arch_decomp_setup.
>
> Is the DEBUG_LL_OMAP7XX(1, herald) correct? Is port 1 the right
> UART to use?
>

That line is correct, as far as I'm aware.  It resolves to the same
configuration that was there previously.

> If there's no entry, then you need to add a DEBUG_LL_OMAP7XX line
> suitable for your board.
>
> If you see something like "Uncompressing Linux..." then your know
> the uncompress.h part is working. Once you get that working, then
> it's time to look at the debug-macro.S again, see the addruart
> macro that reads the debug port number from UART1 scratchpad
> register.
>
> Regards,
>
> Tony
>
>

Alright, I found it.  0c8219f0302d0d27fda52c790d38406801e547ec removed
this code from uncompress.h:

    -               if (check_port(uart, shift))
    -                       break;

which was in a while loop in putc(), with check_port defined as:

    #define UART_OMAP_MDR1         0x08    /* mode definition register */
    #define check_port(base, shift) ((base[UART_OMAP_MDR1 << shift] & 7) == 0)

and shift = 0.  Without the break, putc() was silent.  If I put in
similar code back into putc():

    +       if ((uart_base[0x08] & 7) != 0)
    +               return;

it boots.

Unfortunately, I don't have access to the serial console, so I have no
idea if any of the serial port work was actually working (the Herald
is a phone, and the only output I have is USB, which can act as
serial, but I'm ill-equipped to make that work).  I suspect this is
always returning true, and as such we're not getting serial output
through the uncompress.  Not sure what's needed to fix this, but at
least that little bit of code makes it boot for me.

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

Re: Commit 0c8219f0... breaks omap7xx support

[Tony Lindgren]

* Cory Maccarrone <darkstar6262@gmail.com> [100308 19:03]:
> On Mon, Mar 8, 2010 at 6:46 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> Alright, I found it.  0c8219f0302d0d27fda52c790d38406801e547ec removed
> this code from uncompress.h:
> 
>     -               if (check_port(uart, shift))
>     -                       break;
> 
> which was in a while loop in putc(), with check_port defined as:
> 
>     #define UART_OMAP_MDR1         0x08    /* mode definition register */
>     #define check_port(base, shift) ((base[UART_OMAP_MDR1 << shift] & 7) == 0)
> 
> and shift = 0.  Without the break, putc() was silent.  If I put in
> similar code back into putc():
> 
>     +       if ((uart_base[0x08] & 7) != 0)
>     +               return;
> 
> it boots.
> 
> Unfortunately, I don't have access to the serial console, so I have no
> idea if any of the serial port work was actually working (the Herald
> is a phone, and the only output I have is USB, which can act as
> serial, but I'm ill-equipped to make that work).  I suspect this is
> always returning true, and as such we're not getting serial output
> through the uncompress.  Not sure what's needed to fix this, but at
> least that little bit of code makes it boot for me.

OK. It's not a bug it's a feature :)

If you don't have anything connected to the serial port, then
DEBUG_LL should not do anything as it just waits for fifo to empty.

Sounds like we should either have most of uncompress.h in ifdef
CONFIG_DEBUG_LL so it does not do anything unless set.

We may also want to also define DEBUG_LL_OMAP_NONE in uncompress.h
to allow booting systems with no serial port with DEBUG_LL set,
but I don't know if that's really needed.

Does your system boot without any patches if DEBUG_LL is not set
in your .config?

Regards,

Tony 

Re: Commit 0c8219f0... breaks omap7xx support

[Cory Maccarrone]

On Tue, Mar 9, 2010 at 7:56 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Cory Maccarrone <darkstar6262@gmail.com> [100308 19:03]:
>> On Mon, Mar 8, 2010 at 6:46 PM, Tony Lindgren <tony@atomide.com> wrote:
>>
>> Alright, I found it.  0c8219f0302d0d27fda52c790d38406801e547ec removed
>> this code from uncompress.h:
>>
>>     -               if (check_port(uart, shift))
>>     -                       break;
>>
>> which was in a while loop in putc(), with check_port defined as:
>>
>>     #define UART_OMAP_MDR1         0x08    /* mode definition register */
>>     #define check_port(base, shift) ((base[UART_OMAP_MDR1 << shift] & 7) == 0)
>>
>> and shift = 0.  Without the break, putc() was silent.  If I put in
>> similar code back into putc():
>>
>>     +       if ((uart_base[0x08] & 7) != 0)
>>     +               return;
>>
>> it boots.
>>
>> Unfortunately, I don't have access to the serial console, so I have no
>> idea if any of the serial port work was actually working (the Herald
>> is a phone, and the only output I have is USB, which can act as
>> serial, but I'm ill-equipped to make that work).  I suspect this is
>> always returning true, and as such we're not getting serial output
>> through the uncompress.  Not sure what's needed to fix this, but at
>> least that little bit of code makes it boot for me.
>
> OK. It's not a bug it's a feature :)
>
> If you don't have anything connected to the serial port, then
> DEBUG_LL should not do anything as it just waits for fifo to empty.
>
> Sounds like we should either have most of uncompress.h in ifdef
> CONFIG_DEBUG_LL so it does not do anything unless set.
>
> We may also want to also define DEBUG_LL_OMAP_NONE in uncompress.h
> to allow booting systems with no serial port with DEBUG_LL set,
> but I don't know if that's really needed.
>
> Does your system boot without any patches if DEBUG_LL is not set
> in your .config?
>
> Regards,
>
> Tony
>

No, it doesn't.  Seems like the debugging code is still trying to run,
even disabled.  If I add in that check, it'll boot with DEBUG_LL not
set, but won't without that.

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

Re: Commit 0c8219f0... breaks omap7xx support

[Tony Lindgren]

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

* Cory Maccarrone <darkstar6262@gmail.com> [100309 15:38]:
> On Tue, Mar 9, 2010 at 7:56 AM, Tony Lindgren <tony@atomide.com> wrote:
> >
> > Does your system boot without any patches if DEBUG_LL is not set
> > in your .config?
> 
> No, it doesn't.  Seems like the debugging code is still trying to run,
> even disabled.  If I add in that check, it'll boot with DEBUG_LL not
> set, but won't without that.

Sorry for the delay on this. How about with the following patch with
DEBUG_LL not set?

Note that this is using the shift register now too.

Regards,

Tony

[-- Attachment #2: debug-ll.patch --]
[-- Type: text/x-diff, Size: 1270 bytes --]

>From 240b34f7efbf26ad4f39438d055b0931e8e7496e Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Mon, 15 Mar 2010 10:47:08 -0700
Subject: [PATCH] omap: Add back UART MDR1 check into uncompress.h

Recent DEBUG_LL and uncompress.h changes removed the check_port()
as pointed out by Cory Maccarrone <darkstar6262@gmail.com>.

This causes some boards to not boot, so add back the MDR1 register
check. The MDR1 register tells the mode of omap uart. Based on
an earlier patch by Cory Maccarrone <darkstar6262@gmail.com>.

Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/plat-omap/include/plat/uncompress.h b/arch/arm/plat-omap/include/plat/uncompress.h
index 81d9ec5..dc37b2c 100644
--- a/arch/arm/plat-omap/include/plat/uncompress.h
+++ b/arch/arm/plat-omap/include/plat/uncompress.h
@@ -24,6 +24,8 @@
 
 #include <plat/serial.h>
 
+#define MDR1_MODE_MASK			0x07
+
 static volatile u8 *uart1_base;
 static int uart1_shift;
 
@@ -48,6 +50,10 @@ static void putc(int c)
 	if (!uart_base)
 		return;
 
+	/* Check for UART 16x mode */
+	if ((uart_base[UART_OMAP_MDR1 << uart_shift] & MDR1_MODE_MASK) != 0)
+		return;
+
 	while (!(uart_base[UART_LSR << uart_shift] & UART_LSR_THRE))
 		barrier();
 	uart_base[UART_TX << uart_shift] = c;

Re: Commit 0c8219f0... breaks omap7xx support

[Tony Lindgren]

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

* Tony Lindgren <tony@atomide.com> [100315 11:35]:
> * Cory Maccarrone <darkstar6262@gmail.com> [100309 15:38]:
> > On Tue, Mar 9, 2010 at 7:56 AM, Tony Lindgren <tony@atomide.com> wrote:
> > >
> > > Does your system boot without any patches if DEBUG_LL is not set
> > > in your .config?
> > 
> > No, it doesn't.  Seems like the debugging code is still trying to run,
> > even disabled.  If I add in that check, it'll boot with DEBUG_LL not
> > set, but won't without that.
> 
> Sorry for the delay on this. How about with the following patch with
> DEBUG_LL not set?
> 
> Note that this is using the shift register now too.

Then we may want to do this too for the boards that don't have any UART.

Tony

[-- Attachment #2: debug-ll-no-serial.patch --]
[-- Type: text/x-diff, Size: 1106 bytes --]

>From b33f67e61cc45558cfbda86b3e2032ffbcb2f99c Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Mon, 15 Mar 2010 10:47:08 -0700
Subject: [PATCH] omap: Fix uncompress code for boards with no UART

With the recent DEBUG_LL and uncompress code changes, boards
with no UART configured fail to boot as pointed out by
Cory Maccarrone <darkstar6262@gmail.com>.

Some boards may not have UARTs available at all, so add
DEBUG_LL_NONE option for that.

Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/plat-omap/include/plat/uncompress.h b/arch/arm/plat-omap/include/plat/uncompress.h
index dc37b2c..7d69d39 100644
--- a/arch/arm/plat-omap/include/plat/uncompress.h
+++ b/arch/arm/plat-omap/include/plat/uncompress.h
@@ -66,6 +66,13 @@ static inline void flush(void)
 /*
  * Macros to configure UART1 and debug UART
  */
+
+#define DEBUG_LL_NONE(mach)						\
+	if (machine_is_##mach()) {					\
+		uart_base = 0;						\
+		break;							\
+	}
+
 #define _DEBUG_LL_ENTRY(mach, uart1_phys, uart1_shft,			\
 			dbg_uart, dbg_shft, dbg_id)			\
 	if (machine_is_##mach()) {					\

Re: Commit 0c8219f0... breaks omap7xx support

[Cory Maccarrone]

On Mon, Mar 15, 2010 at 11:29 AM, Tony Lindgren <tony@atomide.com> wrote:
>
> * Cory Maccarrone <darkstar6262@gmail.com> [100309 15:38]:
> > On Tue, Mar 9, 2010 at 7:56 AM, Tony Lindgren <tony@atomide.com> wrote:
> > >
> > > Does your system boot without any patches if DEBUG_LL is not set
> > > in your .config?
> >
> > No, it doesn't.  Seems like the debugging code is still trying to run,
> > even disabled.  If I add in that check, it'll boot with DEBUG_LL not
> > set, but won't without that.
>
> Sorry for the delay on this. How about with the following patch with
> DEBUG_LL not set?
>
> Note that this is using the shift register now too.
>
> Regards,
>
> Tony

Sorry I took so long on this one.  This patch worked beautifully for
me on the latest kernel tree.  Any chance of it getting in for one of
the RCs of this next kernel?

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

Re: Commit 0c8219f0... breaks omap7xx support

[Tony Lindgren]

* Cory Maccarrone <darkstar6262@gmail.com> [100529 06:56]:
> On Mon, Mar 15, 2010 at 11:29 AM, Tony Lindgren <tony@atomide.com> wrote:
> >
> > * Cory Maccarrone <darkstar6262@gmail.com> [100309 15:38]:
> > > On Tue, Mar 9, 2010 at 7:56 AM, Tony Lindgren <tony@atomide.com> wrote:
> > > >
> > > > Does your system boot without any patches if DEBUG_LL is not set
> > > > in your .config?
> > >
> > > No, it doesn't.  Seems like the debugging code is still trying to run,
> > > even disabled.  If I add in that check, it'll boot with DEBUG_LL not
> > > set, but won't without that.
> >
> > Sorry for the delay on this. How about with the following patch with
> > DEBUG_LL not set?
> >
> > Note that this is using the shift register now too.
> >
> > Regards,
> >
> > Tony
> 
> Sorry I took so long on this one.  This patch worked beautifully for
> me on the latest kernel tree.  Any chance of it getting in for one of
> the RCs of this next kernel?

Yeah this we should be able to merge as a fix. Can I add your Testedy-by
to the patch?

Regards,

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