Re: [PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2011-01-06 Thread Kevin Hilman
Kevin Hilman  writes:

>  writes:
>
>> Hi,
>>
>> Kevin Hilman [khil...@ti.com]:
>>> Aaro Koskinen  writes:
>>>
>>> > Make !CONFIG_SUSPEND init declarations identical on all OMAPs and
>>> > eliminate some ifdefs.
>>> >
>>> > Signed-off-by: Aaro Koskinen 
>>>
>>> I like this solution, but it introduces compiler warnings:
>>>
>>> [...]
>>>
>>> As you likely noticed, removing the const leads to checkpatch warnings:
>>>
>>> WARNING: struct platform_suspend_ops should normally be const
>>>
>>> so the choice is between a checkpatch warning or a bunch of compiler
>>> warnings.
>>>
>>> Alternatively, I just posted a patch[1] to linux-pm propsing to fix this
>>> at the source.  Let's see what happens there.  Merging $SUBJECT patch
>>> will depend on how this is fixed upstream.
>>
>> Sorry, I should have mentioned this when I posted the patch. I was aware of
>> this issue, but I thought this was already fixed in upstream. Check the 
>> following
>> commit in linux-next:
>>
>> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=2f55ac072f5344519348c0c94b3d2f4cca46847b
>>
>> (suspend: constify platform_suspend_ops)
>
> ah, thanks.  somehow I missed that one when looking to see if this was
> already fixed.

OK, now your patch and the one from linux-next are conflicting as they
both touch the platform_suspend_ops in pm*.c.

So for now, I'm gonna drop this one, but it should be rebased/reposted as
soon as 2.6.38-rc1 is out.

Thanks,

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2011-01-06 Thread Kevin Hilman
 writes:

> Hi,
>
> Kevin Hilman [khil...@ti.com]:
>> Aaro Koskinen  writes:
>>
>> > Make !CONFIG_SUSPEND init declarations identical on all OMAPs and
>> > eliminate some ifdefs.
>> >
>> > Signed-off-by: Aaro Koskinen 
>>
>> I like this solution, but it introduces compiler warnings:
>>
>> [...]
>>
>> As you likely noticed, removing the const leads to checkpatch warnings:
>>
>> WARNING: struct platform_suspend_ops should normally be const
>>
>> so the choice is between a checkpatch warning or a bunch of compiler
>> warnings.
>>
>> Alternatively, I just posted a patch[1] to linux-pm propsing to fix this
>> at the source.  Let's see what happens there.  Merging $SUBJECT patch
>> will depend on how this is fixed upstream.
>
> Sorry, I should have mentioned this when I posted the patch. I was aware of
> this issue, but I thought this was already fixed in upstream. Check the 
> following
> commit in linux-next:
>
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=2f55ac072f5344519348c0c94b3d2f4cca46847b
>
> (suspend: constify platform_suspend_ops)

ah, thanks.  somehow I missed that one when looking to see if this was
already fixed.

Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2011-01-06 Thread Russell King - ARM Linux
On Thu, Jan 06, 2011 at 12:05:51PM +, aaro.koski...@nokia.com wrote:
> Hi,
> 
> Russell King - ARM Linux [li...@arm.linux.org.uk]:
> > > > -static struct platform_suspend_ops omap_pm_ops = {
> > > > -   .begin  = omap2_pm_begin,
> > > > -   .enter  = omap2_pm_enter,
> > > > -   .end= omap2_pm_end,
> > > > -   .valid  = suspend_valid_only_mem,
> > > > +static const struct platform_suspend_ops omap_pm_ops[] = {
> > > > +   {
> > > > +   .begin  = omap2_pm_begin,
> > > > +   .enter  = omap2_pm_enter,
> > > > +   .end= omap2_pm_end,
> > > > +   .valid  = suspend_valid_only_mem,
> > > > +   }
> > > >  };
> > > > -#else
> > > > -static const struct platform_suspend_ops __initdata omap_pm_ops;
> > > >  #endif /* CONFIG_SUSPEND */
> > > >
> > > >  /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
> > > > @@ -582,7 +582,7 @@ static int __init omap2_pm_init(void)
> > > > 
> > > > omap24xx_cpu_suspend_sz);
> > > > }
> > > >
> > > > -   suspend_set_ops(&omap_pm_ops);
> > > > +   suspend_set_ops(omap_pm_ops);
> >
> > Utterly yuck.  Declaring it as a single element array just to avoid an
> > ifdef.  That's worse than having an ifdef here.
> 
> Why it is worse? Reducing the amount of different preprocessor branches will
> result in better compile test / static analysis coverage.

