Fix for mcf5329evb - spurious interrupts on Linux/uClinux kernel boot

Hi TsiChung,

I followed your most recent advice, and it works like a charm! Thanks!

Attached is a very small patch that implements the fix - when CONFIG_MCF532x is defined, the timer interrupt is masked immediately prior to calling into the kernel. I don't know if this is the right way to submit patches, but I assume you'll take over from here, right?

Aaron, Wolfgang: I sent this directly to you in case you're interested...

Take care,
-Bob

P.S. Aaron, you were right - I do indeed have 32 MBytes... Thanks! -------------- next part --------------
A non-text attachment was scrubbed...
Name: mcf5329.spuriousints-fix.patch
Type: text/x-patch
Size: 912 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070807/5f39a670/attachment.bin

Could one of you guys please send this fix as a real patch with a proper signed-off line please. I'll commit the outstanding patches from TsiChung later and will ask Wolfgang to pull from the ColdFire custodian repository.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

Stefan, sorry, in hindsight it is clear that I could have handled this better...

TsiChung, I really meant to send this to you for you to submit, as I didn't want to step on your changes. That, and I don't know the proper patch submittal process.

List, I sent it to the list for others to use.

Some day maybe I'll get the hang of this... :-[
-Bob

Dear Robert,

I am interested, but can you please send a proper patch, with
Signed-off-by: line and all that? Please see
http://www.denx.de/wiki/UBoot/Patches

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is better to marry than to burn.
                                - Bible ``I Corinthians'' ch. 7, v. 9

Dear Wolfgang, Robert, and Tsi-Chung,

[...]

is this considered the correct solution?

Please correct me if I am wrong, but should not interrupts be completely disabled before transferring control to the linux kernel to avoid any confusion?

In this case, I think we should have a function doing this in cpu/.../interrupts.c and call this function always befor control is transferred to the linux kernel. Additionally, this would prevent additional #ifdefs, because the cpu specific things would be handled in cpu/.../interrupts.c.

For mcf532x, interrupt_init() does exactly this, but this seems not the case for mcf52x2, at least not for all the processors handled there.

Any comments of those having an overview over m68k?

Best regards,
Wolfgang

Here I was, preparing the patch in the correct format, and I decided to check my inbox!

I don't know! :)

I don't know the "big picture" for Das U-Boot, so I can't really comment here, other than to note that fewer #ifdefs is good!

I can send my patch in at any time, but until this is resolved, I'm going to hold off...

-Bob

It probably fixes an immediate problem...

Right.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A stone was placed at a ford in a river with the inscription:
"When this stone is covered it is dangerous to ford here."

IIRC the function we are talking about here is do_bootm_linux() (m68k version) which is called from the common do_bootm() function. Before calling do_bootm_linux() the follwing code is called:

/*
* We have reached the point of no return: we are going to
* overwrite all exception vector code, so we cannot easily
* recover from any failures any more...
*/

iflag = disable_interrupts();

So at the point of do_bootm_linux() the interrupts should already be off. If this is not the case, then it is probably better to fix this in the disable_interrupts implementation of the ColdFire.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

Hi Stefan,

you are completely right. Sorry for not recognizing the previous call to disable_interrupts in the first place, I only had looked at do_bootm_linux() itself.

In fact, I also tried calling disable_interrupts() before the call to the kernel, and it did not work.

The problem is that disable_interrupts is in lib_m68k/interrupts.c and simply sets the interrupt level mask to 0x7:

        sr = get_sr ();
        set_sr (sr | 0x0700);

interrupt_init() almost does the opposite:
int interrupt_init(void)

{
        volatile int0_t *intp = (int0_t *) (CFG_INTR_BASE);

        /* Make sure all interrupts are disabled */
        intp->imrh0 |= 0xFFFFFFFF;
        intp->imrl0 |= 0xFFFFFFFF;

        enable_interrupts();
        return 0;
}

There must be something in the kernel that relies on the interrupts being masked in the interrupt controller and not only in the processor, but I can not check right now because I do not have the sourcecode of uClinux I use at work here at home right now.

I do not know if we should blame uClinux here for relying on the interrupt controller being in the reset (all sources disabled) state, or if we should try to fix this in u-boot.

Best regards,
Wolfgang

Hi all,

After some investigation on other archs, most of the peripherals
interrupts are disabled in OS (linux, vxWorks, etc) rather than in u-boot. In u-boot, it is just disabled the global interrupt exception vector. In CF, global interrupt exception vector is set_sr (sr | 0x0700).

Well, my suggestion is kept the way it is in u-boot. And,
diasabled all peripherals interrupts before enabled the global interrupt exception vector in linux.

Regards,
TsiChung

-----Original Message-----
From: Wolfgang Wegner [mailto:wolfgang at leila.ping.de]
Sent: Friday, August 10, 2007 3:22 PM
To: Stefan Roese
Cc: u-boot-users at lists.sourceforge.net; Wolfgang Denk; Aaron Sells; Liew Tsi Chung-r5aahp
Subject: Re: [U-Boot-Users] interrupts in general / Fix for mcf5329evb - spurious interrupts on Linux/uClinux kernel boot

Hi Stefan,

you are completely right. Sorry for not recognizing the previous call to disable_interrupts in the first place, I only had looked at do_bootm_linux() itself.

In fact, I also tried calling disable_interrupts() before the call to the kernel, and it did not work.

The problem is that disable_interrupts is in lib_m68k/interrupts.c and simply sets the interrupt level mask to 0x7:

        sr = get_sr ();
        set_sr (sr | 0x0700);

interrupt_init() almost does the opposite:
int interrupt_init(void)

{
        volatile int0_t *intp = (int0_t *) (CFG_INTR_BASE);

        /* Make sure all interrupts are disabled */
        intp->imrh0 |= 0xFFFFFFFF;
        intp->imrl0 |= 0xFFFFFFFF;

        enable_interrupts();
        return 0;
}

There must be something in the kernel that relies on the interrupts being masked in the interrupt controller and not only in the processor, but I can not check right now because I do not have the sourcecode of uClinux I use at work here at home right now.

I do not know if we should blame uClinux here for relying on the interrupt controller being in the reset (all sources disabled) state, or if we should try to fix this in u-boot.

Best regards,
Wolfgang

Hi all,

I am going to replace the old do_bootm_linux() in lib_m68k/m68k_linux.c to pass bd_t struct, initrd and cmdline. So that, the linux kernel able to obtain the information of bd_t struct, initrd and cmdline thru the stack.

The implementation has been successful proven working very well in other platforms using linux 2.6.xx kernel.

Regards,
TsiChung

-----Original Message-----
From: Stefan Roese [mailto:sr at denx.de]
Sent: Friday, August 10, 2007 8:15 AM
To: u-boot-users at lists.sourceforge.net
Cc: Wolfgang Denk; w.wegner at astro-kom.de; Aaron Sells; Liew Tsi Chung-r5aahp
Subject: Re: [U-Boot-Users] interrupts in general / Fix for mcf5329evb - spurious interrupts on Linux/uClinux kernel boot

IIRC the function we are talking about here is do_bootm_linux() (m68k version) which is called from the common do_bootm() function. Before calling do_bootm_linux() the follwing code is called:

/*
* We have reached the point of no return: we are going to
* overwrite all exception vector code, so we cannot easily
* recover from any failures any more...
*/

iflag = disable_interrupts();

So at the point of do_bootm_linux() the interrupts should already be off. If this is not the case, then it is probably better to fix this in the disable_interrupts implementation of the ColdFire.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de =====================================================================