[PATCH 1/3] ARM: PXA: Zipit Z2: Fix oops in z2_power_off

[PATCH 1/3] ARM: PXA: Zipit Z2: Fix oops in z2_power_off

[Vasily Khoruzhick]

pxa27x_set_pwrmode is marked with __init, so it's not legitimate
to call it in pm_power_off hook.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/include/mach/pxa27x.h |    2 +-
 arch/arm/mach-pxa/pxa27x.c              |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-pxa/include/mach/pxa27x.h b/arch/arm/mach-pxa/include/mach/pxa27x.h
index 7cff640..66c4cbf 100644
--- a/arch/arm/mach-pxa/include/mach/pxa27x.h
+++ b/arch/arm/mach-pxa/include/mach/pxa27x.h
@@ -21,7 +21,7 @@
 
 extern void __init pxa27x_map_io(void);
 extern void __init pxa27x_init_irq(void);
-extern int __init pxa27x_set_pwrmode(unsigned int mode);
+extern int pxa27x_set_pwrmode(unsigned int mode);
 extern void pxa27x_cpu_pm_enter(suspend_state_t state);
 
 #define pxa27x_handle_irq	ichp_handle_irq
diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index bc5a98e..acbaa2b 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -241,7 +241,7 @@ static struct clk_lookup pxa27x_clkregs[] = {
  */
 static unsigned int pwrmode = PWRMODE_SLEEP;
 
-int __init pxa27x_set_pwrmode(unsigned int mode)
+int pxa27x_set_pwrmode(unsigned int mode)
 {
 	switch (mode) {
 	case PWRMODE_SLEEP:
-- 
1.7.9.2

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Vasily Khoruzhick]

Bootloader can clobber PWER value, so save it state on suspend.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/pxa27x.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index acbaa2b..2951872 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -262,6 +262,7 @@ enum {
 	SLEEP_SAVE_PSTR,
 	SLEEP_SAVE_MDREFR,
 	SLEEP_SAVE_PCFR,
+	SLEEP_SAVE_PWER,
 	SLEEP_SAVE_COUNT
 };
 
@@ -269,7 +270,7 @@ void pxa27x_cpu_pm_save(unsigned long *sleep_save)
 {
 	sleep_save[SLEEP_SAVE_MDREFR] = __raw_readl(MDREFR);
 	SAVE(PCFR);
-
+	SAVE(PWER);
 	SAVE(PSTR);
 }
 
@@ -281,6 +282,7 @@ void pxa27x_cpu_pm_restore(unsigned long *sleep_save)
 	PSSR = PSSR_RDH | PSSR_PH;
 
 	RESTORE(PSTR);
+	RESTORE(PWER);
 }
 
 void pxa27x_cpu_pm_enter(suspend_state_t state)
-- 
1.7.9.2

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Russell King - ARM Linux]

On Sun, Feb 26, 2012 at 04:47:41PM +0300, Vasily Khoruzhick wrote:
> Bootloader can clobber PWER value, so save it state on suspend.

What boot loader is this, and why is it being so idiotic?  Why can't it
be fixed?

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Vasily Khoruzhick]

2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Sun, Feb 26, 2012 at 04:47:41PM +0300, Vasily Khoruzhick wrote:
>> Bootloader can clobber PWER value, so save it state on suspend.
>
> What boot loader is this, and why is it being so idiotic? ?Why can't it
> be fixed?

I'm using u-boot on this machine...
Hmm, I've looked throuhg its sources and did not find where it can clobber it.
But PWER register value is not preserved across suspend for some reason
(at least bit 0). Maybe it's some conflict with gpiolib? But
gpio_set_wake() doesn't work
for me on PXA.

Regards
Vasily

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Russell King - ARM Linux]

On Sun, Feb 26, 2012 at 05:55:09PM +0300, Vasily Khoruzhick wrote:
> 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > On Sun, Feb 26, 2012 at 04:47:41PM +0300, Vasily Khoruzhick wrote:
> >> Bootloader can clobber PWER value, so save it state on suspend.
> >
> > What boot loader is this, and why is it being so idiotic? ?Why can't it
> > be fixed?
> 
> I'm using u-boot on this machine...
> Hmm, I've looked throuhg its sources and did not find where it can clobber it.
> But PWER register value is not preserved across suspend for some reason
> (at least bit 0). Maybe it's some conflict with gpiolib? But
> gpio_set_wake() doesn't work
> for me on PXA.

