linuxsh-dev

strange early_printk - board_setup problem

[Markus B. <sup...@go...>]

Hi,

I tried to get earlyprintk running on SH7720 again and this time I was succ=
essfull.=20
I don't know why it didn't work last time, because I did the same again.

Everything is fine, so far. I was able to fix the error in the SH7720
setup which prevented booting.
But, after that I realised that the board setup wasn't called anymore.

When earlyprintk is disabled, the first lines of the bootlog are:

[ =A0 =A00.000000] Linux version 2.6.23-rc1 (markus@linux-markus) (gcc vers=
ion 3.4.6) #9 Fri Aug 3 11:09:01 CEST 2007
[ =A0 =A00.000000] Booting machvec: mpr2
[ =A0 =A00.000000] Magic Panel Release 2 A.2

The third line is from the board setup.

After enabling earlyprintk:

[ =A0 =A00.000000] Linux version 2.6.23-rc1 (markus@linux-markus) (gcc vers=
ion 3.4.6) #8 Mon Aug 6 13:17:37 CEST 2007
[ =A0 =A00.000000] console [sercon0] enabled
[ =A0 =A00.000000] Booting machvec: ./

Boot seemed to be quite normal, but LEDs and ethernet obviously didn't work.

When I added sh_mv=3Dgeneric to the command line, the only noticeable diffe=
rence was the line
[ =A0 =A00.000000] Booting machvec: generic

When I tried to specify any other string (including the correct mv_name) to=
 sh_mv=3D booting stopped after=20
[ =A0 =A00.000000] console [sercon0] enabled

I added some debug outputs to get_mv_byname in machvec.c to see what
strings get compared, but only=20
[ =A0 =A00.000000] ./
[ =A0 =A00.000000] <NULL>
Seem to be there

Problem occurs with gcc 3.4.6, 4.0.4, 4.1.2, only when earlyprintk is enabl=
ed.
Running "strings" on vmlinux shows that the board setup is really compiled =
in.

This are my changes to earlyprintk.c, but I'm really sure, they are not
responsible for the problem.

Can anybody reproduce this problem on another board?
Any hints for fixing this bug?

Regards

Markus


Note:
This patch isn't meant for inclusion, but comments are of course wellcome.


diff --git a/arch/sh/kernel/early_printk.c b/arch/sh/kernel/early_printk.c
index 9833493..a0ff48c 100644
=2D-- a/arch/sh/kernel/early_printk.c
+++ b/arch/sh/kernel/early_printk.c
@@ -13,6 +13,7 @@
 #include <linux/tty.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/delay.h>
=20
 #ifdef CONFIG_SH_STANDARD_BIOS
 #include <asm/sh_bios.h>
