[RFC 0/5] restart cleanups

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

This series cleans up some of the restart code for ARM.  It doesn't go
as far as Will's patch set (or even anywhere close to it) mainly because
of the poor state of the various implementations in various platforms.

Effectively, the various platforms themselves stand in the way of optimal
cleanup of this code.

We already have a function pointer to allow the restart to be hooked at
runtime - it's called arm_pm_restart().

What a lot of platforms use this for is to intercept the restart call,
do something, and then continue on by calling arm_machine_restart().

One platform (mioa701) in particular thinks its a good idea to call
functions like gpio_free() and free_irq() from here.  This is soo far
from reality its untrue; the restart paths can be called from _any_
context including IRQ context.  This alone effectively stands in the
way of moving the IRQ disables before the call to arm_pm_restart().

Other platforms use arm_pm_restart() to intercept the restart, and
force the mode to 'g', 'h' or 's'.  As the mode already defaults to
'h' I don't understand why people felt forcing it in that case was
necessary.  As for the 's' case, this can be done by merely specifying
".soft_reboot = 1" in the machine record.  We can solve the 'g' case
by changing .soft_reboot to be a char "restart_mode" argument which
sets the default restart mode.

Although I have a patch (not published as part of this series) which
makes that change, I've found it less than useful for getting rid of
some of the intercept crap because of other games platforms play in
there (like writing magic values to registers.)

Essentially, we want to move to a state where arm_pm_restart() is _the_
hook for platforms to insert their restart code into, with the default
function being a no-op.  All platforms which want to have working
restart support then have to insert their restart code into this hook.

So, this patch set takes us a little way there by cleaning up some of
the restart code:

1. remove the useless argument to setup_mm_for_reboot()
2. remove the silly indirection in AT91 code and do this like other
   platforms do.
3. remove any local_irq_disable() calls inside arch_reset() because
   this is already the case.
4. factor out the code which fiddles with the MMU tables.
5. provide (and use) a function for soft rebooting which includes the
   setup of the MMU tables and call cpu_reset() - and remove the MMU
   setup from the code paths before arch_reset() is called.

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

Can someone please check whether this code:

/* Hook for arm_pm_restart to ensure we execute the reset code
 * with the caches enabled. It seems at least the S3C2440 has a problem
 * resetting if there is bus activity interrupted by the reset.
 */
static void s3c24xx_pm_restart(char mode, const char *cmd)
{
        if (mode != 's') {
                unsigned long flags;

                local_irq_save(flags);
                __cpuc_flush_kern_all();
                __cpuc_flush_user_all();

                arch_reset(mode, cmd);
                local_irq_restore(flags);
        }

        /* fallback, or unhandled */
        arm_machine_restart(mode, cmd);
}

will be broken by any of these patches - and more importantly whether
this code can simply be deleted with these five patches applied.

Thanks.

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

(Poodle doesn't have any MAINTAINERS entry, I think Richard Purdie last
touched Poodle in a 'maintainery' way back in 2006.)

poodle does this:

static void poodle_restart(char mode, const char *cmd)
{
        arm_machine_restart('h', cmd);
}

Given that the default value of 'mode' (in the absense of any reboot=
command line argument to the kernel) is 'h', this seems to be a no-op.
Below is the history; I suspect that the reason this first appeared was
because of the need to fiddle with RCSR - that's now gone so I suspect
the entire restart handler can be deleted.

What I don't understand in that case is why 'mode' wasn't simply passed
through in the first patch.

Please confirm.

commit 74617fb6b825ea370ae72565f7543306bc08ef6e
Author: Richard Purdie <rpurdie@rpsys.net>
Date:   Mon Jun 19 19:57:12 2006 +0100

    [ARM] 3593/1: Add reboot and shutdown handlers for Zaurus handhelds

    Patch from Richard Purdie

    Add functionality to allow machine specific reboot handlers on ARM.
    Add machine specific reboot and poweroff handlers for all PXA Zaurus
    models.

diff --git a/arch/arm/mach-pxa/poodle.c b/arch/arm/mach-pxa/poodle.c

@@ -247,10 +249,25 @@ static struct platform_device *devices[] __initdata = {
        &poodle_scoop_device,
 };