It sounds like some more debugging is required.

I assume that you've done some basic things, such as:

(1) checking the return value of gpio_set_wake()
(2) checking what's going on internally in gpio_set_wake()

Most users of gpio_set_wake() do not seem to check its return value, so
if it fails you'll never know about it except by seeing incorrect values
in PWER.

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Marek Vasut]

> On Sun, Feb 26, 2012 at 04:47:41PM +0300, Vasily Khoruzhick wrote:
> > Bootloader can clobber PWER value, so save it state on suspend.
> 
> What boot loader is this,

U-Boot, patched, not mainline.

> and why is it being so idiotic?

Mainline doesn't touch PWER, so please be careful about your accusations.

> Why can't it
> be fixed?

Yes, either by using mainline uboot (to rule out the bootloader problem), 
finding bug in linux kernel or reading the CPU manual.

M

PS. Vasily, keep me in Cc next time.

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Russell King - ARM Linux]

On Sun, Feb 26, 2012 at 11:20:27PM +0100, Marek Vasut wrote:
> > On Sun, Feb 26, 2012 at 04:47:41PM +0300, Vasily Khoruzhick wrote:
> > > Bootloader can clobber PWER value, so save it state on suspend.
> > 
> > What boot loader is this,
> 
> U-Boot, patched, not mainline.
> 
> > and why is it being so idiotic?
> 
> Mainline doesn't touch PWER, so please be careful about your accusations.
> 
> > Why can't it
> > be fixed?
> 
> Yes, either by using mainline uboot (to rule out the bootloader problem), 
> finding bug in linux kernel or reading the CPU manual.

Was there supposed to be any useful information in your reply, or are
you just trying to be your usual antagonistic self?

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Marek Vasut]

> On Sun, Feb 26, 2012 at 11:20:27PM +0100, Marek Vasut wrote:
> > > On Sun, Feb 26, 2012 at 04:47:41PM +0300, Vasily Khoruzhick wrote:
> > > > Bootloader can clobber PWER value, so save it state on suspend.
> > > 
> > > What boot loader is this,
> > 
> > U-Boot, patched, not mainline.
> > 
> > > and why is it being so idiotic?
> > 
> > Mainline doesn't touch PWER, so please be careful about your accusations.
> > 
> > > Why can't it
> > > be fixed?
> > 
> > Yes, either by using mainline uboot (to rule out the bootloader problem),
> > finding bug in linux kernel or reading the CPU manual.
> 
> Was there supposed to be any useful information in your reply, or are
> you just trying to be your usual antagonistic self?

Um ... 

1) PWER isn't reconfigured by uboot (see above)
2) That you should try mainline uboot to avoid debugging problems with not-
mainlined patches
3) That the bug with PWER is likely in kernel, since there's an issue with 
gpio_set_wake() on PXA
4) That the CPU might not preserve PWER throughout the suspend and that this 
information might be found in the CPU manual

I was a bit terse there, sorry.

M

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Russell King - ARM Linux]

On Mon, Feb 27, 2012 at 12:47:01AM +0100, Marek Vasut wrote:
> > On Sun, Feb 26, 2012 at 11:20:27PM +0100, Marek Vasut wrote:
> > > > On Sun, Feb 26, 2012 at 04:47:41PM +0300, Vasily Khoruzhick wrote:
> > > > > Bootloader can clobber PWER value, so save it state on suspend.
> > > > 
> > > > What boot loader is this,
> > > 
> > > U-Boot, patched, not mainline.
> > > 
> > > > and why is it being so idiotic?
> > > 
> > > Mainline doesn't touch PWER, so please be careful about your accusations.
> > > 
> > > > Why can't it
> > > > be fixed?
> > > 
> > > Yes, either by using mainline uboot (to rule out the bootloader problem),
> > > finding bug in linux kernel or reading the CPU manual.
> > 
> > Was there supposed to be any useful information in your reply, or are
> > you just trying to be your usual antagonistic self?
> 
> Um ... 
> 
> 1) PWER isn't reconfigured by uboot (see above)

Given that virtually everyone seems to use a modified version of uboot,
how can you be so sure?

> 2) That you should try mainline uboot to avoid debugging problems with not-
> mainlined patches

Why should _I_ try anything with uboot?  I'm not the one experiencing
problems.

