[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Hi there,

as mentioned on list before some boards/cpus might need some extra code to be 
executed _before_ rebooting. E.g. the i.MX hangs when doing "reset" while the 
LCD controller is still enabled.

That is why I'd like to introduce CONFIG_BOARD_PRE_RESET and board_pre_reset().
The attached does exactly that. But only for cpu/arm920t.
If wished I'll add the appropriate code to all cpu/$(CPU)/cpu.c files.

* Patch by Steven Scholz, 25 Oct 2004:
   - Introducing CONFIG_BOARD_PRE_RESET and board_pre_reset() to allow
     code to be executed before actually rebooting

-- 
Steven Scholz

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: board_pre_reset.patch
Url: http://lists.denx.de/pipermail/u-boot/attachments/20041025/a2a33314/attachment.txt 

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[VanBaren, Gerald]

> -----Original Message-----
> From: u-boot-users-admin at lists.sourceforge.net
> [mailto:u-boot-users-admin at lists.sourceforge.net] On Behalf
> Of Steven Scholz
> Sent: Monday, October 25, 2004 10:28 AM
> To: u-boot-users
> Subject: [U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET
>
> Hi there,
>
> as mentioned on list before some boards/cpus might need some
> extra code to be executed _before_ rebooting. E.g. the i.MX
> hangs when doing "reset" while the LCD controller is still enabled.
>
> That is why I'd like to introduce CONFIG_BOARD_PRE_RESET and
> board_pre_reset().
> The attached does exactly that. But only for cpu/arm920t.
> If wished I'll add the appropriate code to all cpu/$(CPU)/cpu.c files.
>
> * Patch by Steven Scholz, 25 Oct 2004:
>    - Introducing CONFIG_BOARD_PRE_RESET and board_pre_reset() to allow
>      code to be executed before actually rebooting
>
> --
> Steven Scholz


Hi Steven,

I like your suggestion a lot, but I would suggest a "#ifdef" around the
common.h kind of like I hand-pasted in below (probably incorrectly, but
you get my meaning).

gvb


Index: include/common.h
===================================================================
RCS file: /cvsroot/u-boot/u-boot/include/common.h,v
retrieving revision 1.46
diff -p -u -r1.46 common.h
--- include/common.h	10 Oct 2004 18:41:14 -0000	1.46
+++ include/common.h	25 Oct 2004 13:32:15 -0000
@@ -318,6 +318,9 @@ int board_late_init (void);
 int board_postclk_init (void); /* after clocks/timebase, before
env/serial */
 int board_early_init_r (void);
 void board_poweroff (void);
+#ifdef CONFIG_BOARD_PRE_RESET
+void board_pre_reset (void);
+#endif

 #if defined(CFG_DRAM_TEST)
 int testdram(void);



******************************************
The following messages are brought to you by the Lawyers' League of
IdioSpeak:

******************************************
The information contained in, or attached to, this e-mail, may contain confidential information and is intended solely for the use of the individual or entity to whom they are addressed and may be subject to legal privilege.  If you have received this e-mail in error you should notify the sender immediately by reply e-mail, delete the message from your system and notify your system manager.  Please do not copy it for any purpose, or disclose its contents to any other person.  The views or opinions presented in this e-mail are solely those of the author and do not necessarily represent those of the company.  The recipient should check this e-mail and any attachments for the presence of viruses.  The company accepts no liability for any damage caused, directly or indirectly, by any virus transmitted in this email.
******************************************

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Hi Gerald,

> I like your suggestion a lot, but I would suggest a "#ifdef" around the
> common.h kind of like I hand-pasted in below (probably incorrectly, but
> you get my meaning).

>  int board_postclk_init (void);
>  int board_early_init_r (void);
>  void board_poweroff (void);
> +#ifdef CONFIG_BOARD_PRE_RESET
> +void board_pre_reset (void);
> +#endif

Since functions like board_postclk_init and board_early_init_r etc. don't have 
an #ifdef I decided to go the same way.

Do you see any problems without #ifdef ?

-- 
Steven Scholz

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[VanBaren, Gerald]

> -----Original Message-----
> From: Steven Scholz [mailto:steven.scholz at imc-berlin.de]
> Sent: Monday, October 25, 2004 10:40 AM
> To: VanBaren, Gerald (AGRE)
> Cc: u-boot-users
> Subject: Re: [U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET
>
> Hi Gerald,
>
> > I like your suggestion a lot, but I would suggest a "#ifdef" around
> > the common.h kind of like I hand-pasted in below (probably
> > incorrectly, but you get my meaning).
>
> >  int board_postclk_init (void);
> >  int board_early_init_r (void);
> >  void board_poweroff (void);
> > +#ifdef CONFIG_BOARD_PRE_RESET
> > +void board_pre_reset (void);
> > +#endif
>
> Since functions like board_postclk_init and
> board_early_init_r etc. don't have an #ifdef I decided to go
> the same way.
>
> Do you see any problems without #ifdef ?
>
> --
> Steven Scholz

The reason I suggested the #ifdef is because the actual code
implementation is surrounded by the #ifdef so, if you don't have
CONFIG_BOARD_PRE_RESET defined, you will have a declaration of
board_pre_reset() but no code implementing it.  I forgot off-hand if
this is a problem, but I think it is (too lazy to be a language lawyer
at the moment).

gvb

******************************************
The following messages are brought to you by the Lawyers' League of
IdioSpeak:

******************************************
The information contained in, or attached to, this e-mail, may contain confidential information and is intended solely for the use of the individual or entity to whom they are addressed and may be subject to legal privilege.  If you have received this e-mail in error you should notify the sender immediately by reply e-mail, delete the message from your system and notify your system manager.  Please do not copy it for any purpose, or disclose its contents to any other person.  The views or opinions presented in this e-mail are solely those of the author and do not necessarily represent those of the company.  The recipient should check this e-mail and any attachments for the presence of viruses.  The company accepts no liability for any damage caused, directly or indirectly, by any virus transmitted in this email.
******************************************

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Hi Gerald,

>>Do you see any problems without #ifdef ?
> 
> The reason I suggested the #ifdef is because the actual code
> implementation is surrounded by the #ifdef so, if you don't have
> CONFIG_BOARD_PRE_RESET defined, you will have a declaration of
> board_pre_reset() but no code implementing it.

This is how it's done all over the U-Boot source code.
So I take it for some kind of "coding style" ... ;-)

Steven

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Wolfgang Denk]

In message <065ACD8E84315E4394C835E398C8D5EB90F44D@COSSMGMBX02.email.corp.tld> you wrote:
> 
> The reason I suggested the #ifdef is because the actual code
> implementation is surrounded by the #ifdef so, if you don't have
> CONFIG_BOARD_PRE_RESET defined, you will have a declaration of
> board_pre_reset() but no code implementing it.  I forgot off-hand if
> this is a problem, but I think it is (too lazy to be a language lawyer
> at the moment).

It is not a problem.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Where a calculator on the ENIAC is equipped with 18,000 vacuum  tubes
and  weighs  30  tons,  computers  in  the future may have only 1,000
vacuum tubes and weigh only 1/2 tons. - Popular Mechanics, March 1949

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Wolfgang Denk]