+static void poodle_poweroff(void)
+{
+       RCSR = RCSR_HWR | RCSR_WDR | RCSR_SMR | RCSR_GPR;
+       arm_machine_restart('h');
+}
+
+static void poodle_restart(char mode)
+{
+       RCSR = RCSR_HWR | RCSR_WDR | RCSR_SMR | RCSR_GPR;
+       arm_machine_restart('h');
+}
+
 static void __init poodle_init(void)
 {
        int ret = 0;

+       pm_power_off = poodle_poweroff;
+       arm_pm_restart = poodle_restart;

...
commit dc38e2ad53ca27968919dea6d7fa60575782d5a6
Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
Date:   Thu May 8 16:50:39 2008 +0100

    [ARM] pxa: Fix RCSR handling

    Related to d3930614e68bdf83a120d904c039a64e9f75dba1.

    RCSR is only present on PXA2xx CPUs, not on PXA3xx CPUs.  Therefore,
    we should not be unconditionally writing to RCSR from generic code.

    Since we now clear the RCSR status from the SoC specific PXA PM code
    and before reset in the arch_reset() function, the duplication in
    the corgi, poodle, spitz and tosa code can be removed.

diff --git a/arch/arm/mach-pxa/poodle.c b/arch/arm/mach-pxa/poodle.c
index ca5ac19..0b30f25 100644
--- a/arch/arm/mach-pxa/poodle.c
+++ b/arch/arm/mach-pxa/poodle.c
@@ -326,13 +326,11 @@ static struct platform_device *devices[] __initdata = {

 static void poodle_poweroff(void)
 {
-       RCSR = RCSR_HWR | RCSR_WDR | RCSR_SMR | RCSR_GPR;
        arm_machine_restart('h');
 }

 static void poodle_restart(char mode)
 {
-       RCSR = RCSR_HWR | RCSR_WDR | RCSR_SMR | RCSR_GPR;
        arm_machine_restart('h');
 }

[RFC 0/5] restart cleanups

[Will Deacon]

Hi Russell,

On Tue, Nov 01, 2011 at 04:07:31PM +0000, Russell King - ARM Linux wrote:
> This series cleans up some of the restart code for ARM.  It doesn't go
> as far as Will's patch set (or even anywhere close to it) mainly because
> of the poor state of the various implementations in various platforms.

Thanks for helping out with this!

> Effectively, the various platforms themselves stand in the way of optimal
> cleanup of this code.

Agreed. I'm hoping that people will complain if we break them by accident.

> Other platforms use arm_pm_restart() to intercept the restart, and
> force the mode to 'g', 'h' or 's'.  As the mode already defaults to
> 'h' I don't understand why people felt forcing it in that case was
> necessary.  As for the 's' case, this can be done by merely specifying
> ".soft_reboot = 1" in the machine record.  We can solve the 'g' case
> by changing .soft_reboot to be a char "restart_mode" argument which
> sets the default restart mode.

Actually, I think we can go as far as deleteing the 'g' mode and using the
GPIO reset as a fallback if we fail in the usual 'h' case. This mode is only
used by mach-pxa, so it would be good to get rid of it.

We can always do this in a later patch series if it gets in the way (I have
a series to clean up the reboot modes already).

> So, this patch set takes us a little way there by cleaning up some of
> the restart code:
> 
> 1. remove the useless argument to setup_mm_for_reboot()

I'm definitely in favour of this change, but be aware that I plan to modify
setup_mm_for_reboot in the future to:

  (a) Use a static identity mapping so we don't have to pgd_alloc explicitly
      [I can't see this affecting anything]

  (b) Extend the identity mapping into kernel space, leaving some room for
      the kernel image. If this is all called from soft_reboot, which just
      does the cpu_reset(addr) afterwards, then we're fine. I have code to
      take care of changing stack and switching to the new mapping.

> 5. provide (and use) a function for soft rebooting which includes the
>    setup of the MMU tables and call cpu_reset() - and remove the MMU
>    setup from the code paths before arch_reset() is called.

This function will probably grow to contain the arm_machine_reset function
body that I have for kexec.

If you're happy with the direction I'd like to take this in, then you can
have my Acked-by tags for patches 1 and 5.

Cheers,

Will

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

On Tue, Nov 01, 2011 at 07:23:43PM +0000, Will Deacon wrote:
> Hi Russell,
> 
> On Tue, Nov 01, 2011 at 04:07:31PM +0000, Russell King - ARM Linux wrote:
> > Other platforms use arm_pm_restart() to intercept the restart, and
> > force the mode to 'g', 'h' or 's'.  As the mode already defaults to
> > 'h' I don't understand why people felt forcing it in that case was
> > necessary.  As for the 's' case, this can be done by merely specifying
> > ".soft_reboot = 1" in the machine record.  We can solve the 'g' case
> > by changing .soft_reboot to be a char "restart_mode" argument which
> > sets the default restart mode.
> 
> Actually, I think we can go as far as deleteing the 'g' mode and using the
> GPIO reset as a fallback if we fail in the usual 'h' case. This mode is only
> used by mach-pxa, so it would be good to get rid of it.
> 
> We can always do this in a later patch series if it gets in the way (I have
> a series to clean up the reboot modes already).

I think it's useful to distinguish between them especially as there's
platforms in PXA land doing weirdo things with this.  Some platforms
use the GPIO reset thing as a way to shut the system down as well as
reboot it, so it's not purely just about reboot (which is in itself
annoying).

The other thing is that the generic code shouldn't care what value 'mode'
actually is - we just pass it through to the platform handler (via
arm_pm_restart()) and if the platform handler wants to ignore it and
just do the software reboot thing, it uses soft_restart(addr) which
does all the necessary setup for that.

The nice thing about that is we no longer have to worry about calling
arm_pm_restart() with 'h' and a platform doing a soft-reboot instead.

> > So, this patch set takes us a little way there by cleaning up some of
> > the restart code:
> > 
> > 1. remove the useless argument to setup_mm_for_reboot()
> 
> I'm definitely in favour of this change, but be aware that I plan to modify
> setup_mm_for_reboot in the future to:
> 
>   (a) Use a static identity mapping so we don't have to pgd_alloc explicitly
>       [I can't see this affecting anything]
> 
>   (b) Extend the identity mapping into kernel space, leaving some room for
>       the kernel image. If this is all called from soft_reboot, which just
>       does the cpu_reset(addr) afterwards, then we're fine. I have code to
>       take care of changing stack and switching to the new mapping.

Well, bear in mind that (1) + (5) produce something really self-contained.
We no longer fiddle with MMU tables for anything but the 'soft reboot'
path.  That means things like hardware based reset (eg, using watchdogs,
asserting GPIOs, etc) are all dealt with without fiddling with the MMU
and caches.

The side effect of this is that the whole soft-reboot thing is now a
self-contained chunk of code which can deal with setting things up in
order to do what it needs to do without fear of trampling over hardware
mappings.

> > 5. provide (and use) a function for soft rebooting which includes the
> >    setup of the MMU tables and call cpu_reset() - and remove the MMU
> >    setup from the code paths before arch_reset() is called.
> 
> This function will probably grow to contain the arm_machine_reset function
> body that I have for kexec.

arm_machine_restart() has become almost empty - I want to move the
IRQ disables out into machine_restart() and kill off arm_machine_restart()
entirely (at the moment its the 'default' action, which is to call the
old arch_reset() thing.)

Eventually, when we have everyone hooking into arm_pm_restart() instead,
arm_machine_restart() will be unused and meaningless to keep around.

[RFC 0/5] restart cleanups

[Will Deacon]

On Tue, Nov 01, 2011 at 07:41:54PM +0000, Russell King - ARM Linux wrote:
> The other thing is that the generic code shouldn't care what value 'mode'
> actually is - we just pass it through to the platform handler (via
> arm_pm_restart()) and if the platform handler wants to ignore it and
> just do the software reboot thing, it uses soft_restart(addr) which
> does all the necessary setup for that.

Ah yes, you're right. I think it might be worth defining 'h' as REBOOT_MODE_HARD
and 's' as REBOOT_MODE_SOFT, but there's no reason to disallow other
combinations giving that the core code no longer has to do anything with
these.

> The nice thing about that is we no longer have to worry about calling
> arm_pm_restart() with 'h' and a platform doing a soft-reboot instead.

Yes, that's truly horrific. It's a miracle soft reboot worked at all from v6
onwards.

> > I'm definitely in favour of this change, but be aware that I plan to modify
> > setup_mm_for_reboot in the future to:
> > 
> >   (a) Use a static identity mapping so we don't have to pgd_alloc explicitly
> >       [I can't see this affecting anything]
> > 
> >   (b) Extend the identity mapping into kernel space, leaving some room for
> >       the kernel image. If this is all called from soft_reboot, which just
> >       does the cpu_reset(addr) afterwards, then we're fine. I have code to
> >       take care of changing stack and switching to the new mapping.
> 
> Well, bear in mind that (1) + (5) produce something really self-contained.
> We no longer fiddle with MMU tables for anything but the 'soft reboot'
> path.  That means things like hardware based reset (eg, using watchdogs,
> asserting GPIOs, etc) are all dealt with without fiddling with the MMU
> and caches.

Yup, so we should be able to add the scary MMU and cache stuff into
soft_reboot. We can even call this function from the kexec code.

> arm_machine_restart() has become almost empty - I want to move the
> IRQ disables out into machine_restart() and kill off arm_machine_restart()
> entirely (at the moment its the 'default' action, which is to call the
> old arch_reset() thing.)
> 
> Eventually, when we have everyone hooking into arm_pm_restart() instead,
> arm_machine_restart() will be unused and meaningless to keep around.

That's where our friend Coccinelle comes in at the end of my patch series.

Will

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

On Tue, Nov 01, 2011 at 09:19:33PM +0000, Will Deacon wrote:
> Ah yes, you're right. I think it might be worth defining 'h' as
> REBOOT_MODE_HARD and 's' as REBOOT_MODE_SOFT, but there's no reason to
> disallow other combinations giving that the core code no longer has to
> do anything with these.

I don't see any need to define these to anything, especially as we just
take the first character of the reboot= argument.  If we did change it,
it would imply that we'd need to change how we parsed that argument as
well.

> > The nice thing about that is we no longer have to worry about calling
> > arm_pm_restart() with 'h' and a platform doing a soft-reboot instead.
> 
> Yes, that's truly horrific. It's a miracle soft reboot worked at all from v6
> onwards.

The short answer is it doesn't - and it's something that with this new
structure we can fix - we're no longer concerned by whatever the platform
code is doing between the soft restart setup code and actually executing
the soft restart itself.

Thankfully, at the point where we have v6 support, most platforms have
advanced to the point where there's (mostly) always a hardware method
to do a restart, which is much more preferable than trying to do a
soft-reboot (it ensures that all peripheral devices are placed into a
known state, whereas a soft restart doesn't have that guarantee.)

> > arm_machine_restart() has become almost empty - I want to move the
> > IRQ disables out into machine_restart() and kill off arm_machine_restart()
> > entirely (at the moment its the 'default' action, which is to call the
> > old arch_reset() thing.)
> > 
> > Eventually, when we have everyone hooking into arm_pm_restart() instead,
> > arm_machine_restart() will be unused and meaningless to keep around.
> 
> That's where our friend Coccinelle comes in at the end of my patch series.

We do have this problem - mioa701 - which insists on doing things like
kfree, free_irq, gpio_free in this path, which is a bad idea as it can
be called from many different contexts including IRQ context.  That
will appear to work for '/sbin/reboot' but not for things like softdog
or a sysrq-b.  Arguably, mioa701 is broken in this regard.

However, we don't know whether removing the code will then prevent it
rebooting even for a userspace-invoked reboot.  We need feedback from
the platform maintainer for that (added Philipp and Robert in the
hope one of them can answer this - and maybe send a patch to fix it.)

[RFC 0/5] restart cleanups

[Robert Jarzmik]

----- Mail Original -----
De: "Russell King - ARM Linux" <linux@arm.linux.org.uk>
> We do have this problem - mioa701 - which insists on doing things like
> kfree, free_irq, gpio_free in this path, which is a bad idea as it can
True. You mean gsm_exit() and bootstrap_exit() I think.
These were added to free resources in a shutdown. If freeing is pointless,
in a reboot case, and stale resources don't matter, well, we can kill these
2 functions.

> However, we don't know whether removing the code will then prevent it
> rebooting even for a userspace-invoked reboot.  We need feedback from
> the platform maintainer for that (added Philipp and Robert in the
> hope one of them can answer this - and maybe send a patch to fix it.)
I'll try to test the patch serie this evening after my daywork,
and give feedback.

The main facts of mioa701 board regarding halting/rebooting:
 - watchdog reset kills RTC time
 - there is no GPIO reset (which would preserve time, sic ...)
 - mioa701 is based on PXA272 (arm v5)

Cheers.

--
Robert

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

On Wed, Nov 02, 2011 at 08:46:06AM +0100, Robert Jarzmik wrote:
> ----- Mail Original -----
> De: "Russell King - ARM Linux" <linux@arm.linux.org.uk>
> > We do have this problem - mioa701 - which insists on doing things like
> > kfree, free_irq, gpio_free in this path, which is a bad idea as it can
> True. You mean gsm_exit() and bootstrap_exit() I think.
> These were added to free resources in a shutdown. If freeing is pointless,
> in a reboot case, and stale resources don't matter, well, we can kill these
> 2 functions.

Provided you're talking about software state only, then it is pointless
because the state of the system software will be lost on reboot anyway.

[RFC 0/5] restart cleanups

[Nicolas Pitre]

On Tue, 1 Nov 2011, Russell King - ARM Linux wrote:

> This series cleans up some of the restart code for ARM.  It doesn't go
> as far as Will's patch set (or even anywhere close to it) mainly because
> of the poor state of the various implementations in various platforms.
[...]

For the 5 patches:

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>


Nicolas

[RFC 0/5] restart cleanups

[Richard Purdie]

On Tue, 2011-11-01 at 17:02 +0000, Russell King - ARM Linux wrote:
> (Poodle doesn't have any MAINTAINERS entry, I think Richard Purdie last
> touched Poodle in a 'maintainery' way back in 2006.)
> 
> poodle does this:
> 
> static void poodle_restart(char mode, const char *cmd)
> {
>         arm_machine_restart('h', cmd);
> }
> 
> Given that the default value of 'mode' (in the absense of any reboot=
> command line argument to the kernel) is 'h', this seems to be a no-op.
> Below is the history; I suspect that the reason this first appeared was
> because of the need to fiddle with RCSR - that's now gone so I suspect
> the entire restart handler can be deleted.
> 
> What I don't understand in that case is why 'mode' wasn't simply passed
> through in the first patch.

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

On Wed, Nov 02, 2011 at 01:49:07PM +0000, Richard Purdie wrote:
> >From what I remember that hardware either always reboots or always
> halts. I think the option was therefore left hardcoded to make it clear
> it wasn't expected to work. Later Zaurii models could do either but
> required some manual poking of registers to make it happen iirc.
> 
> Regardless, you can probably clean this up as you suggest now.

Thanks, I've now committed a change which removes the indirection
entirely from poodle.

[RFC 0/5] restart cleanups

[Robert Jarzmik]

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> However, we don't know whether removing the code will then prevent it
> rebooting even for a userspace-invoked reboot.  We need feedback from
> the platform maintainer for that (added Philipp and Robert in the
> hope one of them can answer this - and maybe send a patch to fix it.)

OK, I tested the serie on mioa701 board, and everything works as before. I
tested it with mioa701.c amended so that no more halt/reboot hooks are used
anymore. The resources freeing is gone as well.

I will submit the following patch [1] to get rid of the hooks to Eric, unless
you want it in your tree.

Cheers.

-- 
Robert

[1]

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

On Wed, Nov 02, 2011 at 10:33:00PM +0100, Robert Jarzmik wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> > However, we don't know whether removing the code will then prevent it
> > rebooting even for a userspace-invoked reboot.  We need feedback from
> > the platform maintainer for that (added Philipp and Robert in the
> > hope one of them can answer this - and maybe send a patch to fix it.)
> 
> OK, I tested the serie on mioa701 board, and everything works as before. I
> tested it with mioa701.c amended so that no more halt/reboot hooks are used
> anymore. The resources freeing is gone as well.
> 
> I will submit the following patch [1] to get rid of the hooks to Eric, unless
> you want it in your tree.

I think I should pick it up because I have a patch changing the
.soft_reboot stuff.

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

On Tue, Nov 01, 2011 at 04:38:17PM +0000, Russell King - ARM Linux wrote:
> Can someone please check whether this code:
> 
> /* Hook for arm_pm_restart to ensure we execute the reset code
>  * with the caches enabled. It seems at least the S3C2440 has a problem
>  * resetting if there is bus activity interrupted by the reset.
>  */
> static void s3c24xx_pm_restart(char mode, const char *cmd)
> {
>         if (mode != 's') {
>                 unsigned long flags;
> 
>                 local_irq_save(flags);
>                 __cpuc_flush_kern_all();
>                 __cpuc_flush_user_all();
> 
>                 arch_reset(mode, cmd);
>                 local_irq_restore(flags);
>         }
> 
>         /* fallback, or unhandled */
>         arm_machine_restart(mode, cmd);
> }
> 
> will be broken by any of these patches - and more importantly whether
> this code can simply be deleted with these five patches applied.

Okay, I'm going to delete the above and hope for the best; I'm taking
the lack of reply as meaning that no one in the Samsung community is
that interested in this code.

[RFC 0/5] restart cleanups

[Kukjin Kim]

Russell King - ARM Linux wrote:
> 
> On Tue, Nov 01, 2011 at 04:38:17PM +0000, Russell King - ARM Linux wrote:
> > Can someone please check whether this code:
> >
> > /* Hook for arm_pm_restart to ensure we execute the reset code
> >  * with the caches enabled. It seems at least the S3C2440 has a problem
> >  * resetting if there is bus activity interrupted by the reset.
> >  */
> > static void s3c24xx_pm_restart(char mode, const char *cmd)
> > {
> >         if (mode != 's') {
> >                 unsigned long flags;
> >
> >                 local_irq_save(flags);
> >                 __cpuc_flush_kern_all();
> >                 __cpuc_flush_user_all();
> >
> >                 arch_reset(mode, cmd);
> >                 local_irq_restore(flags);
> >         }
> >
> >         /* fallback, or unhandled */
> >         arm_machine_restart(mode, cmd);
> > }
> >
> > will be broken by any of these patches - and more importantly whether
> > this code can simply be deleted with these five patches applied.
> 
> Okay, I'm going to delete the above and hope for the best; I'm taking
> the lack of reply as meaning that no one in the Samsung community is
> that interested in this code.

Russell,
Sorry for late response.

According to its commit message, it is needed...

[ARM] 4987/1: S3C24XX: Ensure watchdog reset initiated from cached code.

    There seems to be some problem with at-least the S3C2440 and
    bus traffic during an reset. It is unlikely, but still possible
    that the system will hang in such a way that the watchdog cannot
    get the system out of the state it is in.

    Change to making the code that calls the watchdog reset run from
    cached memory so that instruction fetches have quiesced before the
    watchdog fires.

But frankly, I'm not sure now and there is no S3C2440 on my desk :(
Let me check again, if no response on this, please keep going on.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

On Thu, Nov 03, 2011 at 06:24:30PM +0900, Kukjin Kim wrote:
> Russell King - ARM Linux wrote:
> > 
> > On Tue, Nov 01, 2011 at 04:38:17PM +0000, Russell King - ARM Linux wrote:
> > > Can someone please check whether this code:
> > >
> > > /* Hook for arm_pm_restart to ensure we execute the reset code
> > >  * with the caches enabled. It seems at least the S3C2440 has a problem
> > >  * resetting if there is bus activity interrupted by the reset.
> > >  */
> > > static void s3c24xx_pm_restart(char mode, const char *cmd)
> > > {
> > >         if (mode != 's') {
> > >                 unsigned long flags;
> > >
> > >                 local_irq_save(flags);
> > >                 __cpuc_flush_kern_all();
> > >                 __cpuc_flush_user_all();
> > >
> > >                 arch_reset(mode, cmd);
> > >                 local_irq_restore(flags);
> > >         }
> > >
> > >         /* fallback, or unhandled */
> > >         arm_machine_restart(mode, cmd);
> > > }
> > >
> > > will be broken by any of these patches - and more importantly whether
> > > this code can simply be deleted with these five patches applied.
> > 
> > Okay, I'm going to delete the above and hope for the best; I'm taking
> > the lack of reply as meaning that no one in the Samsung community is
> > that interested in this code.
> 
> Russell,
> Sorry for late response.
> 
> According to its commit message, it is needed...
> 
> [ARM] 4987/1: S3C24XX: Ensure watchdog reset initiated from cached code.
> 
>     There seems to be some problem with at-least the S3C2440 and
>     bus traffic during an reset. It is unlikely, but still possible
>     that the system will hang in such a way that the watchdog cannot
>     get the system out of the state it is in.
> 
>     Change to making the code that calls the watchdog reset run from
>     cached memory so that instruction fetches have quiesced before the
>     watchdog fires.
> 
> But frankly, I'm not sure now and there is no S3C2440 on my desk :(
> Let me check again, if no response on this, please keep going on.

And now consider it along side the set of patches that I posted as
part of this thread, which have the effect that arch_reset() will
now be called with caches enabled.

Given that, is it still required?

[RFC 0/5] restart cleanups

[Andrea Adami]

On Wed, Nov 2, 2011 at 3:49 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
>
> On Wed, Nov 02, 2011 at 01:49:07PM +0000, Richard Purdie wrote:
> > >From what I remember that hardware either always reboots or always
> > halts. I think the option was therefore left hardcoded to make it clear
> > it wasn't expected to work. Later Zaurii models could do either but
> > required some manual poking of registers to make it happen iirc.
> >
> > Regardless, you can probably clean this up as you suggest now.
>
> Thanks, I've now committed a change which removes the indirection
> entirely from poodle.
>
Hi, sorry for the late reply

I'm playing with poodle and I can confirm that poweroff does indeed
not work with 2.6.3x kernels.
See this thread:
http://lists.linuxtogo.org/pipermail/zaurus-devel/2011-May/000501.html

I'll be happy to test again and report

Regards

Andrea



> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC 0/5] restart cleanups

[Russell King - ARM Linux]

On Fri, Nov 04, 2011 at 02:58:02PM +0100, Andrea Adami wrote:
> On Wed, Nov 2, 2011 at 3:49 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> >
> > On Wed, Nov 02, 2011 at 01:49:07PM +0000, Richard Purdie wrote:
> > > >From what I remember that hardware either always reboots or always
> > > halts. I think the option was therefore left hardcoded to make it clear
> > > it wasn't expected to work. Later Zaurii models could do either but
> > > required some manual poking of registers to make it happen iirc.
> > >
> > > Regardless, you can probably clean this up as you suggest now.
> >
> > Thanks, I've now committed a change which removes the indirection
> > entirely from poodle.
> >
> Hi, sorry for the late reply
> 
> I'm playing with poodle and I can confirm that poweroff does indeed
> not work with 2.6.3x kernels.
> See this thread:
> http://lists.linuxtogo.org/pipermail/zaurus-devel/2011-May/000501.html

Okay, at least we know that restart works with that.

I'm not offering to try and change the poweroff behaviour for the better,
as I know absolutely nothing about the Zaurus platforms.

[RFC 0/5] restart cleanups

[Andrea Adami]

On Fri, Nov 4, 2011 at 3:01 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Nov 04, 2011 at 02:58:02PM +0100, Andrea Adami wrote:
>> On Wed, Nov 2, 2011 at 3:49 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> >
>> > On Wed, Nov 02, 2011 at 01:49:07PM +0000, Richard Purdie wrote:
>> > > >From what I remember that hardware either always reboots or always
>> > > halts. I think the option was therefore left hardcoded to make it clear
>> > > it wasn't expected to work. Later Zaurii models could do either but
>> > > required some manual poking of registers to make it happen iirc.
>> > >
>> > > Regardless, you can probably clean this up as you suggest now.
>> >
>> > Thanks, I've now committed a change which removes the indirection
>> > entirely from poodle.
>> >
>> Hi, sorry for the late reply
>>
>> I'm playing with poodle and I can confirm that poweroff does indeed
>> not work with 2.6.3x kernels.
>> See this thread:
>> http://lists.linuxtogo.org/pipermail/zaurus-devel/2011-May/000501.html
>
> Okay, at least we know that restart works with that.
Yes, indeed

>
> I'm not offering to try and change the poweroff behaviour for the better,
> as I know absolutely nothing about the Zaurus platforms.
>
I see, np. I'm just puzzled because after a cold reset (pulling
battery out) poodle is acting as if powered off and does not restart
itself.
Apparently we cannot reach this state again after boot.

FYI the modern zaurus (corgi and spitz) do
suspend/resume/poweroff/restart flawlessy so it's only this machine
failing poweroff (and maybe collie, no idea).

Regards

Andrea