I'll point out that Vasily is reporting that _his_ boot loader, whatever
it is, is clobbering the PWER register.  That's *his* statement in the
commit log, and I have no reason *not* to think that he's put in the
research to confirm that.  Otherwise, why make such a positive statement
about where the problem lies?

> 3) That the bug with PWER is likely in kernel, since there's an issue with 
> gpio_set_wake() on PXA

I've reviewed this function and its associated data, and can't see
anything wrong with it (see below).

> 4) That the CPU might not preserve PWER throughout the suspend and that this 
> information might be found in the CPU manual

No.  The PXA270 manual says that this register is preserved on exit from
deep sleep and sleep modes.  It appends a note against each bit in the
reset status for the PWER register which says "Exit from sleep or
deep sleep mode does not clear or set this bit".

The setup for GPIO0 is:

        for (i = 0; i <= 15; i++) {
                /* skip GPIO2, 5, 6, 7, 8 */
                if (GPIO_bit(i) & 0x1e4)
                        continue;

                gpio_desc[i].can_wakeup = 1;
                gpio_desc[i].mask = GPIO_bit(i);
        }

So, gpio_desc[0].can_wakeup = 1 and gpio_desc[0].mask is the bitmask for
GPIO0.

gpio_set_wake() does this:

        d = &gpio_desc[gpio];
        c = d->config;

        if (!d->valid)
                return -EINVAL;