In message <417D0D67.2000000@imc-berlin.de> you wrote:
>
> as mentioned on list before some boards/cpus might need some extra code to be 
> executed _before_ rebooting. E.g. the i.MX hangs when doing "reset" while the 
> LCD controller is still enabled.

I remember that we discussed this before, but now that I think  about
this  again  I changed my mind: I don't think there is a valid reason
to add this code.

> That is why I'd like to introduce CONFIG_BOARD_PRE_RESET and board_pre_reset().

No. You write: i.MX hangs when doing "reset". I guess the board  does
not  hand  if  you  power cycle it, or if you press the reset button,
right? So the conclusion is that the reset code does not do what  you
think it does - i. e. it does not cause a reset of the processor.

This is a problem that should be fixed. Once fixed, this patch is not
needed any more.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You don't have to worry about me. I might have been born yesterday...
but I stayed up all night.

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Wolfgang Denk wrote:
> In message <417D0D67.2000000@imc-berlin.de> you wrote:
> 
>>as mentioned on list before some boards/cpus might need some extra code to be 
>>executed _before_ rebooting. E.g. the i.MX hangs when doing "reset" while the 
>>LCD controller is still enabled.
> 
> 
> I remember that we discussed this before, but now that I think  about
> this  again  I changed my mind: I don't think there is a valid reason
> to add this code.
> 
> 
>>That is why I'd like to introduce CONFIG_BOARD_PRE_RESET and board_pre_reset().
> 
> 
> No. You write: i.MX hangs when doing "reset". I guess the board  does
> not  hand  if  you  power cycle it, or if you press the reset button,
> right? So the conclusion is that the reset code does not do what  you
> think it does - i. e. it does not cause a reset of the processor.
> 
> This is a problem that should be fixed. Once fixed, this patch is not
> needed any more.

Well, it's a bug in the CPU. If the LCDC DMA is running, the cpu won't reboot 
with a simple "mov pc, r0".
But then switching off the i.MX LCD controller has to be done in 
cpu/arm920t/start.S where reset_cpu() is. And you don't wanna do this. There 
already are enough #ifdefs in cpu/arm920t/start.S.

And how about the AT91RM9200(DK) where the author wants to "shutdown the console 
to avoid strange chars during reset"???