You're not doing that.  You're just spreading the yuck around making it
far worse:

- in some header file
#ifndef CONFIG_FOO
#define omap_pm_ops NULL
#endif

- in lots of C files
#ifdef CONFIG_FOO
static const struct platform_suspend_ops omap_pm_ops[] = {
{
...
}
};
#endif

suspend_set_ops(omap_pm_ops);

So, rather than just having the full story in each file, it's spread
across two files.  Not only that, but over time CONFIG_FOO will probably
change, and that will lead to compile errors.

You're creating an array just to be able to use the symbol as a pointer.
That's a hack rather than an elegant solution.

So no, your implementation is _NOT_ a sane approach.

I'm going to explicitly say NAK to your patch here and now.  I really
don't like it one bit.  It's a complete hack through and through, and
that hack is spread across many files.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2011-01-06 Thread aaro.koskinen
Hi,

Russell King - ARM Linux [li...@arm.linux.org.uk]:
> > > -static struct platform_suspend_ops omap_pm_ops = {
> > > -   .begin  = omap2_pm_begin,
> > > -   .enter  = omap2_pm_enter,
> > > -   .end= omap2_pm_end,
> > > -   .valid  = suspend_valid_only_mem,
> > > +static const struct platform_suspend_ops omap_pm_ops[] = {
> > > +   {
> > > +   .begin  = omap2_pm_begin,
> > > +   .enter  = omap2_pm_enter,
> > > +   .end= omap2_pm_end,
> > > +   .valid  = suspend_valid_only_mem,
> > > +   }
> > >  };
> > > -#else
> > > -static const struct platform_suspend_ops __initdata omap_pm_ops;
> > >  #endif /* CONFIG_SUSPEND */
> > >
> > >  /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
> > > @@ -582,7 +582,7 @@ static int __init omap2_pm_init(void)
> > > omap24xx_cpu_suspend_sz);
> > > }
> > >
> > > -   suspend_set_ops(&omap_pm_ops);
> > > +   suspend_set_ops(omap_pm_ops);
>
> Utterly yuck.  Declaring it as a single element array just to avoid an
> ifdef.  That's worse than having an ifdef here.

Why it is worse? Reducing the amount of different preprocessor branches will
result in better compile test / static analysis coverage.

> There's another solution.  Don't mess about with sticking such stuff in
> the header either.
>
> #ifdef WHATEVER
> static const struct platform_suspend_ops omap_pm_ops = {
> .begin  = omap2_pm_begin,
> .enter  = omap2_pm_enter,
> .end= omap2_pm_end,
> .valid  = suspend_valid_only_mem,
> };
> #define PM_OPS omap_pm_ops
> #else
> #define PM_OPS NULL
> #endif
> ...
> 
> suspend_set_ops(PM_OPS);
>
> That keeps it all nice and local, and you can see exactly what's going on
> without having it spread across many different random files.

I don't think it's obviously better or simpler. There's already made a mistake 
in
your example...

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2011-01-06 Thread Russell King - ARM Linux
On Wed, Jan 05, 2011 at 04:25:32PM -0800, Kevin Hilman wrote:
> As you likely noticed, removing the const leads to checkpatch warnings:
> 
> WARNING: struct platform_suspend_ops should normally be const
> 
> so the choice is between a checkpatch warning or a bunch of compiler
> warnings.

checkpatch should not be checking that data declarations are const.
That's a decision for those people declaring them to decide upon, as
they may need to modify the structure before registration.

By doing so, we end up encouraging people to write more code - such
as declaring it __initconst, and then kmalloc'ing, memcpy'ing and
modifying that before registering it.