well, that's already been set valid by pxa27x_mfp_init().

        if (d->keypad_gpio && (MFP_AF(d->config) == 0) &&

it's not a keypad gpio...

        mux_taken = (PWER & d->mux_mask) & (~d->mask);
        if (on && mux_taken)
                return -EBUSY;

well, d->mux_mask is zero, so this check fails.

        if (d->can_wakeup && (c & MFP_LPM_CAN_WAKEUP)) {
                if (on) {
                        PWER = (PWER & ~d->mux_mask) | d->mask;

Now, we know d->can_wakeup is true.  However, what about the MFP
configuration?

Well, that depends on the platform.  I suspect that's where the problem
with wakeup sources not working is located, because the MFP configuration
hasn't been properly set, and that comes from platform code.

So please, stop blaming PXA code without full and proper analysis of
bugs.  It's a waste of everyone's time, and it hurts proper kernel
maintenance if these ill-researched 'workarounds' get merged instead
of proper fixes.

[PATCH 2/3] ARM: PXA27x: save/restore PWER on suspend/resume

[Marek Vasut]

> On Mon, Feb 27, 2012 at 12:47:01AM +0100, Marek Vasut wrote:
> > > On Sun, Feb 26, 2012 at 11:20:27PM +0100, Marek Vasut wrote:
> > > > > On Sun, Feb 26, 2012 at 04:47:41PM +0300, Vasily Khoruzhick wrote:
> > > > > > Bootloader can clobber PWER value, so save it state on suspend.
> > > > > 
> > > > > What boot loader is this,
> > > > 
> > > > U-Boot, patched, not mainline.
> > > > 
> > > > > and why is it being so idiotic?
> > > > 
> > > > Mainline doesn't touch PWER, so please be careful about your
> > > > accusations.
> > > > 
> > > > > Why can't it
> > > > > be fixed?
> > > > 
> > > > Yes, either by using mainline uboot (to rule out the bootloader
> > > > problem), finding bug in linux kernel or reading the CPU manual.
> > > 
> > > Was there supposed to be any useful information in your reply, or are
> > > you just trying to be your usual antagonistic self?
> > 
> > Um ...
> > 
> > 1) PWER isn't reconfigured by uboot (see above)
> 
> Given that virtually everyone seems to use a modified version of uboot,
> how can you be so sure?

It's impossible to support every single version of some bootloader patched in 
various obscure ways, sorry.

> 
> > 2) That you should try mainline uboot to avoid debugging problems with
> > not- mainlined patches
> 
> Why should _I_ try anything with uboot?  I'm not the one experiencing
> problems.
> 
> I'll point out that Vasily is reporting that _his_ boot loader, whatever
> it is, is clobbering the PWER register.  That's *his* statement in the
> commit log, and I have no reason *not* to think that he's put in the
> research to confirm that.  Otherwise, why make such a positive statement
> about where the problem lies?

Well if it does clober the register, he should use bootloader that's not broken.

> 
> > 3) That the bug with PWER is likely in kernel, since there's an issue
> > with gpio_set_wake() on PXA
> 
> I've reviewed this function and its associated data, and can't see
> anything wrong with it (see below).
> 
> > 4) That the CPU might not preserve PWER throughout the suspend and that
> > this information might be found in the CPU manual
> 
> No.  The PXA270 manual says that this register is preserved on exit from
> deep sleep and sleep modes.  It appends a note against each bit in the
> reset status for the PWER register which says "Exit from sleep or
> deep sleep mode does not clear or set this bit".
> 
> The setup for GPIO0 is:
> 
>         for (i = 0; i <= 15; i++) {
>                 /* skip GPIO2, 5, 6, 7, 8 */
>                 if (GPIO_bit(i) & 0x1e4)
>                         continue;
> 
>                 gpio_desc[i].can_wakeup = 1;
>                 gpio_desc[i].mask = GPIO_bit(i);
>         }
> 
> So, gpio_desc[0].can_wakeup = 1 and gpio_desc[0].mask is the bitmask for
> GPIO0.
> 
> gpio_set_wake() does this:
> 
>         d = &gpio_desc[gpio];
>         c = d->config;
> 
>         if (!d->valid)
>                 return -EINVAL;
> 
> well, that's already been set valid by pxa27x_mfp_init().
> 
>         if (d->keypad_gpio && (MFP_AF(d->config) == 0) &&
> 
> it's not a keypad gpio...
> 
>         mux_taken = (PWER & d->mux_mask) & (~d->mask);
>         if (on && mux_taken)
>                 return -EBUSY;
> 
> well, d->mux_mask is zero, so this check fails.
> 
>         if (d->can_wakeup && (c & MFP_LPM_CAN_WAKEUP)) {
>                 if (on) {
>                         PWER = (PWER & ~d->mux_mask) | d->mask;
> 
> Now, we know d->can_wakeup is true.  However, what about the MFP
> configuration?
> 
> Well, that depends on the platform.  I suspect that's where the problem
> with wakeup sources not working is located, because the MFP configuration
> hasn't been properly set, and that comes from platform code.

Ok, agreed.
> 
> So please, stop blaming PXA code without full and proper analysis of
> bugs.

I said in kernel, not in PXA code.

> It's a waste of everyone's time, and it hurts proper kernel
> maintenance if these ill-researched 'workarounds' get merged instead
> of proper fixes.

Agreed. So this all boils down to the point where Vasily should check his MFP 
settings.

M

[PATCH 3/3] ARM: PXA27x: Zipit Z2: disable wake on GPIO0

[Vasily Khoruzhick]

We don't want machine to wake up on AC state change

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
 arch/arm/mach-pxa/z2.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
index 0fb5617..bb672ee 100644
--- a/arch/arm/mach-pxa/z2.c
+++ b/arch/arm/mach-pxa/z2.c
@@ -746,6 +746,13 @@ static void z2_power_off(void)
 #define z2_power_off   NULL
 #endif
 
+static void __init z2_map_io(void)
+{
+	pxa27x_map_io();
+
+	PWER &= ~(1 << 0);
+}
+
 /******************************************************************************
  * Machine init
  ******************************************************************************/
@@ -776,7 +783,7 @@ static void __init z2_init(void)
 
 MACHINE_START(ZIPIT2, "Zipit Z2")
 	.atag_offset	= 0x100,
-	.map_io		= pxa27x_map_io,
+	.map_io		= z2_map_io,
 	.init_irq	= pxa27x_init_irq,
 	.handle_irq	= pxa27x_handle_irq,
 	.timer		= &pxa_timer,
-- 
1.7.9.2

[PATCH 3/3] ARM: PXA27x: Zipit Z2: disable wake on GPIO0

[Russell King - ARM Linux]

On Sun, Feb 26, 2012 at 04:47:42PM +0300, Vasily Khoruzhick wrote:
> We don't want machine to wake up on AC state change

NAK.  Stop polluting the machine callback methods.  Use the right one.

> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
>  arch/arm/mach-pxa/z2.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
> index 0fb5617..bb672ee 100644
> --- a/arch/arm/mach-pxa/z2.c
> +++ b/arch/arm/mach-pxa/z2.c
> @@ -746,6 +746,13 @@ static void z2_power_off(void)
>  #define z2_power_off   NULL
>  #endif
>  
> +static void __init z2_map_io(void)
> +{
> +	pxa27x_map_io();
> +
> +	PWER &= ~(1 << 0);
> +}
> +
>  /******************************************************************************
>   * Machine init
>   ******************************************************************************/
> @@ -776,7 +783,7 @@ static void __init z2_init(void)
>  
>  MACHINE_START(ZIPIT2, "Zipit Z2")
>  	.atag_offset	= 0x100,
> -	.map_io		= pxa27x_map_io,
> +	.map_io		= z2_map_io,
>  	.init_irq	= pxa27x_init_irq,
>  	.handle_irq	= pxa27x_handle_irq,
>  	.timer		= &pxa_timer,
> -- 
> 1.7.9.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] ARM: PXA27x: Zipit Z2: disable wake on GPIO0

[Vasily Khoruzhick]

2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Sun, Feb 26, 2012 at 04:47:42PM +0300, Vasily Khoruzhick wrote:
>> We don't want machine to wake up on AC state change
>
> NAK. ?Stop polluting the machine callback methods. ?Use the right one.

Ok, what's right one?

[PATCH 3/3] ARM: PXA27x: Zipit Z2: disable wake on GPIO0

[Russell King - ARM Linux]

On Sun, Feb 26, 2012 at 06:11:17PM +0300, Vasily Khoruzhick wrote:
> 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > On Sun, Feb 26, 2012 at 04:47:42PM +0300, Vasily Khoruzhick wrote:
> >> We don't want machine to wake up on AC state change
> >
> > NAK. ?Stop polluting the machine callback methods. ?Use the right one.
> 
> Ok, what's right one?

        void                    (*fixup)(struct tag *, char **,
                                         struct meminfo *);
        void                    (*reserve)(void);/* reserve mem blocks  */
        void                    (*map_io)(void);/* IO mapping function  */
        void                    (*init_early)(void);
        void                    (*init_irq)(void);
        struct sys_timer        *timer;         /* system tick timer    */
        void                    (*init_machine)(void);

and chose a more appropriate one from that?  Maybe init_early() if it
has to be done early, or preferably the init_machine() if it can wait
until arch_initcall() time.

But stuffing it into fixup, reserve, map_io, init_irq or the timer
initialization is pure abuse of those callbacks.

[PATCH 3/3] ARM: PXA27x: Zipit Z2: disable wake on GPIO0

[Vasily Khoruzhick]

2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Sun, Feb 26, 2012 at 06:11:17PM +0300, Vasily Khoruzhick wrote:
>> 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
>> > On Sun, Feb 26, 2012 at 04:47:42PM +0300, Vasily Khoruzhick wrote:
>> >> We don't want machine to wake up on AC state change
>> >
>> > NAK. ?Stop polluting the machine callback methods. ?Use the right one.
>>
>> Ok, what's right one?
>
> ? ? ? ?void ? ? ? ? ? ? ? ? ? ?(*fixup)(struct tag *, char **,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct meminfo *);
> ? ? ? ?void ? ? ? ? ? ? ? ? ? ?(*reserve)(void);/* reserve mem blocks ?*/
> ? ? ? ?void ? ? ? ? ? ? ? ? ? ?(*map_io)(void);/* IO mapping function ?*/
> ? ? ? ?void ? ? ? ? ? ? ? ? ? ?(*init_early)(void);
> ? ? ? ?void ? ? ? ? ? ? ? ? ? ?(*init_irq)(void);
> ? ? ? ?struct sys_timer ? ? ? ?*timer; ? ? ? ? /* system tick timer ? ?*/
> ? ? ? ?void ? ? ? ? ? ? ? ? ? ?(*init_machine)(void);
>
> and chose a more appropriate one from that? ?Maybe init_early() if it
> has to be done early, or preferably the init_machine() if it can wait
> until arch_initcall() time.
>
> But stuffing it into fixup, reserve, map_io, init_irq or the timer
> initialization is pure abuse of those callbacks.

Ok, thanks.

Regards
Vasily

[PATCH 3/3] ARM: PXA27x: Zipit Z2: disable wake on GPIO0

[Marek Vasut]

> 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > On Sun, Feb 26, 2012 at 06:11:17PM +0300, Vasily Khoruzhick wrote:
> >> 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> >> > On Sun, Feb 26, 2012 at 04:47:42PM +0300, Vasily Khoruzhick wrote:
> >> >> We don't want machine to wake up on AC state change
> >> > 
> >> > NAK.  Stop polluting the machine callback methods.  Use the right one.
> >> 
> >> Ok, what's right one?
> > 
> >        void                    (*fixup)(struct tag *, char **,
> >                                         struct meminfo *);
> >        void                    (*reserve)(void);/* reserve mem blocks  */
> >        void                    (*map_io)(void);/* IO mapping function  */
> >        void                    (*init_early)(void);
> >        void                    (*init_irq)(void);
> >        struct sys_timer        *timer;         /* system tick timer    */
> >        void                    (*init_machine)(void);
> > 
> > and chose a more appropriate one from that?  Maybe init_early() if it
> > has to be done early, or preferably the init_machine() if it can wait
> > until arch_initcall() time.
> > 
> > But stuffing it into fixup, reserve, map_io, init_irq or the timer
> > initialization is pure abuse of those callbacks.
> 
> Ok, thanks.
> 

Or this should be patched in the bootloader too. Also, please dont introduce ad-
hoc constants like "1 << 0", ever!

M

[PATCH 3/3] ARM: PXA27x: Zipit Z2: disable wake on GPIO0

[Russell King - ARM Linux]

On Sun, Feb 26, 2012 at 11:21:49PM +0100, Marek Vasut wrote:
> > 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > > On Sun, Feb 26, 2012 at 06:11:17PM +0300, Vasily Khoruzhick wrote:
> > >> 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > >> > On Sun, Feb 26, 2012 at 04:47:42PM +0300, Vasily Khoruzhick wrote:
> > >> >> We don't want machine to wake up on AC state change
> > >> > 
> > >> > NAK.  Stop polluting the machine callback methods.  Use the right one.
> > >> 
> > >> Ok, what's right one?
> > > 
> > >        void                    (*fixup)(struct tag *, char **,
> > >                                         struct meminfo *);
> > >        void                    (*reserve)(void);/* reserve mem blocks  */
> > >        void                    (*map_io)(void);/* IO mapping function  */
> > >        void                    (*init_early)(void);
> > >        void                    (*init_irq)(void);
> > >        struct sys_timer        *timer;         /* system tick timer    */
> > >        void                    (*init_machine)(void);
> > > 
> > > and chose a more appropriate one from that?  Maybe init_early() if it
> > > has to be done early, or preferably the init_machine() if it can wait
> > > until arch_initcall() time.
> > > 
> > > But stuffing it into fixup, reserve, map_io, init_irq or the timer
> > > initialization is pure abuse of those callbacks.
> > 
> > Ok, thanks.
> > 
> 
> Or this should be patched in the bootloader too. Also, please dont
* introduce ad-hoc constants like "1 << 0", ever!

Fiddling with PWER should not be in the boot loader.  The kernel handles
PWER just fine.

[PATCH 3/3] ARM: PXA27x: Zipit Z2: disable wake on GPIO0

[Marek Vasut]

> On Sun, Feb 26, 2012 at 11:21:49PM +0100, Marek Vasut wrote:
> > > 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > > > On Sun, Feb 26, 2012 at 06:11:17PM +0300, Vasily Khoruzhick wrote:
> > > >> 2012/2/26 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > > >> > On Sun, Feb 26, 2012 at 04:47:42PM +0300, Vasily Khoruzhick wrote:
> > > >> >> We don't want machine to wake up on AC state change
> > > >> > 
> > > >> > NAK.  Stop polluting the machine callback methods.  Use the right
> > > >> > one.
> > > >> 
> > > >> Ok, what's right one?
> > > >> 
> > > >        void                    (*fixup)(struct tag *, char **,
> > > >        
> > > >                                         struct meminfo *);
> > > >        
> > > >        void                    (*reserve)(void);/* reserve mem blocks
> > > >         */ void                    (*map_io)(void);/* IO mapping
> > > >        function  */ void                    (*init_early)(void);
> > > >        void                    (*init_irq)(void);
> > > >        struct sys_timer        *timer;         /* system tick timer  
> > > >         */ void                    (*init_machine)(void);
> > > > 
> > > > and chose a more appropriate one from that?  Maybe init_early() if it
> > > > has to be done early, or preferably the init_machine() if it can wait
> > > > until arch_initcall() time.
> > > > 
> > > > But stuffing it into fixup, reserve, map_io, init_irq or the timer
> > > > initialization is pure abuse of those callbacks.
> > > 
> > > Ok, thanks.
> > 
> > Or this should be patched in the bootloader too. Also, please dont
> 
> * introduce ad-hoc constants like "1 << 0", ever!
> 
> Fiddling with PWER should not be in the boot loader.  The kernel handles
> PWER just fine.

Right, good point