-- 
Steven Scholz

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Wolfgang Denk]

In message <417DF771.5010904@imc-berlin.de> you wrote:
>
> Well, it's a bug in the CPU. If the LCDC DMA is running, the cpu won't reboot 
> with a simple "mov pc, r0".

Ummm... you don't call this a "reset", do you?

> And how about the AT91RM9200(DK) where the author wants to "shutdown the console 
> to avoid strange chars during reset"???

Well, that's IMHO polishing, and forgiveable ;-)

In your case, I think you really want to  implement  a  _real_  reset
function  that  takes the CPU back into it's virgin state. Is there a
watchdog on this hardware?

Best regards,

Wolfgang Denk

-- 
See us @ Embedded/Electronica Munich, Nov 09 - 12, Hall A.6 Booth 513
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."

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Wolfgang Denk wrote:

> In message <417DF771.5010904@imc-berlin.de> you wrote:
> 
>>Well, it's a bug in the CPU. If the LCDC DMA is running, the cpu won't reboot 
>>with a simple "mov pc, r0".
> 
> 
> Ummm... you don't call this a "reset", do you?

Well that's what is done (work) for most ARM920T... Execeptions are

#ifdef CONFIG_S3C2400
         bl      disable_interrupts
# ifdef CONFIG_TRAB
         bl      disable_vfd
# endif
...

BTW cominh to think about it: Does "bl disable_vfd" by any chance disable video 
controller for CONFIG_TRAB !?!? ;-)

> In your case, I think you really want to  implement  a  _real_  reset
> function  that  takes the CPU back into it's virgin state. Is there a
> watchdog on this hardware?

Sure. That means you have board _and_ cpu specific reset code!
Some want to toggle a GPIO pin. Some want to use a cpu watchdog...
So should we move the reset code out of cpu/arm920t/start.S into the brand new 
cpu/arm920t/$(SOC)/reset.{S.c} !?

-- 
Steven Scholz

R: [U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Paolo Broggini]

> -----Messaggio originale-----
> Da: u-boot-users-admin at lists.sourceforge.net
> [mailto:u-boot-users-admin at lists.sourceforge.net]Per conto di Steven
> Scholz
> Inviato: martedi, 26. ottobre 2004 10:32
> Cc: u-boot-users
> Oggetto: Re: [U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET
> 
> 
> Wolfgang Denk wrote:
> 
> > In message <417DF771.5010904@imc-berlin.de> you wrote:
> > 
> >>Well, it's a bug in the CPU. If the LCDC DMA is running, the 
> cpu won't reboot 
> >>with a simple "mov pc, r0".
> > 
> > 
> > Ummm... you don't call this a "reset", do you?
> 
> Well that's what is done (work) for most ARM920T... Execeptions are
> 
> #ifdef CONFIG_S3C2400
>          bl      disable_interrupts
> # ifdef CONFIG_TRAB
>          bl      disable_vfd
> # endif
> ...
> 
> BTW cominh to think about it: Does "bl disable_vfd" by any chance 
> disable video 
> controller for CONFIG_TRAB !?!? ;-)
> 
> > In your case, I think you really want to  implement  a  _real_  reset
> > function  that  takes the CPU back into it's virgin state. Is there a
> > watchdog on this hardware?
> 
> Sure. That means you have board _and_ cpu specific reset code!
> Some want to toggle a GPIO pin. Some want to use a cpu watchdog...
> So should we move the reset code out of cpu/arm920t/start.S into 
> the brand new 
> cpu/arm920t/$(SOC)/reset.{S.c} !?

This would be a good step towards clean up the clobbered start.s !!!
-P.Broggini

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Dear Wolfgang,

>>> Well, it's a bug in the CPU. If the LCDC DMA is running, the cpu 
>>> won't reboot with a simple "mov pc, r0".
>> Ummm... you don't call this a "reset", do you?
> Well that's what is done (work) for most ARM920T... Execeptions are
> 
> #ifdef CONFIG_S3C2400
>         bl      disable_interrupts
> # ifdef CONFIG_TRAB
>         bl      disable_vfd
> # endif
> ...
> 
> BTW cominh to think about it: Does "bl disable_vfd" by any chance 
> disable video controller for CONFIG_TRAB !?!? ;-)
> 
>> In your case, I think you really want to  implement  a  _real_  reset
>> function  that  takes the CPU back into it's virgin state. Is there a
>> watchdog on this hardware?
> 
> Sure. That means you have board _and_ cpu specific reset code!
> Some want to toggle a GPIO pin. Some want to use a cpu watchdog...
> So should we move the reset code out of cpu/arm920t/start.S into the 
> brand new cpu/arm920t/$(SOC)/reset.{S.c} !?

Any new opinions on this topic? Or is it closed for you and my suggestion rejected?