> > -static struct platform_suspend_ops omap_pm_ops = {
> > -   .begin  = omap2_pm_begin,
> > -   .enter  = omap2_pm_enter,
> > -   .end= omap2_pm_end,
> > -   .valid  = suspend_valid_only_mem,
> > +static const struct platform_suspend_ops omap_pm_ops[] = {
> > +   {
> > +   .begin  = omap2_pm_begin,
> > +   .enter  = omap2_pm_enter,
> > +   .end= omap2_pm_end,
> > +   .valid  = suspend_valid_only_mem,
> > +   }
> >  };
> > -#else
> > -static const struct platform_suspend_ops __initdata omap_pm_ops;
> >  #endif /* CONFIG_SUSPEND */
> >  
> >  /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
> > @@ -582,7 +582,7 @@ static int __init omap2_pm_init(void)
> > omap24xx_cpu_suspend_sz);
> > }
> >  
> > -   suspend_set_ops(&omap_pm_ops);
> > +   suspend_set_ops(omap_pm_ops);

Utterly yuck.  Declaring it as a single element array just to avoid an
ifdef.  That's worse than having an ifdef here.

There's another solution.  Don't mess about with sticking such stuff in
the header either.

#ifdef WHATEVER
static const struct platform_suspend_ops omap_pm_ops = {
.begin  = omap2_pm_begin,
.enter  = omap2_pm_enter,
.end= omap2_pm_end,
.valid  = suspend_valid_only_mem,
};
#define PM_OPS omap_pm_ops
#else
#define PM_OPS NULL
#endif
...

suspend_set_ops(PM_OPS);

That keeps it all nice and local, and you can see exactly what's going on
without having it spread across many different random files.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2011-01-06 Thread aaro.koskinen
Hi,

Kevin Hilman [khil...@ti.com]:
> Aaro Koskinen  writes:
>
> > Make !CONFIG_SUSPEND init declarations identical on all OMAPs and
> > eliminate some ifdefs.
> >
> > Signed-off-by: Aaro Koskinen 
>
> I like this solution, but it introduces compiler warnings:
>
> [...]
>
> As you likely noticed, removing the const leads to checkpatch warnings:
>
> WARNING: struct platform_suspend_ops should normally be const
>
> so the choice is between a checkpatch warning or a bunch of compiler
> warnings.
>
> Alternatively, I just posted a patch[1] to linux-pm propsing to fix this
> at the source.  Let's see what happens there.  Merging $SUBJECT patch
> will depend on how this is fixed upstream.

Sorry, I should have mentioned this when I posted the patch. I was aware of
this issue, but I thought this was already fixed in upstream. Check the 
following
commit in linux-next:

http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=2f55ac072f5344519348c0c94b3d2f4cca46847b

(suspend: constify platform_suspend_ops)

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2011-01-05 Thread Kevin Hilman
Hi Aaro,

Aaro Koskinen  writes:

> Make !CONFIG_SUSPEND init declarations identical on all OMAPs and
> eliminate some ifdefs.
>
> Signed-off-by: Aaro Koskinen 

I like this solution, but it introduces compiler warnings:

/opt/home/khilman/work.local/kernel/omap/pm/arch/arm/mach-omap2/pm44xx.c: In 
function 'omap4_pm_init':
/opt/home/khilman/work.local/kernel/omap/pm/arch/arm/mach-omap2/pm44xx.c:119: 
warning: passing argument 1 of 'suspend_set_ops' discards qualifiers from 
pointer target type
/opt/home/khilman/work.local/kernel/omap/pm/include/linux/suspend.h:125: note: 
expected 'struct platform_suspend_ops *' but argument is of type 'const struct 
platform_suspend_ops *'
/opt/home/khilman/work.local/kernel/omap/pm/arch/arm/mach-omap2/pm24xx.c: In 
function 'omap2_pm_init':
/opt/home/khilman/work.local/kernel/omap/pm/arch/arm/mach-omap2/pm24xx.c:585: 
warning: passing argument 1 of 'suspend_set_ops' discards qualifiers from 
pointer target type
/opt/home/khilman/work.local/kernel/omap/pm/include/linux/suspend.h:125: note: 
expected 'struct platform_suspend_ops *' but argument is of type 'const struct 
platform_suspend_ops *'
/opt/home/khilman/work.local/kernel/omap/pm/arch/arm/mach-omap2/pm34xx.c: In 
function 'omap3_pm_init':
/opt/home/khilman/work.local/kernel/omap/pm/arch/arm/mach-omap2/pm34xx.c:1072: 
warning: passing argument 1 of 'suspend_set_ops' discards qualifiers from 
pointer target type
/opt/home/khilman/work.local/kernel/omap/pm/include/linux/suspend.h:125:
note: expected 'struct platform_suspend_ops *' but argument is of type
'const struct platform_suspend_ops *'