@@ -62,6 +63,11 @@ static struct console bios_console =3D {
 #include <linux/serial_core.h>
 #include "../../../drivers/serial/sh-sci.h"
=20
+#if defined(CONFIG_CPU_SUBTYPE_SH7720)
+#define EPK_SCSMR_VALUE 0x000
+#define EPK_SCBRR_VALUE 0x00C
+#endif
+
 static struct uart_port scif_port =3D {
 	.mapbase	=3D CONFIG_EARLY_SCIF_CONSOLE_PORT,
 	.membase	=3D (char __iomem *)CONFIG_EARLY_SCIF_CONSOLE_PORT,
@@ -69,7 +75,7 @@ static struct uart_port scif_port =3D {
=20
 static void scif_sercon_putc(int c)
 {
=2D	while (((sci_in(&scif_port, SCFDR) & 0x1f00 >> 8) =3D=3D 16))
+	while (((sci_in(&scif_port, SCFDR) & 0x1f00 >> 8) >=3D 16))
 		;
=20
 	sci_out(&scif_port, SCxTDR, c);
@@ -105,7 +111,24 @@ static struct console scif_console =3D {
 	.index		=3D -1,
 };
=20
=2D#if defined(CONFIG_CPU_SH4) && !defined(CONFIG_SH_STANDARD_BIOS)
+#if !defined(CONFIG_SH_STANDARD_BIOS)
+#if defined(CONFIG_CPU_SUBTYPE_SH7720)
+static void scif_sercon_init(char *s)
+{
+	ctrl_outw(0x0000, PORT_PTCR);
+
+	sci_out(&scif_port, SCSCR, 0x0000 );      /* clear TE and RE */
+	sci_out(&scif_port, SCFCR, 0x4006 );      /* reset through TCRST, TFRST, =
RFRST */
+	sci_out(&scif_port, SCSCR, 0x0000 );      /* select internal clock */
+	sci_out(&scif_port, SCSMR, EPK_SCSMR_VALUE );
+	sci_out(&scif_port, SCBRR, EPK_SCBRR_VALUE );
+
+	mdelay(1);
+
+	sci_out(&scif_port, SCFCR, 0x0030 );   /* TTRG=3Db'11 */
+	sci_out(&scif_port, SCSCR, 0x0030 );   /* TE, RE */
+}
+#elif defined(CONFIG_CPU_SH4)
 #define DEFAULT_BAUD 115200
 /*
  * Simple SCIF init, primarily aimed at SH7750 and other similar SH-4
@@ -146,7 +169,8 @@ static void scif_sercon_init(char *s)
 	ctrl_outw(0, scif_port.mapbase + 36);
 	ctrl_outw(0x30, scif_port.mapbase + 8);
 }
=2D#endif /* CONFIG_CPU_SH4 && !CONFIG_SH_STANDARD_BIOS */
+#endif /* defined(CONFIG_CPU_SUBTYPE_SH7720) */
+#endif /* !defined(CONFIG_SH_STANDARD_BIOS) */
 #endif /* CONFIG_EARLY_SCIF_CONSOLE */
=20
 /*
@@ -186,7 +210,8 @@ int __init setup_early_printk(char *buf)
 	if (!strncmp(buf, "serial", 6)) {
 		early_console =3D &scif_console;
=20
=2D#if defined(CONFIG_CPU_SH4) && !defined(CONFIG_SH_STANDARD_BIOS)
+#if (defined(CONFIG_CPU_SH4) || defined(CONFIG_CPU_SUBTYPE_SH7720)) && \
+    !defined(CONFIG_SH_STANDARD_BIOS)
 		scif_sercon_init(buf + 6);
 #endif
 	}

Re: strange early_printk - board_setup problem

[Paul M. <le...@li...>]

On Mon, Aug 06, 2007 at 10:50:53PM +0200, Markus Brunner wrote:
> [ ? ?0.000000] Linux version 2.6.23-rc1 (markus@linux-markus) (gcc version 3.4.6) #8 Mon Aug 6 13:17:37 CEST 2007
> [ ? ?0.000000] console [sercon0] enabled
> [ ? ?0.000000] Booting machvec: ./
> 
> Boot seemed to be quite normal, but LEDs and ethernet obviously didn't work.
> 
> When I added sh_mv=generic to the command line, the only noticeable difference was the line
> [ ? ?0.000000] Booting machvec: generic
> 
> When I tried to specify any other string (including the correct mv_name) to sh_mv= booting stopped after 
> [ ? ?0.000000] console [sercon0] enabled
> 
> I added some debug outputs to get_mv_byname in machvec.c to see what
> strings get compared, but only 
> [ ? ?0.000000] ./
> [ ? ?0.000000] <NULL>
> Seem to be there
> 
That's a bit unusual. Can you print out what the size of the machvec
section is (ie, &__machvec_end - &__machvec_start)? Also, if you give it
a sh_mv=foo, do you get the proper list? Beyond that, what boot loader
are you using? Have you tried this with sh-bios support enabled and
disabled in the kernel? Are the results the same for both? You might also
try bumping the load address up higher in case the boot loader happens to
be stepping on something.

If that doesn't reveal any clues, can you build with debugging symbols
and post your kernel image somewhere so I can disassemble it?

Re: strange early_printk - board_setup problem

[EXTERNAL B. M. (P. ST-FIR/Eng) <ext...@de...>]

...

> Also, if you give it a sh_mv=3Dfoo, do you get the proper list?=20
No.=20
>> When I tried to specify any other string (including the correct =
mv_name) to sh_mv=3D booting stopped after=20
>> [ =A0 =A00.000000] console [sercon0] enabled

> Beyond that, what boot loader are you using?=20
I don't have a real bootloader on the board. I load zImage at address =
0x8c210000 with a JTAG debugger.=20

> You might also try bumping the load address up higher in case=20
> the boot loader happens to be stepping on something.
Afaik no other address will work.

> Have you tried this with sh-bios support  enabled and disabled in the =
kernel?=20
> Are the results the same for both?=20
I don't have a BIOS on the board. Enabling the sh-bios in the kernel =
will break the kernel.
=20
> If that doesn't reveal any clues, can you build with debugging symbols
> and post your kernel image somewhere so I can disassemble it?
Not possible at the moment, but i will do that in about 7 hours.

> That's a bit unusual. Can you print out what the size of the machvec
> section is (ie, &__machvec_end - &__machvec_start)?=20

Yes, and in combination with nm this gives some interesting results:

This is without earlyprintk. Note that mv_mpr2 has the same address as =
__machvec_start

[    0.000000] Linux version 2.6.23-rc2 (markus@linux-markus) (gcc =
version 3.4.6) #38 Tue Aug 7 09:25:06 CEST 2007
[    0.000000] machvec start: 0x8c2c1a70 machvec end 0x8c2c1b20  size: =
0x2c
[    0.000000] Booting machvec: mpr2
[    0.000000] Magic Panel Release 2 A.2

markus@linux-markus:~/kernels$ sh3-unknown-linux-gnu-nm vmlinux | grep =
mpr2
8c2c18c0 t __initcall_mpr2_devices_setup6
8c2c1b00 T init_mpr2_IRQ
8c2c03d4 t mpr2_devices
8c2b1870 t mpr2_devices_setup
8c2b16d0 t mpr2_setup
8c2c1a70 t mv_mpr2

This is with earlyprintk enabled. Now size and addresses have changed =
and mv_mpr2 isn't at the beginning of the machvec anymore.

[    0.000000] Linux version 2.6.23-rc2 (markus@linux-markus) (gcc =
version 3.4.6) #36 Tue Aug 7 09:19:31 CEST 2007
[    0.000000] console [sercon0] enabled
[    0.000000] machvec start: 0x8c2c1bfc machvec end 0x8c2c1cb0  size: =
0x2d
[    0.000000] Booting machvec: ./
[    0.000000] Node 0: start_pfn =3D 0xc000, low =3D 0xff00

markus@linux-markus:~/kernels$ sh3-unknown-linux-gnu-nm vmlinux | grep =
mpr2
8c2c1a4c t __initcall_mpr2_devices_setup6
8c2c1c90 T init_mpr2_IRQ
8c2c0554 t mpr2_devices
8c2b19e0 t mpr2_devices_setup
8c2b1840 t mpr2_setup
8c2c1c00 t mv_mpr2


Regards

Markus

Re: strange early_printk - board_setup problem

[Paul M. <le...@li...>]

On Tue, Aug 07, 2007 at 10:37:10AM +0200, EXTERNAL Brunner Markus (Praktikant; ST-FIR/Eng) wrote:
> markus@linux-markus:~/kernels$ sh3-unknown-linux-gnu-nm vmlinux | grep mpr2
> 8c2c18c0 t __initcall_mpr2_devices_setup6
> 8c2c1b00 T init_mpr2_IRQ
> 8c2c03d4 t mpr2_devices
> 8c2b1870 t mpr2_devices_setup
> 8c2b16d0 t mpr2_setup
> 8c2c1a70 t mv_mpr2
> 
> This is with earlyprintk enabled. Now size and addresses have changed and mv_mpr2 isn't at the beginning of the machvec anymore.
> 
This is rather interesting. Looking at your board patch again, it seems
like this might be caused by the __initmv sprinkling you have on other
functions. __initmv is intended _only_ for the machvec description
itself, init functions referenced from within it should be flagged as
__init. We only ever expect to find full machvec structs within the
machvec section, so this is likely what is screwing it up. Please try
again with __initmv removed from everything but the machvec definition
itself :-)

Re: strange early_printk - board_setup problem

[EXTERNAL B. M. (P. ST-FIR/Eng) <ext...@de...>]

Paul Mundt wrote:
> On Tue, Aug 07, 2007 at 10:37:10AM +0200, EXTERNAL Brunner Markus
> (Praktikant; ST-FIR/Eng) wrote:=20
>> markus@linux-markus:~/kernels$ sh3-unknown-linux-gnu-nm vmlinux |
>> grep mpr2 8c2c18c0 t __initcall_mpr2_devices_setup6
>> 8c2c1b00 T init_mpr2_IRQ
>> 8c2c03d4 t mpr2_devices
>> 8c2b1870 t mpr2_devices_setup
>> 8c2b16d0 t mpr2_setup
>> 8c2c1a70 t mv_mpr2
>>=20
>> This is with earlyprintk enabled. Now size and addresses have
>> changed and mv_mpr2 isn't at the beginning of the machvec anymore.=20
>>=20
> This is rather interesting. Looking at your board patch again, it
> seems=20
> like this might be caused by the __initmv sprinkling you have on other
> functions. __initmv is intended _only_ for the machvec description
> itself, init functions referenced from within it should be flagged as
> __init. We only ever expect to find full machvec structs within the
> machvec section, so this is likely what is screwing it up. Please try
> again with __initmv removed from everything but the machvec definition
> itself :-)
>=20

Thanks a lot! This fixed it.=20
I thought I already removed all of them, but one was still left.

Regards

Markus

Re: strange early_printk - board_setup problem

[Paul M. <le...@li...>]

On Tue, Aug 07, 2007 at 11:09:47AM +0200, EXTERNAL Brunner Markus (Praktikant; ST-FIR/Eng) wrote:
> Paul Mundt wrote:
> > On Tue, Aug 07, 2007 at 10:37:10AM +0200, EXTERNAL Brunner Markus
> > (Praktikant; ST-FIR/Eng) wrote: 
> >> markus@linux-markus:~/kernels$ sh3-unknown-linux-gnu-nm vmlinux |
> >> grep mpr2 8c2c18c0 t __initcall_mpr2_devices_setup6
> >> 8c2c1b00 T init_mpr2_IRQ
> >> 8c2c03d4 t mpr2_devices
> >> 8c2b1870 t mpr2_devices_setup
> >> 8c2b16d0 t mpr2_setup
> >> 8c2c1a70 t mv_mpr2
> >> 
> >> This is with earlyprintk enabled. Now size and addresses have
> >> changed and mv_mpr2 isn't at the beginning of the machvec anymore. 
> >> 
> > This is rather interesting. Looking at your board patch again, it
> > seems 
> > like this might be caused by the __initmv sprinkling you have on other
> > functions. __initmv is intended _only_ for the machvec description
> > itself, init functions referenced from within it should be flagged as
> > __init. We only ever expect to find full machvec structs within the
> > machvec section, so this is likely what is screwing it up. Please try
> > again with __initmv removed from everything but the machvec definition
> > itself :-)
> > 
> 
> Thanks a lot! This fixed it. 
> I thought I already removed all of them, but one was still left.
> 
Does something like this help?

diff --git a/arch/sh/kernel/machvec.c b/arch/sh/kernel/machvec.c
index 23c5948..129b2cf 100644
--- a/arch/sh/kernel/machvec.c
+++ b/arch/sh/kernel/machvec.c
@@ -91,6 +91,13 @@ void __init sh_mv_setup(void)
 				(unsigned long)&__machvec_start);
 
 		/*
+		 * Sanity check for machvec section alignment. Ensure
+		 * __initmv hasn't been misused.
+		 */
+		if (machvec_size % sizeof(struct sh_machine_vector))
+			panic("machvec misaligned, invalid __initmv use?");
+
+		/*
 		 * If the machvec hasn't been preselected, use the first
 		 * vector (usually the only one) from .machvec.init.
 		 */

Re: strange early_printk - board_setup problem

[EXTERNAL B. M. (P. ST-FIR/Eng) <ext...@de...>]

Paul Mundt wrote:
> Does something like this help?
>=20
> diff --git a/arch/sh/kernel/machvec.c b/arch/sh/kernel/machvec.c
> index 23c5948..129b2cf 100644
> --- a/arch/sh/kernel/machvec.c
> +++ b/arch/sh/kernel/machvec.c
> @@ -91,6 +91,13 @@ void __init sh_mv_setup(void)
>  				(unsigned long)&__machvec_start);
>=20
>  		/*
> +		 * Sanity check for machvec section alignment. Ensure
> +		 * __initmv hasn't been misused.
> +		 */
> +		if (machvec_size % sizeof(struct sh_machine_vector))
> +			panic("machvec misaligned, invalid __initmv
use?");
> +
> +		/*
>  		 * If the machvec hasn't been preselected, use the first
>  		 * vector (usually the only one) from .machvec.init.
>  		 */

It would have found my fault and it will probably find the faults of
others.=20
I don't think this error is very extraordinary, because __initmv abuse
was once a goodness. ;-)
http://lkml.org/lkml/diff/2007/6/23/116/1

Re: strange early_printk - board_setup problem

[Paul M. <le...@li...>]

On Tue, Aug 07, 2007 at 11:58:45AM +0200, EXTERNAL Brunner Markus (Praktikant;  ST-FIR/Eng) wrote:
> Paul Mundt wrote:
> > Does something like this help?
> > 
> > diff --git a/arch/sh/kernel/machvec.c b/arch/sh/kernel/machvec.c
> > index 23c5948..129b2cf 100644
> > --- a/arch/sh/kernel/machvec.c
> > +++ b/arch/sh/kernel/machvec.c
> > @@ -91,6 +91,13 @@ void __init sh_mv_setup(void)
> >  				(unsigned long)&__machvec_start);
> > 
> >  		/*
> > +		 * Sanity check for machvec section alignment. Ensure
> > +		 * __initmv hasn't been misused.
> > +		 */
> > +		if (machvec_size % sizeof(struct sh_machine_vector))
> > +			panic("machvec misaligned, invalid __initmv
> use?");
> > +
> > +		/*
> >  		 * If the machvec hasn't been preselected, use the first
> >  		 * vector (usually the only one) from .machvec.init.
> >  		 */
> 
> It would have found my fault and it will probably find the faults of
> others. 
> I don't think this error is very extraordinary, because __initmv abuse
> was once a goodness. ;-)
> http://lkml.org/lkml/diff/2007/6/23/116/1
> 
Yes, I didn't ack the patch primarily because it completely missed the
point. The __initmv cleanup largely happened here:

	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=82f81f4784479df17a80caff4a7156da0a2f7dea

I'll apply the sanity check so people aren't more inclined to step on
this. This is actually the first time this issue has come up, but it's
also the first time we've enabled use of the machvec section
unconditionally. Hopefully the sanity check will curb some confusion.