-- 
Steven Scholz

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Wolfgang,

I wrote:

> as mentioned on list before some boards/cpus might need some extra code 
> to be executed _before_ rebooting. E.g. the i.MX hangs when doing 
> "reset" while the LCD controller is still enabled.
> 
> That is why I'd like to introduce CONFIG_BOARD_PRE_RESET and 
> board_pre_reset().
> The attached does exactly that. But only for cpu/arm920t.
> If wished I'll add the appropriate code to all cpu/$(CPU)/cpu.c files.
> 
> * Patch by Steven Scholz, 25 Oct 2004:
>   - Introducing CONFIG_BOARD_PRE_RESET and board_pre_reset() to allow
>     code to be executed before actually rebooting

any new opinions on this topic?

-- 
Steven Scholz

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Wolfgang Denk]

In message <41A346BD.2060207@imc-berlin.de> you wrote:
> 
> > * Patch by Steven Scholz, 25 Oct 2004:
> >   - Introducing CONFIG_BOARD_PRE_RESET and board_pre_reset() to allow
> >     code to be executed before actually rebooting
> 
> any new opinions on this topic?

No.

[Actually I think that it might make sense to perform certain cleanup
operations in case of an orderly reboot; on the other hand the reboot
code _must_not_ depend on such cleanup. In your case there  seems  to
be  such  a  dependency.  I'd  rather see that you fix the reset code
first, and then (if there is still need for it) we  might  allow  for
pre-reset cleanup functions.]

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Quote from a recent meeting:   "We are going to continue having these
meetings everyday until I find out why no work is getting done."

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Wolfgang Denk wrote:

> In message <41A346BD.2060207@imc-berlin.de> you wrote:
> 
>>>* Patch by Steven Scholz, 25 Oct 2004:
>>>  - Introducing CONFIG_BOARD_PRE_RESET and board_pre_reset() to allow
>>>    code to be executed before actually rebooting
>>
>>any new opinions on this topic?
> 
> 
> No.
> 
> [Actually I think that it might make sense to perform certain cleanup
> operations in case of an orderly reboot; on the other hand the reboot
> code _must_not_ depend on such cleanup. In your case there  seems  to
> be  such  a  dependency.  I'd  rather see that you fix the reset code
> first, and then (if there is still need for it) we  might  allow  for
> pre-reset cleanup functions.]

But as I pointed out apparently other boards need something like that too:

.globl reset_cpu
reset_cpu:
...
# ifdef CONFIG_TRAB
	bl	disable_vfd
# endif

So reset is cpu (better SoC) specific and might even be board specific.

Of coures we can clobber cpu/arm920t/start.S with reset code for every cpu, SoC 
and board ... adding loads of nice #ifdef's ;-)

--
Steven

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Wolfgang Denk]

In message <41A35BBE.90501@imc-berlin.de> you wrote:
>
> But as I pointed out apparently other boards need something like that too:

No.

> .globl reset_cpu
> reset_cpu:
> ...
> # ifdef CONFIG_TRAB
> 	bl	disable_vfd
> # endif

The TRAB board will perfectly reset the  board  if  you  remove  this
code.  It  just doesn't look as nice (flicker on the display). That's
what I meant that some boards may _want_ such a function.

> So reset is cpu (better SoC) specific and might even be board specific.

You may want to run board-specific code _before_ reset, but the reset
operation itself should IMHO not depend on board  specific  features.
It's always processor (CPU or SoC or what you may call it) specific.

> Of coures we can clobber cpu/arm920t/start.S with reset code for every cpu, SoC 
> and board ... adding loads of nice #ifdef's ;-)

This is certainly NOT my intention. I think I explained before what I
have in mind.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
God made machine language; all the rest is the work of man.

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Steven Scholz]

Wolfgang Denk wrote:

>>So reset is cpu (better SoC) specific and might even be board specific.
> 
> You may want to run board-specific code _before_ reset, but the reset
> operation itself should IMHO not depend on board  specific  features.
> It's always processor (CPU or SoC or what you may call it) specific.

Sorry. But I have to ask again:

If I don't want to rely on the processor reset (since it might be broken), how 
am I able to use my boards pecific hardware watchdog?

-- 
Steven Scholz

[U-Boot-Users] PATCH: Introducing CONFIG_BOARD_PRE_RESET

[Wolfgang Denk]

In message <41A36B54.3000904@imc-berlin.de> you wrote:
> 
> If I don't want to rely on the processor reset (since it might be broken), how 

If you cannot rely on the basic functions of your processor than  you
are lost. Get new hardware and go back to START. Do not cash $100.

> am I able to use my boards pecific hardware watchdog?

That's a catch question. I do recognize it as such ;-)

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Killing is stupid; useless!
	-- McCoy, "A Private Little War", stardate 4211.8