As you likely noticed, removing the const leads to checkpatch warnings:

WARNING: struct platform_suspend_ops should normally be const

so the choice is between a checkpatch warning or a bunch of compiler
warnings.

Alternatively, I just posted a patch[1] to linux-pm propsing to fix this
at the source.  Let's see what happens there.  Merging $SUBJECT patch
will depend on how this is fixed upstream.

Kevin

[1] https://patchwork.kernel.org/patch/455831/

> ---
>  arch/arm/mach-omap2/pm.h |4 
>  arch/arm/mach-omap2/pm24xx.c |   16 
>  arch/arm/mach-omap2/pm34xx.c |   16 
>  arch/arm/mach-omap2/pm44xx.c |   17 +
>  4 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 1c1b0ab..704766b 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -138,4 +138,8 @@ static inline int omap4_twl_init(void)
>  }
>  #endif
>  
> +#ifndef CONFIG_SUSPEND
> +#define omap_pm_ops NULL
> +#endif
> +
>  #endif
> diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
> index dac2d1d..e65b329 100644
> --- a/arch/arm/mach-omap2/pm24xx.c
> +++ b/arch/arm/mach-omap2/pm24xx.c
> @@ -350,14 +350,14 @@ static void omap2_pm_end(void)
>   enable_hlt();
>  }
>  
> -static struct platform_suspend_ops omap_pm_ops = {
> - .begin  = omap2_pm_begin,
> - .enter  = omap2_pm_enter,
> - .end= omap2_pm_end,
> - .valid  = suspend_valid_only_mem,
> +static const struct platform_suspend_ops omap_pm_ops[] = {
> + {
> + .begin  = omap2_pm_begin,
> + .enter  = omap2_pm_enter,
> + .end= omap2_pm_end,
> + .valid  = suspend_valid_only_mem,
> + }
>  };
> -#else
> -static const struct platform_suspend_ops __initdata omap_pm_ops;
>  #endif /* CONFIG_SUSPEND */
>  
>  /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
> @@ -582,7 +582,7 @@ static int __init omap2_pm_init(void)
>   omap24xx_cpu_suspend_sz);
>   }
>  
> - suspend_set_ops(&omap_pm_ops);
> + suspend_set_ops(omap_pm_ops);
>   pm_idle = omap2_pm_idle;
>  
>   return 0;
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 5b323f2..a4c9283 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -605,11 +605,13 @@ static void omap3_pm_end(void)
>   return;
>  }
>  
> -static struct platform_suspend_ops omap_pm_ops = {
> - .begin  = omap3_pm_begin,
> - .end= omap3_pm_end,
> - .enter  = omap3_pm_enter,
> - .valid  = suspend_valid_only_mem,
> +static const struct platform_suspend_ops omap_pm_ops[] = {
> + {
> + .begin  = omap3_pm_begin,
> + .end= omap3_pm_end,
> + .enter  = omap3_pm_enter,
> + .valid  = suspend_valid_only_mem,
> + }
>  };
>  #endif /* CONFIG_SUSPEND */
>  
> @@ -1067,9 +1069,7 @@ static int __init omap3_pm_init(void)
>   core_clkdm = clkdm_lookup("core_clkdm");
>  
>   omap_push_sram_idle();
> -#ifdef CONFIG_SUSPEND
> - suspend_set_ops(&omap_pm_ops);
> -#endif /* CONFIG_SUSPEND */
> + suspend_s

Re: [PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2011-01-04 Thread Kevin Hilman
Aaro Koskinen  writes:

> Make !CONFIG_SUSPEND init declarations identical on all OMAPs and
> eliminate some ifdefs.
>
> Signed-off-by: Aaro Koskinen 

Very nice.

I was going to tackle something like this with that last cleanup I did,
but was rather late for the main 2.6.38 merge window so I just fixed
problem at hand.  This is much better, thanks.

Will queue for 2.6.38-rc2 in my pm-fixes branch.

Kevin

> ---
>  arch/arm/mach-omap2/pm.h |4 
>  arch/arm/mach-omap2/pm24xx.c |   16 
>  arch/arm/mach-omap2/pm34xx.c |   16 
>  arch/arm/mach-omap2/pm44xx.c |   17 +
>  4 files changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 1c1b0ab..704766b 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -138,4 +138,8 @@ static inline int omap4_twl_init(void)
>  }
>  #endif
>  
> +#ifndef CONFIG_SUSPEND
> +#define omap_pm_ops NULL
> +#endif
> +
>  #endif
> diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
> index dac2d1d..e65b329 100644
> --- a/arch/arm/mach-omap2/pm24xx.c
> +++ b/arch/arm/mach-omap2/pm24xx.c
> @@ -350,14 +350,14 @@ static void omap2_pm_end(void)
>   enable_hlt();
>  }
>  
> -static struct platform_suspend_ops omap_pm_ops = {
> - .begin  = omap2_pm_begin,
> - .enter  = omap2_pm_enter,
> - .end= omap2_pm_end,
> - .valid  = suspend_valid_only_mem,
> +static const struct platform_suspend_ops omap_pm_ops[] = {
> + {
> + .begin  = omap2_pm_begin,
> + .enter  = omap2_pm_enter,
> + .end= omap2_pm_end,
> + .valid  = suspend_valid_only_mem,
> + }
>  };
> -#else
> -static const struct platform_suspend_ops __initdata omap_pm_ops;
>  #endif /* CONFIG_SUSPEND */
>  
>  /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
> @@ -582,7 +582,7 @@ static int __init omap2_pm_init(void)
>   omap24xx_cpu_suspend_sz);
>   }
>  
> - suspend_set_ops(&omap_pm_ops);
> + suspend_set_ops(omap_pm_ops);
>   pm_idle = omap2_pm_idle;
>  
>   return 0;
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 5b323f2..a4c9283 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -605,11 +605,13 @@ static void omap3_pm_end(void)
>   return;
>  }
>  
> -static struct platform_suspend_ops omap_pm_ops = {
> - .begin  = omap3_pm_begin,
> - .end= omap3_pm_end,
> - .enter  = omap3_pm_enter,
> - .valid  = suspend_valid_only_mem,
> +static const struct platform_suspend_ops omap_pm_ops[] = {
> + {
> + .begin  = omap3_pm_begin,
> + .end= omap3_pm_end,
> + .enter  = omap3_pm_enter,
> + .valid  = suspend_valid_only_mem,
> + }
>  };
>  #endif /* CONFIG_SUSPEND */
>  
> @@ -1067,9 +1069,7 @@ static int __init omap3_pm_init(void)
>   core_clkdm = clkdm_lookup("core_clkdm");
>  
>   omap_push_sram_idle();
> -#ifdef CONFIG_SUSPEND
> - suspend_set_ops(&omap_pm_ops);
> -#endif /* CONFIG_SUSPEND */
> + suspend_set_ops(omap_pm_ops);
>  
>   pm_idle = omap3_pm_idle;
>   omap3_idle_init();
> diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
> index e9f4862..6022c0a 100644
> --- a/arch/arm/mach-omap2/pm44xx.c
> +++ b/arch/arm/mach-omap2/pm44xx.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  
> +#include "pm.h"
>  #include "powerdomain.h"
>  #include 
>  
> @@ -65,11 +66,13 @@ static void omap4_pm_end(void)
>   return;
>  }
>  
> -static struct platform_suspend_ops omap_pm_ops = {
> - .begin  = omap4_pm_begin,
> - .end= omap4_pm_end,
> - .enter  = omap4_pm_enter,
> - .valid  = suspend_valid_only_mem,
> +static const struct platform_suspend_ops omap_pm_ops[] = {
> + {
> + .begin  = omap4_pm_begin,
> + .end= omap4_pm_end,
> + .enter  = omap4_pm_enter,
> + .valid  = suspend_valid_only_mem,
> + }
>  };
>  #endif /* CONFIG_SUSPEND */
>  
> @@ -113,9 +116,7 @@ static int __init omap4_pm_init(void)
>   }
>  #endif
>  
> -#ifdef CONFIG_SUSPEND
> - suspend_set_ops(&omap_pm_ops);
> -#endif /* CONFIG_SUSPEND */
> + suspend_set_ops(omap_pm_ops);
>  
>  err2:
>   return ret;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] arm: mach-omap2: pm: cleanup !CONFIG_SUSPEND handling

2010-12-29 Thread Aaro Koskinen
Make !CONFIG_SUSPEND init declarations identical on all OMAPs and
eliminate some ifdefs.

Signed-off-by: Aaro Koskinen 
---
 arch/arm/mach-omap2/pm.h |4 
 arch/arm/mach-omap2/pm24xx.c |   16 
 arch/arm/mach-omap2/pm34xx.c |   16 
 arch/arm/mach-omap2/pm44xx.c |   17 +
 4 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 1c1b0ab..704766b 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -138,4 +138,8 @@ static inline int omap4_twl_init(void)
 }
 #endif
 
+#ifndef CONFIG_SUSPEND
+#define omap_pm_ops NULL
+#endif
+
 #endif
diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
index dac2d1d..e65b329 100644
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -350,14 +350,14 @@ static void omap2_pm_end(void)
enable_hlt();
 }
 
-static struct platform_suspend_ops omap_pm_ops = {
-   .begin  = omap2_pm_begin,
-   .enter  = omap2_pm_enter,
-   .end= omap2_pm_end,
-   .valid  = suspend_valid_only_mem,
+static const struct platform_suspend_ops omap_pm_ops[] = {
+   {
+   .begin  = omap2_pm_begin,
+   .enter  = omap2_pm_enter,
+   .end= omap2_pm_end,
+   .valid  = suspend_valid_only_mem,
+   }
 };
-#else
-static const struct platform_suspend_ops __initdata omap_pm_ops;
 #endif /* CONFIG_SUSPEND */
 
 /* XXX This function should be shareable between OMAP2xxx and OMAP3 */
@@ -582,7 +582,7 @@ static int __init omap2_pm_init(void)
omap24xx_cpu_suspend_sz);
}
 
-   suspend_set_ops(&omap_pm_ops);
+   suspend_set_ops(omap_pm_ops);
pm_idle = omap2_pm_idle;
 
return 0;
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 5b323f2..a4c9283 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -605,11 +605,13 @@ static void omap3_pm_end(void)
return;
 }
 
-static struct platform_suspend_ops omap_pm_ops = {
-   .begin  = omap3_pm_begin,
-   .end= omap3_pm_end,
-   .enter  = omap3_pm_enter,
-   .valid  = suspend_valid_only_mem,
+static const struct platform_suspend_ops omap_pm_ops[] = {
+   {
+   .begin  = omap3_pm_begin,
+   .end= omap3_pm_end,
+   .enter  = omap3_pm_enter,
+   .valid  = suspend_valid_only_mem,
+   }
 };
 #endif /* CONFIG_SUSPEND */
 
@@ -1067,9 +1069,7 @@ static int __init omap3_pm_init(void)
core_clkdm = clkdm_lookup("core_clkdm");
 
omap_push_sram_idle();
-#ifdef CONFIG_SUSPEND
-   suspend_set_ops(&omap_pm_ops);
-#endif /* CONFIG_SUSPEND */
+   suspend_set_ops(omap_pm_ops);
 
pm_idle = omap3_pm_idle;
omap3_idle_init();
diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
index e9f4862..6022c0a 100644
--- a/arch/arm/mach-omap2/pm44xx.c
+++ b/arch/arm/mach-omap2/pm44xx.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 
+#include "pm.h"
 #include "powerdomain.h"
 #include 
 
@@ -65,11 +66,13 @@ static void omap4_pm_end(void)
return;
 }
 
-static struct platform_suspend_ops omap_pm_ops = {
-   .begin  = omap4_pm_begin,
-   .end= omap4_pm_end,
-   .enter  = omap4_pm_enter,
-   .valid  = suspend_valid_only_mem,
+static const struct platform_suspend_ops omap_pm_ops[] = {
+   {
+   .begin  = omap4_pm_begin,
+   .end= omap4_pm_end,
+   .enter  = omap4_pm_enter,
+   .valid  = suspend_valid_only_mem,
+   }
 };
 #endif /* CONFIG_SUSPEND */
 
@@ -113,9 +116,7 @@ static int __init omap4_pm_init(void)
}
 #endif
 
-#ifdef CONFIG_SUSPEND
-   suspend_set_ops(&omap_pm_ops);
-#endif /* CONFIG_SUSPEND */
+   suspend_set_ops(omap_pm_ops);
 
 err2:
return ret;
-- 
1.5.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html