Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-03-16 Thread Tony Lindgren
* Jon Hunter  [150212 04:37]:
> 
> On 02/12/2015 11:26 AM, Jon Hunter wrote:
> > 
> > On 02/11/2015 09:14 PM, Tony Lindgren wrote:
> >> * Paul Walmsley  [150211 13:03]:
> >>> On Wed, 11 Feb 2015, Tony Lindgren wrote:
> >>>
>  * Paul Walmsley  [150210 18:28]:
> > On Tue, 10 Feb 2015, Jon Hunter wrote:
> >> On 07/02/2015 00:23, Paul Walmsley wrote:
> >
> >> Unfortunately, there is not a single TRM for the omap5910 but 
> >> individual 
> >> documents for each chapter in the original TRM. Check out the 
> >> "OMAP5910 
> >> Dual-Core Processor Timer Reference Guide" and possibly the "OMAP5910 
> >> Dual-Core Processor Clock Generation and System Reset Management 
> >> Reference Guide"
> >>
> >> The omap15xx/5910 did have a 32k timer but as you can see it appears it
> >> was never supported by the kernel for this device (not sure why). I do
> >> recall that there is some errata regarding the 32k timer, if you look 
> >> at
> >> the omap5910 errata document and search for 32k you should find it.
> >
> > OK thanks for the context.  I probably am not going to investigate 
> > adding 
> > support for this timer on OMAP1510/5910 - am primarily trying to avoid 
> > causing a regression on the existing platforms.
> 
>  At least I've never seen the 32KiHz timer registers in any 15xx
>  documentation. Jon are you sure you're not mixing up 5910 (15xx)
>  and 5912 (16xx)?
> >>>
> >>> It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 
> >>> 3 
> >>> "32-kHz Timer", at the link Jon mentioned.  Have not checked the errata 
> >>> that Jon mentioned though.
> >>
> >> Interesting. Looks like it's the same as on 16xx at 0xfffb9000.
> >> AFAIK that never worked on 15xx. Or maybe the issue was that 15xx
> >> is missing the constantly running 32KiHz counter making the timer
> >> unusable from PM point of view as the clockevent alone is not enough.
> >>
> >>> Regarding the patch: I'd suggest keeping the compilation warning fixes 
> >>> (which was the original purpose of the patch) from anything that changes 
> >>> the logic too much.   That way if there's an error in the patch that 
> >>> changes the logic and it needs to be reverted, it won't also revert the 
> >>> warning fixes.
> >>
> >> Makes sense to me.
> > 
> > Yes that's fine with me as well, I don't wish to over complicate
> > matters. I have a couple minor comments though and will respond to the
> > latest patch rev.
> 
> Actually, nevermind the latest version is fine with me. Jon

Applying the second version into omap-for-v4.1/fixes-not-urgent.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-03-16 Thread Tony Lindgren
* Jon Hunter jgchun...@gmail.com [150212 04:37]:
 
 On 02/12/2015 11:26 AM, Jon Hunter wrote:
  
  On 02/11/2015 09:14 PM, Tony Lindgren wrote:
  * Paul Walmsley p...@pwsan.com [150211 13:03]:
  On Wed, 11 Feb 2015, Tony Lindgren wrote:
 
  * Paul Walmsley p...@pwsan.com [150210 18:28]:
  On Tue, 10 Feb 2015, Jon Hunter wrote:
  On 07/02/2015 00:23, Paul Walmsley wrote:
 
  Unfortunately, there is not a single TRM for the omap5910 but 
  individual 
  documents for each chapter in the original TRM. Check out the 
  OMAP5910 
  Dual-Core Processor Timer Reference Guide and possibly the OMAP5910 
  Dual-Core Processor Clock Generation and System Reset Management 
  Reference Guide
 
  The omap15xx/5910 did have a 32k timer but as you can see it appears it
  was never supported by the kernel for this device (not sure why). I do
  recall that there is some errata regarding the 32k timer, if you look 
  at
  the omap5910 errata document and search for 32k you should find it.
 
  OK thanks for the context.  I probably am not going to investigate 
  adding 
  support for this timer on OMAP1510/5910 - am primarily trying to avoid 
  causing a regression on the existing platforms.
 
  At least I've never seen the 32KiHz timer registers in any 15xx
  documentation. Jon are you sure you're not mixing up 5910 (15xx)
  and 5912 (16xx)?
 
  It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 
  3 
  32-kHz Timer, at the link Jon mentioned.  Have not checked the errata 
  that Jon mentioned though.
 
  Interesting. Looks like it's the same as on 16xx at 0xfffb9000.
  AFAIK that never worked on 15xx. Or maybe the issue was that 15xx
  is missing the constantly running 32KiHz counter making the timer
  unusable from PM point of view as the clockevent alone is not enough.
 
  Regarding the patch: I'd suggest keeping the compilation warning fixes 
  (which was the original purpose of the patch) from anything that changes 
  the logic too much.   That way if there's an error in the patch that 
  changes the logic and it needs to be reverted, it won't also revert the 
  warning fixes.
 
  Makes sense to me.
  
  Yes that's fine with me as well, I don't wish to over complicate
  matters. I have a couple minor comments though and will respond to the
  latest patch rev.
 
 Actually, nevermind the latest version is fine with me. Jon

Applying the second version into omap-for-v4.1/fixes-not-urgent.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-12 Thread Jon Hunter

On 02/12/2015 11:26 AM, Jon Hunter wrote:
> 
> On 02/11/2015 09:14 PM, Tony Lindgren wrote:
>> * Paul Walmsley  [150211 13:03]:
>>> On Wed, 11 Feb 2015, Tony Lindgren wrote:
>>>
 * Paul Walmsley  [150210 18:28]:
> On Tue, 10 Feb 2015, Jon Hunter wrote:
>> On 07/02/2015 00:23, Paul Walmsley wrote:
>
>> Unfortunately, there is not a single TRM for the omap5910 but individual 
>> documents for each chapter in the original TRM. Check out the "OMAP5910 
>> Dual-Core Processor Timer Reference Guide" and possibly the "OMAP5910 
>> Dual-Core Processor Clock Generation and System Reset Management 
>> Reference Guide"
>>
>> The omap15xx/5910 did have a 32k timer but as you can see it appears it
>> was never supported by the kernel for this device (not sure why). I do
>> recall that there is some errata regarding the 32k timer, if you look at
>> the omap5910 errata document and search for 32k you should find it.
>
> OK thanks for the context.  I probably am not going to investigate adding 
> support for this timer on OMAP1510/5910 - am primarily trying to avoid 
> causing a regression on the existing platforms.

 At least I've never seen the 32KiHz timer registers in any 15xx
 documentation. Jon are you sure you're not mixing up 5910 (15xx)
 and 5912 (16xx)?
>>>
>>> It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 3 
>>> "32-kHz Timer", at the link Jon mentioned.  Have not checked the errata 
>>> that Jon mentioned though.
>>
>> Interesting. Looks like it's the same as on 16xx at 0xfffb9000.
>> AFAIK that never worked on 15xx. Or maybe the issue was that 15xx
>> is missing the constantly running 32KiHz counter making the timer
>> unusable from PM point of view as the clockevent alone is not enough.
>>
>>> Regarding the patch: I'd suggest keeping the compilation warning fixes 
>>> (which was the original purpose of the patch) from anything that changes 
>>> the logic too much.   That way if there's an error in the patch that 
>>> changes the logic and it needs to be reverted, it won't also revert the 
>>> warning fixes.
>>
>> Makes sense to me.
> 
> Yes that's fine with me as well, I don't wish to over complicate
> matters. I have a couple minor comments though and will respond to the
> latest patch rev.

Actually, nevermind the latest version is fine with me. Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-12 Thread Jon Hunter

On 02/11/2015 09:14 PM, Tony Lindgren wrote:
> * Paul Walmsley  [150211 13:03]:
>> On Wed, 11 Feb 2015, Tony Lindgren wrote:
>>
>>> * Paul Walmsley  [150210 18:28]:
 On Tue, 10 Feb 2015, Jon Hunter wrote:
> On 07/02/2015 00:23, Paul Walmsley wrote:

> Unfortunately, there is not a single TRM for the omap5910 but individual 
> documents for each chapter in the original TRM. Check out the "OMAP5910 
> Dual-Core Processor Timer Reference Guide" and possibly the "OMAP5910 
> Dual-Core Processor Clock Generation and System Reset Management 
> Reference Guide"
>
> The omap15xx/5910 did have a 32k timer but as you can see it appears it
> was never supported by the kernel for this device (not sure why). I do
> recall that there is some errata regarding the 32k timer, if you look at
> the omap5910 errata document and search for 32k you should find it.

 OK thanks for the context.  I probably am not going to investigate adding 
 support for this timer on OMAP1510/5910 - am primarily trying to avoid 
 causing a regression on the existing platforms.
>>>
>>> At least I've never seen the 32KiHz timer registers in any 15xx
>>> documentation. Jon are you sure you're not mixing up 5910 (15xx)
>>> and 5912 (16xx)?
>>
>> It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 3 
>> "32-kHz Timer", at the link Jon mentioned.  Have not checked the errata 
>> that Jon mentioned though.
> 
> Interesting. Looks like it's the same as on 16xx at 0xfffb9000.
> AFAIK that never worked on 15xx. Or maybe the issue was that 15xx
> is missing the constantly running 32KiHz counter making the timer
> unusable from PM point of view as the clockevent alone is not enough.
> 
>> Regarding the patch: I'd suggest keeping the compilation warning fixes 
>> (which was the original purpose of the patch) from anything that changes 
>> the logic too much.   That way if there's an error in the patch that 
>> changes the logic and it needs to be reverted, it won't also revert the 
>> warning fixes.
> 
> Makes sense to me.

Yes that's fine with me as well, I don't wish to over complicate
matters. I have a couple minor comments though and will respond to the
latest patch rev.

Cheers
Jon


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


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-12 Thread Jon Hunter

On 02/11/2015 09:14 PM, Tony Lindgren wrote:
 * Paul Walmsley p...@pwsan.com [150211 13:03]:
 On Wed, 11 Feb 2015, Tony Lindgren wrote:

 * Paul Walmsley p...@pwsan.com [150210 18:28]:
 On Tue, 10 Feb 2015, Jon Hunter wrote:
 On 07/02/2015 00:23, Paul Walmsley wrote:

 Unfortunately, there is not a single TRM for the omap5910 but individual 
 documents for each chapter in the original TRM. Check out the OMAP5910 
 Dual-Core Processor Timer Reference Guide and possibly the OMAP5910 
 Dual-Core Processor Clock Generation and System Reset Management 
 Reference Guide

 The omap15xx/5910 did have a 32k timer but as you can see it appears it
 was never supported by the kernel for this device (not sure why). I do
 recall that there is some errata regarding the 32k timer, if you look at
 the omap5910 errata document and search for 32k you should find it.

 OK thanks for the context.  I probably am not going to investigate adding 
 support for this timer on OMAP1510/5910 - am primarily trying to avoid 
 causing a regression on the existing platforms.

 At least I've never seen the 32KiHz timer registers in any 15xx
 documentation. Jon are you sure you're not mixing up 5910 (15xx)
 and 5912 (16xx)?

 It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 3 
 32-kHz Timer, at the link Jon mentioned.  Have not checked the errata 
 that Jon mentioned though.
 
 Interesting. Looks like it's the same as on 16xx at 0xfffb9000.
 AFAIK that never worked on 15xx. Or maybe the issue was that 15xx
 is missing the constantly running 32KiHz counter making the timer
 unusable from PM point of view as the clockevent alone is not enough.
 
 Regarding the patch: I'd suggest keeping the compilation warning fixes 
 (which was the original purpose of the patch) from anything that changes 
 the logic too much.   That way if there's an error in the patch that 
 changes the logic and it needs to be reverted, it won't also revert the 
 warning fixes.
 
 Makes sense to me.

Yes that's fine with me as well, I don't wish to over complicate
matters. I have a couple minor comments though and will respond to the
latest patch rev.

Cheers
Jon


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-12 Thread Jon Hunter

On 02/12/2015 11:26 AM, Jon Hunter wrote:
 
 On 02/11/2015 09:14 PM, Tony Lindgren wrote:
 * Paul Walmsley p...@pwsan.com [150211 13:03]:
 On Wed, 11 Feb 2015, Tony Lindgren wrote:

 * Paul Walmsley p...@pwsan.com [150210 18:28]:
 On Tue, 10 Feb 2015, Jon Hunter wrote:
 On 07/02/2015 00:23, Paul Walmsley wrote:

 Unfortunately, there is not a single TRM for the omap5910 but individual 
 documents for each chapter in the original TRM. Check out the OMAP5910 
 Dual-Core Processor Timer Reference Guide and possibly the OMAP5910 
 Dual-Core Processor Clock Generation and System Reset Management 
 Reference Guide

 The omap15xx/5910 did have a 32k timer but as you can see it appears it
 was never supported by the kernel for this device (not sure why). I do
 recall that there is some errata regarding the 32k timer, if you look at
 the omap5910 errata document and search for 32k you should find it.

 OK thanks for the context.  I probably am not going to investigate adding 
 support for this timer on OMAP1510/5910 - am primarily trying to avoid 
 causing a regression on the existing platforms.

 At least I've never seen the 32KiHz timer registers in any 15xx
 documentation. Jon are you sure you're not mixing up 5910 (15xx)
 and 5912 (16xx)?

 It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 3 
 32-kHz Timer, at the link Jon mentioned.  Have not checked the errata 
 that Jon mentioned though.

 Interesting. Looks like it's the same as on 16xx at 0xfffb9000.
 AFAIK that never worked on 15xx. Or maybe the issue was that 15xx
 is missing the constantly running 32KiHz counter making the timer
 unusable from PM point of view as the clockevent alone is not enough.

 Regarding the patch: I'd suggest keeping the compilation warning fixes 
 (which was the original purpose of the patch) from anything that changes 
 the logic too much.   That way if there's an error in the patch that 
 changes the logic and it needs to be reverted, it won't also revert the 
 warning fixes.

 Makes sense to me.
 
 Yes that's fine with me as well, I don't wish to over complicate
 matters. I have a couple minor comments though and will respond to the
 latest patch rev.

Actually, nevermind the latest version is fine with me. Jon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Tony Lindgren
* Paul Walmsley  [150211 13:03]:
> On Wed, 11 Feb 2015, Tony Lindgren wrote:
> 
> > * Paul Walmsley  [150210 18:28]:
> > > On Tue, 10 Feb 2015, Jon Hunter wrote:
> > > > On 07/02/2015 00:23, Paul Walmsley wrote:
> > > 
> > > > Unfortunately, there is not a single TRM for the omap5910 but 
> > > > individual 
> > > > documents for each chapter in the original TRM. Check out the "OMAP5910 
> > > > Dual-Core Processor Timer Reference Guide" and possibly the "OMAP5910 
> > > > Dual-Core Processor Clock Generation and System Reset Management 
> > > > Reference Guide"
> > > > 
> > > > The omap15xx/5910 did have a 32k timer but as you can see it appears it
> > > > was never supported by the kernel for this device (not sure why). I do
> > > > recall that there is some errata regarding the 32k timer, if you look at
> > > > the omap5910 errata document and search for 32k you should find it.
> > > 
> > > OK thanks for the context.  I probably am not going to investigate adding 
> > > support for this timer on OMAP1510/5910 - am primarily trying to avoid 
> > > causing a regression on the existing platforms.
> > 
> > At least I've never seen the 32KiHz timer registers in any 15xx
> > documentation. Jon are you sure you're not mixing up 5910 (15xx)
> > and 5912 (16xx)?
> 
> It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 3 
> "32-kHz Timer", at the link Jon mentioned.  Have not checked the errata 
> that Jon mentioned though.

Interesting. Looks like it's the same as on 16xx at 0xfffb9000.
AFAIK that never worked on 15xx. Or maybe the issue was that 15xx
is missing the constantly running 32KiHz counter making the timer
unusable from PM point of view as the clockevent alone is not enough.

> Regarding the patch: I'd suggest keeping the compilation warning fixes 
> (which was the original purpose of the patch) from anything that changes 
> the logic too much.   That way if there's an error in the patch that 
> changes the logic and it needs to be reverted, it won't also revert the 
> warning fixes.

Makes sense to me.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Tony Lindgren
* Jon Hunter  [150211 09:43]:
> 
> Thinking about this some more, I don't understand the dependency on the
> DM_TIMER here. For an omap1 device, regardless of whether the DM_TIMERs
> are enable or not, the device should be able to enter low power if the
> 32K is enabled. Hence, shouldn't this have been !(CONFIG_OMAP_32K_TIMER)
> above?

Sounds about right, there are separate timers on omap1 and additional
gp timers. There's no 32KiHz timer on 1510 variants, including
720 and 730.
 
> Furthermore, you will get the above warning on a omap16xx only build if
> you disable DM_TIMERs and keep MPU_TIMER enabled, which should be a
> valid thing to do.
> 
> Tony, I see you added the DM_TIMER dependency in commit
> be26a008414414c69a4ae9fe9877401c3ba62c5a. I understand your motivation,
> but why not just use !(CONFIG_OMAP_32K_TIMER) here? Bit 9 of the idlect1
> is only for the TIMCK clock that is used for the MPU timers and not the
> DM_TIMERs.

Hmm yes looks like you're right. That check be done based on
!CONFIG_OMAP_32K_TIMER like you're saying.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Tony Lindgren
* Paul Walmsley  [150210 18:28]:
> On Tue, 10 Feb 2015, Jon Hunter wrote:
> > On 07/02/2015 00:23, Paul Walmsley wrote:
> 
> > Unfortunately, there is not a single TRM for the omap5910 but individual 
> > documents for each chapter in the original TRM. Check out the "OMAP5910 
> > Dual-Core Processor Timer Reference Guide" and possibly the "OMAP5910 
> > Dual-Core Processor Clock Generation and System Reset Management 
> > Reference Guide"
> > 
> > The omap15xx/5910 did have a 32k timer but as you can see it appears it
> > was never supported by the kernel for this device (not sure why). I do
> > recall that there is some errata regarding the 32k timer, if you look at
> > the omap5910 errata document and search for 32k you should find it.
> 
> OK thanks for the context.  I probably am not going to investigate adding 
> support for this timer on OMAP1510/5910 - am primarily trying to avoid 
> causing a regression on the existing platforms.

At least I've never seen the 32KiHz timer registers in any 15xx
documentation. Jon are you sure you're not mixing up 5910 (15xx)
and 5912 (16xx)?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Paul Walmsley
On Wed, 11 Feb 2015, Tony Lindgren wrote:

> * Paul Walmsley  [150210 18:28]:
> > On Tue, 10 Feb 2015, Jon Hunter wrote:
> > > On 07/02/2015 00:23, Paul Walmsley wrote:
> > 
> > > Unfortunately, there is not a single TRM for the omap5910 but individual 
> > > documents for each chapter in the original TRM. Check out the "OMAP5910 
> > > Dual-Core Processor Timer Reference Guide" and possibly the "OMAP5910 
> > > Dual-Core Processor Clock Generation and System Reset Management 
> > > Reference Guide"
> > > 
> > > The omap15xx/5910 did have a 32k timer but as you can see it appears it
> > > was never supported by the kernel for this device (not sure why). I do
> > > recall that there is some errata regarding the 32k timer, if you look at
> > > the omap5910 errata document and search for 32k you should find it.
> > 
> > OK thanks for the context.  I probably am not going to investigate adding 
> > support for this timer on OMAP1510/5910 - am primarily trying to avoid 
> > causing a regression on the existing platforms.
> 
> At least I've never seen the 32KiHz timer registers in any 15xx
> documentation. Jon are you sure you're not mixing up 5910 (15xx)
> and 5912 (16xx)?

It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 3 
"32-kHz Timer", at the link Jon mentioned.  Have not checked the errata 
that Jon mentioned though.

Regarding the patch: I'd suggest keeping the compilation warning fixes 
(which was the original purpose of the patch) from anything that changes 
the logic too much.   That way if there's an error in the patch that 
changes the logic and it needs to be reverted, it won't also revert the 
warning fixes.


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Jon Hunter
Hi Paul,

On 02/11/2015 02:25 AM, Paul Walmsley wrote:
> Hi John,
> 
> thanks for the review,
> 
> On Tue, 10 Feb 2015, Jon Hunter wrote:

[snip]

> Subject: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs
> 
> Building an OMAP1510-only Kconfig generates the following warnings:
> 
> arch/arm/mach-omap1/pm.c: In function ‘omap1_pm_idle’:
> arch/arm/mach-omap1/pm.c:123:2: warning: #warning Enable 32kHz OS timer in 
> order to allow sleep states in idle [-Wcpp]
>  #warning Enable 32kHz OS timer in order to allow sleep states in idle
>   ^
> arch/arm/mach-omap1/pm.c: At top level:
> arch/arm/mach-omap1/pm.c:76:23: warning: ‘enable_dyn_sleep’ defined but not 
> used [-Wunused-variable]
>  static unsigned short enable_dyn_sleep = 0;
>^
> 
> These are not so easy to fix in an obviously correct fashion, since I
> don't have these devices up and running in my testbed.  So, use
> arch/arm/plat-omap/Kconfig and the existing pm.c source as a guide,
> and posit that deep power saving states are only supported on OMAP16xx
> chips with kernels built with both CONFIG_OMAP_DM_TIMER=y and
> CONFIG_OMAP_32K_TIMER=y.
> 
> While here, clean up a few printk()s and unnecessary #ifdefs.
> 
> This second version of the patch incorporates several suggestions from
> Jon Hunter .
> 
> Signed-off-by: Paul Walmsley 
> Cc: Jon Hunter 
> Cc: Aaro Koskinen 
> Cc: Tuukka Tikkanen 
> Cc: Kevin Hilman 
> Cc: Tony Lindgren 
> Cc: Russell King 
> Cc: linux-o...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Acked-by: Jon Hunter 
> ---
>  arch/arm/mach-omap1/pm.c | 51 
> 
>  1 file changed, 25 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
> index 34b4c0044961..dd94567c3628 100644
> --- a/arch/arm/mach-omap1/pm.c
> +++ b/arch/arm/mach-omap1/pm.c
> @@ -71,13 +71,7 @@ static unsigned int 
> mpui7xx_sleep_save[MPUI7XX_SLEEP_SAVE_SIZE];
>  static unsigned int mpui1510_sleep_save[MPUI1510_SLEEP_SAVE_SIZE];
>  static unsigned int mpui1610_sleep_save[MPUI1610_SLEEP_SAVE_SIZE];
>  
> -#ifndef CONFIG_OMAP_32K_TIMER
> -
> -static unsigned short enable_dyn_sleep = 0;
> -
> -#else
> -
> -static unsigned short enable_dyn_sleep = 1;
> +static unsigned short enable_dyn_sleep;
>  
>  static ssize_t idle_show(struct kobject *kobj, struct kobj_attribute *attr,
>char *buf)
> @@ -90,8 +84,9 @@ static ssize_t idle_store(struct kobject *kobj, struct 
> kobj_attribute *attr,
>  {
>   unsigned short value;
>   if (sscanf(buf, "%hu", ) != 1 ||
> - (value != 0 && value != 1)) {
> - printk(KERN_ERR "idle_sleep_store: Invalid value\n");
> + (value != 0 && value != 1) ||
> + (value != 0 && !IS_ENABLED(CONFIG_OMAP_32K_TIMER))) {
> + pr_err("idle_sleep_store: Invalid value\n");
>   return -EINVAL;
>   }
>   enable_dyn_sleep = value;
> @@ -101,7 +96,6 @@ static ssize_t idle_store(struct kobject *kobj, struct 
> kobj_attribute *attr,
>  static struct kobj_attribute sleep_while_idle_attr =
>   __ATTR(sleep_while_idle, 0644, idle_show, idle_store);
>  
> -#endif
>  
>  static void (*omap_sram_suspend)(unsigned long r0, unsigned long r1) = NULL;
>  
> @@ -115,16 +109,11 @@ void omap1_pm_idle(void)
>  {
>   extern __u32 arm_idlect1_mask;
>   __u32 use_idlect1 = arm_idlect1_mask;
> - int do_sleep = 0;
>  
>   local_fiq_disable();
>  
>  #if defined(CONFIG_OMAP_MPU_TIMER) && !defined(CONFIG_OMAP_DM_TIMER)
> -#warning Enable 32kHz OS timer in order to allow sleep states in idle

Thinking about this some more, I don't understand the dependency on the
DM_TIMER here. For an omap1 device, regardless of whether the DM_TIMERs
are enable or not, the device should be able to enter low power if the
32K is enabled. Hence, shouldn't this have been !(CONFIG_OMAP_32K_TIMER)
above?

Furthermore, you will get the above warning on a omap16xx only build if
you disable DM_TIMERs and keep MPU_TIMER enabled, which should be a
valid thing to do.

Tony, I see you added the DM_TIMER dependency in commit
be26a008414414c69a4ae9fe9877401c3ba62c5a. I understand your motivation,
but why not just use !(CONFIG_OMAP_32K_TIMER) here? Bit 9 of the idlect1
is only for the TIMCK clock that is used for the MPU timers and not the
DM_TIMERs.

-#if defined(CONFIG_OMAP_MPU_TIMER) && !defined(CONFIG_OMAP_DM_TIMER)
-#warning Enable 32kHz OS timer in order to allow sleep states in idle
+#if !defined(CONFIG_OMAP_32K_TIMER)
use_idlect1 = u

Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Jon Hunter
Hi Paul,

On 02/11/2015 02:25 AM, Paul Walmsley wrote:
 Hi John,
 
 thanks for the review,
 
 On Tue, 10 Feb 2015, Jon Hunter wrote:

[snip]

 Subject: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs
 
 Building an OMAP1510-only Kconfig generates the following warnings:
 
 arch/arm/mach-omap1/pm.c: In function ‘omap1_pm_idle’:
 arch/arm/mach-omap1/pm.c:123:2: warning: #warning Enable 32kHz OS timer in 
 order to allow sleep states in idle [-Wcpp]
  #warning Enable 32kHz OS timer in order to allow sleep states in idle
   ^
 arch/arm/mach-omap1/pm.c: At top level:
 arch/arm/mach-omap1/pm.c:76:23: warning: ‘enable_dyn_sleep’ defined but not 
 used [-Wunused-variable]
  static unsigned short enable_dyn_sleep = 0;
^
 
 These are not so easy to fix in an obviously correct fashion, since I
 don't have these devices up and running in my testbed.  So, use
 arch/arm/plat-omap/Kconfig and the existing pm.c source as a guide,
 and posit that deep power saving states are only supported on OMAP16xx
 chips with kernels built with both CONFIG_OMAP_DM_TIMER=y and
 CONFIG_OMAP_32K_TIMER=y.
 
 While here, clean up a few printk()s and unnecessary #ifdefs.
 
 This second version of the patch incorporates several suggestions from
 Jon Hunter jgchun...@gmail.com.
 
 Signed-off-by: Paul Walmsley p...@pwsan.com
 Cc: Jon Hunter jonath...@nvidia.com
 Cc: Aaro Koskinen aaro.koski...@iki.fi
 Cc: Tuukka Tikkanen tuukka.tikka...@linaro.org
 Cc: Kevin Hilman khil...@deeprootsystems.com
 Cc: Tony Lindgren t...@atomide.com
 Cc: Russell King li...@arm.linux.org.uk
 Cc: linux-o...@vger.kernel.org
 Cc: linux-arm-ker...@lists.infradead.org
 Cc: linux-kernel@vger.kernel.org
 Acked-by: Jon Hunter jgchun...@gmail.com
 ---
  arch/arm/mach-omap1/pm.c | 51 
 
  1 file changed, 25 insertions(+), 26 deletions(-)
 
 diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
 index 34b4c0044961..dd94567c3628 100644
 --- a/arch/arm/mach-omap1/pm.c
 +++ b/arch/arm/mach-omap1/pm.c
 @@ -71,13 +71,7 @@ static unsigned int 
 mpui7xx_sleep_save[MPUI7XX_SLEEP_SAVE_SIZE];
  static unsigned int mpui1510_sleep_save[MPUI1510_SLEEP_SAVE_SIZE];
  static unsigned int mpui1610_sleep_save[MPUI1610_SLEEP_SAVE_SIZE];
  
 -#ifndef CONFIG_OMAP_32K_TIMER
 -
 -static unsigned short enable_dyn_sleep = 0;
 -
 -#else
 -
 -static unsigned short enable_dyn_sleep = 1;
 +static unsigned short enable_dyn_sleep;
  
  static ssize_t idle_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
 @@ -90,8 +84,9 @@ static ssize_t idle_store(struct kobject *kobj, struct 
 kobj_attribute *attr,
  {
   unsigned short value;
   if (sscanf(buf, %hu, value) != 1 ||
 - (value != 0  value != 1)) {
 - printk(KERN_ERR idle_sleep_store: Invalid value\n);
 + (value != 0  value != 1) ||
 + (value != 0  !IS_ENABLED(CONFIG_OMAP_32K_TIMER))) {
 + pr_err(idle_sleep_store: Invalid value\n);
   return -EINVAL;
   }
   enable_dyn_sleep = value;
 @@ -101,7 +96,6 @@ static ssize_t idle_store(struct kobject *kobj, struct 
 kobj_attribute *attr,
  static struct kobj_attribute sleep_while_idle_attr =
   __ATTR(sleep_while_idle, 0644, idle_show, idle_store);
  
 -#endif
  
  static void (*omap_sram_suspend)(unsigned long r0, unsigned long r1) = NULL;
  
 @@ -115,16 +109,11 @@ void omap1_pm_idle(void)
  {
   extern __u32 arm_idlect1_mask;
   __u32 use_idlect1 = arm_idlect1_mask;
 - int do_sleep = 0;
  
   local_fiq_disable();
  
  #if defined(CONFIG_OMAP_MPU_TIMER)  !defined(CONFIG_OMAP_DM_TIMER)
 -#warning Enable 32kHz OS timer in order to allow sleep states in idle

Thinking about this some more, I don't understand the dependency on the
DM_TIMER here. For an omap1 device, regardless of whether the DM_TIMERs
are enable or not, the device should be able to enter low power if the
32K is enabled. Hence, shouldn't this have been !(CONFIG_OMAP_32K_TIMER)
above?

Furthermore, you will get the above warning on a omap16xx only build if
you disable DM_TIMERs and keep MPU_TIMER enabled, which should be a
valid thing to do.

Tony, I see you added the DM_TIMER dependency in commit
be26a008414414c69a4ae9fe9877401c3ba62c5a. I understand your motivation,
but why not just use !(CONFIG_OMAP_32K_TIMER) here? Bit 9 of the idlect1
is only for the TIMCK clock that is used for the MPU timers and not the
DM_TIMERs.

-#if defined(CONFIG_OMAP_MPU_TIMER)  !defined(CONFIG_OMAP_DM_TIMER)
-#warning Enable 32kHz OS timer in order to allow sleep states in idle
+#if !defined(CONFIG_OMAP_32K_TIMER)
use_idlect1 = use_idlect1  ~(1  9);
-#else
-   if (enable_dyn_sleep)
-   do_sleep = 1;
 #endif


   use_idlect1 = use_idlect1  ~(1  9);
 -#else
 - if (enable_dyn_sleep)
 - do_sleep = 1;
  #endif
  
  #ifdef CONFIG_OMAP_DM_TIMER
 @@ -134,10 +123,12 @@ void omap1_pm_idle(void

Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Tony Lindgren
* Paul Walmsley p...@pwsan.com [150210 18:28]:
 On Tue, 10 Feb 2015, Jon Hunter wrote:
  On 07/02/2015 00:23, Paul Walmsley wrote:
 
  Unfortunately, there is not a single TRM for the omap5910 but individual 
  documents for each chapter in the original TRM. Check out the OMAP5910 
  Dual-Core Processor Timer Reference Guide and possibly the OMAP5910 
  Dual-Core Processor Clock Generation and System Reset Management 
  Reference Guide
  
  The omap15xx/5910 did have a 32k timer but as you can see it appears it
  was never supported by the kernel for this device (not sure why). I do
  recall that there is some errata regarding the 32k timer, if you look at
  the omap5910 errata document and search for 32k you should find it.
 
 OK thanks for the context.  I probably am not going to investigate adding 
 support for this timer on OMAP1510/5910 - am primarily trying to avoid 
 causing a regression on the existing platforms.

At least I've never seen the 32KiHz timer registers in any 15xx
documentation. Jon are you sure you're not mixing up 5910 (15xx)
and 5912 (16xx)?

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Tony Lindgren
* Jon Hunter jgchun...@gmail.com [150211 09:43]:
 
 Thinking about this some more, I don't understand the dependency on the
 DM_TIMER here. For an omap1 device, regardless of whether the DM_TIMERs
 are enable or not, the device should be able to enter low power if the
 32K is enabled. Hence, shouldn't this have been !(CONFIG_OMAP_32K_TIMER)
 above?

Sounds about right, there are separate timers on omap1 and additional
gp timers. There's no 32KiHz timer on 1510 variants, including
720 and 730.
 
 Furthermore, you will get the above warning on a omap16xx only build if
 you disable DM_TIMERs and keep MPU_TIMER enabled, which should be a
 valid thing to do.
 
 Tony, I see you added the DM_TIMER dependency in commit
 be26a008414414c69a4ae9fe9877401c3ba62c5a. I understand your motivation,
 but why not just use !(CONFIG_OMAP_32K_TIMER) here? Bit 9 of the idlect1
 is only for the TIMCK clock that is used for the MPU timers and not the
 DM_TIMERs.

Hmm yes looks like you're right. That check be done based on
!CONFIG_OMAP_32K_TIMER like you're saying.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Paul Walmsley
On Wed, 11 Feb 2015, Tony Lindgren wrote:

 * Paul Walmsley p...@pwsan.com [150210 18:28]:
  On Tue, 10 Feb 2015, Jon Hunter wrote:
   On 07/02/2015 00:23, Paul Walmsley wrote:
  
   Unfortunately, there is not a single TRM for the omap5910 but individual 
   documents for each chapter in the original TRM. Check out the OMAP5910 
   Dual-Core Processor Timer Reference Guide and possibly the OMAP5910 
   Dual-Core Processor Clock Generation and System Reset Management 
   Reference Guide
   
   The omap15xx/5910 did have a 32k timer but as you can see it appears it
   was never supported by the kernel for this device (not sure why). I do
   recall that there is some errata regarding the 32k timer, if you look at
   the omap5910 errata document and search for 32k you should find it.
  
  OK thanks for the context.  I probably am not going to investigate adding 
  support for this timer on OMAP1510/5910 - am primarily trying to avoid 
  causing a regression on the existing platforms.
 
 At least I've never seen the 32KiHz timer registers in any 15xx
 documentation. Jon are you sure you're not mixing up 5910 (15xx)
 and 5912 (16xx)?

It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 3 
32-kHz Timer, at the link Jon mentioned.  Have not checked the errata 
that Jon mentioned though.

Regarding the patch: I'd suggest keeping the compilation warning fixes 
(which was the original purpose of the patch) from anything that changes 
the logic too much.   That way if there's an error in the patch that 
changes the logic and it needs to be reverted, it won't also revert the 
warning fixes.


- Paul
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-11 Thread Tony Lindgren
* Paul Walmsley p...@pwsan.com [150211 13:03]:
 On Wed, 11 Feb 2015, Tony Lindgren wrote:
 
  * Paul Walmsley p...@pwsan.com [150210 18:28]:
   On Tue, 10 Feb 2015, Jon Hunter wrote:
On 07/02/2015 00:23, Paul Walmsley wrote:
   
Unfortunately, there is not a single TRM for the omap5910 but 
individual 
documents for each chapter in the original TRM. Check out the OMAP5910 
Dual-Core Processor Timer Reference Guide and possibly the OMAP5910 
Dual-Core Processor Clock Generation and System Reset Management 
Reference Guide

The omap15xx/5910 did have a 32k timer but as you can see it appears it
was never supported by the kernel for this device (not sure why). I do
recall that there is some errata regarding the 32k timer, if you look at
the omap5910 errata document and search for 32k you should find it.
   
   OK thanks for the context.  I probably am not going to investigate adding 
   support for this timer on OMAP1510/5910 - am primarily trying to avoid 
   causing a regression on the existing platforms.
  
  At least I've never seen the 32KiHz timer registers in any 15xx
  documentation. Jon are you sure you're not mixing up 5910 (15xx)
  and 5912 (16xx)?
 
 It's documented in the OMAP5910 Timer Reference Guide (SPRU682A) Section 3 
 32-kHz Timer, at the link Jon mentioned.  Have not checked the errata 
 that Jon mentioned though.

Interesting. Looks like it's the same as on 16xx at 0xfffb9000.
AFAIK that never worked on 15xx. Or maybe the issue was that 15xx
is missing the constantly running 32KiHz counter making the timer
unusable from PM point of view as the clockevent alone is not enough.

 Regarding the patch: I'd suggest keeping the compilation warning fixes 
 (which was the original purpose of the patch) from anything that changes 
 the logic too much.   That way if there's an error in the patch that 
 changes the logic and it needs to be reverted, it won't also revert the 
 warning fixes.

Makes sense to me.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-10 Thread Paul Walmsley
VAL;
> > }
> > enable_dyn_sleep = value;
> > @@ -101,7 +96,6 @@ static ssize_t idle_store(struct kobject *kobj,
> > struct kobj_attribute *attr,
> >  static struct kobj_attribute sleep_while_idle_attr =
> > __ATTR(sleep_while_idle, 0644, idle_show, idle_store);
> > 
> > -#endif
> > 
> >  static void (*omap_sram_suspend)(unsigned long r0, unsigned long r1) =
> > NULL;
> > 
> > @@ -120,12 +114,11 @@ void omap1_pm_idle(void)
> > local_fiq_disable();
> > 
> >  #if defined(CONFIG_OMAP_MPU_TIMER) && !defined(CONFIG_OMAP_DM_TIMER)
> > -#warning Enable 32kHz OS timer in order to allow sleep states in idle
> > use_idlect1 = use_idlect1 & ~(1 << 9);
> > -#else
> > +#endif
> > +
> > if (enable_dyn_sleep)
> > do_sleep = 1;
> 
> Do we still need this do_sleep variable now? Looking at the code, I
> think we could use enable_dyn_sleep directly.

OK dropped it.

> 
> > -#endif
> > 
> >  #ifdef CONFIG_OMAP_DM_TIMER
> > use_idlect1 = omap_dm_timer_modify_idlect_mask(use_idlect1);
> > @@ -635,15 +628,24 @@ static const struct platform_suspend_ops
> > omap_pm_ops = {
> > 
> >  static int __init omap_pm_init(void)
> >  {
> > -
> > -#ifdef CONFIG_OMAP_32K_TIMER
> > -   int error;
> > -#endif
> > +   int error = 0;
> > 
> > if (!cpu_class_is_omap1())
> > return -ENODEV;
> > 
> > -   printk("Power Management for TI OMAP.\n");
> > +   pr_info("Power Management for TI OMAP.\n");
> > +
> > +   if (!IS_ENABLED(CONFIG_OMAP_32K_TIMER))
> > +   pr_info("OMAP1 PM: sleep states in idle disabled due to no 
> > 32KiHz
> > timer\n");
> > +
> > +   if (!IS_ENABLED(CONFIG_OMAP_DM_TIMER))
> > +   pr_info("OMAP1 PM: sleep states in idle disabled due to no 
> > DMTIMER
> > support\n");
> > +
> > +   if (IS_ENABLED(CONFIG_OMAP_32K_TIMER) &&
> > +   IS_ENABLED(CONFIG_OMAP_DM_TIMER) && cpu_is_omap16xx()) {
> 
> Do you need cpu_is_omap16xx() here? I believe DM_TIMER is only available
> on omap16xx onwards.

Makes sense, I'll drop the cpu_is* test.

> > +   pr_info("OMAP1 PM: sleep states in idle enabled\n");
> > +   enable_dyn_sleep = 1;
> > +   }
> > 
> > /*
> >  * We copy the assembler sleep/wakeup routines to SRAM.
> > @@ -693,17 +695,15 @@ static int __init omap_pm_init(void)
> > omap_pm_init_debugfs();
> >  #endif
> > 
> > -#ifdef CONFIG_OMAP_32K_TIMER
> > error = sysfs_create_file(power_kobj, _while_idle_attr.attr);
> > if (error)
> > printk(KERN_ERR "sysfs_create_file failed: %d\n", error);
> > -#endif
> > 
> > if (cpu_is_omap16xx()) {
> > /* configure LOW_PWR pin */
> > omap_cfg_reg(T20_1610_LOW_PWR);
> > }
> > 
> > -   return 0;
> > +   return error;
> >  }
> >  __initcall(omap_pm_init);
> > 
> 
> Otherwise ...
> 
> Acked-by: Jon Hunter 
> 
> Cheers Jon

Updated patch follows.


- Paul

From: Paul Walmsley 
Date: Fri, 6 Feb 2015 15:56:07 -0700
Subject: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

Building an OMAP1510-only Kconfig generates the following warnings:

arch/arm/mach-omap1/pm.c: In function ‘omap1_pm_idle’:
arch/arm/mach-omap1/pm.c:123:2: warning: #warning Enable 32kHz OS timer in 
order to allow sleep states in idle [-Wcpp]
 #warning Enable 32kHz OS timer in order to allow sleep states in idle
  ^
arch/arm/mach-omap1/pm.c: At top level:
arch/arm/mach-omap1/pm.c:76:23: warning: ‘enable_dyn_sleep’ defined but not 
used [-Wunused-variable]
 static unsigned short enable_dyn_sleep = 0;
   ^

These are not so easy to fix in an obviously correct fashion, since I
don't have these devices up and running in my testbed.  So, use
arch/arm/plat-omap/Kconfig and the existing pm.c source as a guide,
and posit that deep power saving states are only supported on OMAP16xx
chips with kernels built with both CONFIG_OMAP_DM_TIMER=y and
CONFIG_OMAP_32K_TIMER=y.

While here, clean up a few printk()s and unnecessary #ifdefs.

This second version of the patch incorporates several suggestions from
Jon Hunter .

Signed-off-by: Paul Walmsley 
Cc: Jon Hunter 
Cc: Aaro Koskinen 
Cc: Tuukka Tikkanen 
Cc: Kevin Hilman 
Cc: Tony Lindgren 
Cc: Russell King 
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Jon Hunter 
---
 arch/arm/mach-omap1/pm.c | 51 +

Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-10 Thread Jon Hunter
Hi Paul,

On 07/02/2015 00:23, Paul Walmsley wrote:
> 
> Building an OMAP1510-only Kconfig generates the following warnings:
> 
> arch/arm/mach-omap1/pm.c: In function ‘omap1_pm_idle’:
> arch/arm/mach-omap1/pm.c:123:2: warning: #warning Enable 32kHz OS timer
> in order to allow sleep states in idle [-Wcpp]
>  #warning Enable 32kHz OS timer in order to allow sleep states in idle
>   ^
> arch/arm/mach-omap1/pm.c: At top level:
> arch/arm/mach-omap1/pm.c:76:23: warning: ‘enable_dyn_sleep’ defined but
> not used [-Wunused-variable]
>  static unsigned short enable_dyn_sleep = 0;
>^
> 
> These are not so easy to fix in an obviously correct fashion, since I
> don't have these devices up and running in my testbed.  So, use
> arch/arm/plat-omap/Kconfig and the existing pm.c source as a guide,
> and posit that deep power saving states are only supported on OMAP16xx
> chips with kernels built with both CONFIG_OMAP_DM_TIMER=y and
> CONFIG_OMAP_32K_TIMER=y.
> 
> While here, clean up a few printk()s and unnecessary #ifdefs.
> 
> Signed-off-by: Paul Walmsley 
> Cc: Jon Hunter 
> Cc: Aaro Koskinen 
> Cc: Tuukka Tikkanen 
> Cc: Kevin Hilman 
> Cc: Tony Lindgren 
> Cc: Russell King 
> Cc: linux-o...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> 
> Hi folks, if anyone out there is still experimenting with OMAP1 PM, or has
> a copy of the OMAP1510 TRMs, could you please check this patch?  I'm
> unable to test it since I don't have any OMAP1 devices currently active
> in the testbed.  It at least compiles and deals with the build warnings:

You can find the omap5910 documents here [1]. The omap5910 is equivalent
to the omap1510. Unfortunately, there is not a single TRM for the
omap5910 but individual documents for each chapter in the original TRM.
Check out the "OMAP5910 Dual-Core Processor Timer Reference Guide" and
possibly the "OMAP5910 Dual-Core Processor
Clock Generation and System Reset Management Reference Guide"

The omap15xx/5910 did have a 32k timer but as you can see it appears it
was never supported by the kernel for this device (not sure why). I do
recall that there is some errata regarding the 32k timer, if you look at
the omap5910 errata document and search for 32k you should find it.

I no longer have access to omap15xx h/w to test. However, I do have
omap5912 if you want me to test.

> http://www.pwsan.com/omap/testlogs/fix-omap-warnings-v3.21/20150206154619/
> 
> Non-critical; targeted for v3.20-rc1 or v3.21-rc1.
> 
> 
>  arch/arm/mach-omap1/pm.c | 42 +-
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
> index 34b4c0044961..d46d8a222fbb 100644
> --- a/arch/arm/mach-omap1/pm.c
> +++ b/arch/arm/mach-omap1/pm.c
> @@ -71,13 +71,7 @@ static unsigned int
> mpui7xx_sleep_save[MPUI7XX_SLEEP_SAVE_SIZE];
>  static unsigned int mpui1510_sleep_save[MPUI1510_SLEEP_SAVE_SIZE];
>  static unsigned int mpui1610_sleep_save[MPUI1610_SLEEP_SAVE_SIZE];
> 
> -#ifndef CONFIG_OMAP_32K_TIMER
> -
> -static unsigned short enable_dyn_sleep = 0;
> -
> -#else
> -
> -static unsigned short enable_dyn_sleep = 1;
> +static unsigned short enable_dyn_sleep;
> 
>  static ssize_t idle_show(struct kobject *kobj, struct kobj_attribute *attr,
>char *buf)
> @@ -90,8 +84,9 @@ static ssize_t idle_store(struct kobject *kobj, struct
> kobj_attribute *attr,
>  {
>   unsigned short value;
>   if (sscanf(buf, "%hu", ) != 1 ||
> - (value != 0 && value != 1)) {
> - printk(KERN_ERR "idle_sleep_store: Invalid value\n");
> + (value != 0 && value != 1) ||
> + (value != 0 && !IS_ENABLED(CONFIG_OMAP_32K_TIMER))) {
> + pr_err("idle_sleep_store: Invalid value\n");
>   return -EINVAL;
>   }
>   enable_dyn_sleep = value;
> @@ -101,7 +96,6 @@ static ssize_t idle_store(struct kobject *kobj,
> struct kobj_attribute *attr,
>  static struct kobj_attribute sleep_while_idle_attr =
>   __ATTR(sleep_while_idle, 0644, idle_show, idle_store);
> 
> -#endif
> 
>  static void (*omap_sram_suspend)(unsigned long r0, unsigned long r1) =
> NULL;
> 
> @@ -120,12 +114,11 @@ void omap1_pm_idle(void)
>   local_fiq_disable();
> 
>  #if defined(CONFIG_OMAP_MPU_TIMER) && !defined(CONFIG_OMAP_DM_TIMER)
> -#warning Enable 32kHz OS timer in order to allow sleep states in idle
>   use_idlect1 = use_idlect1 & ~(1 << 9);
> -#else
> +#endif
> +
>   if (enable_dyn_sleep)
>   do_sleep = 1;

Do we still need this do_sleep variable now? Looking at the code, I
think we could use enable_dyn_sleep directly.

> -#endif
> 
>  #ifdef CONFIG_OMAP_DM_TIMER
>   use_idlect1 = omap_dm_timer_modify_idlect_mask(use_idlect1);
> @@ -635,15 +628,24 @@ static const struct platform_suspend_ops
> omap_pm_ops = {
> 
>  static int __init omap_pm_init(void)
>  {
> -
> -#ifdef CONFIG_OMAP_32K_TIMER
> - 

Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-10 Thread Paul Walmsley
);
  -#else
  +#endif
  +
  if (enable_dyn_sleep)
  do_sleep = 1;
 
 Do we still need this do_sleep variable now? Looking at the code, I
 think we could use enable_dyn_sleep directly.

OK dropped it.

 
  -#endif
  
   #ifdef CONFIG_OMAP_DM_TIMER
  use_idlect1 = omap_dm_timer_modify_idlect_mask(use_idlect1);
  @@ -635,15 +628,24 @@ static const struct platform_suspend_ops
  omap_pm_ops = {
  
   static int __init omap_pm_init(void)
   {
  -
  -#ifdef CONFIG_OMAP_32K_TIMER
  -   int error;
  -#endif
  +   int error = 0;
  
  if (!cpu_class_is_omap1())
  return -ENODEV;
  
  -   printk(Power Management for TI OMAP.\n);
  +   pr_info(Power Management for TI OMAP.\n);
  +
  +   if (!IS_ENABLED(CONFIG_OMAP_32K_TIMER))
  +   pr_info(OMAP1 PM: sleep states in idle disabled due to no 
  32KiHz
  timer\n);
  +
  +   if (!IS_ENABLED(CONFIG_OMAP_DM_TIMER))
  +   pr_info(OMAP1 PM: sleep states in idle disabled due to no 
  DMTIMER
  support\n);
  +
  +   if (IS_ENABLED(CONFIG_OMAP_32K_TIMER) 
  +   IS_ENABLED(CONFIG_OMAP_DM_TIMER)  cpu_is_omap16xx()) {
 
 Do you need cpu_is_omap16xx() here? I believe DM_TIMER is only available
 on omap16xx onwards.

Makes sense, I'll drop the cpu_is* test.

  +   pr_info(OMAP1 PM: sleep states in idle enabled\n);
  +   enable_dyn_sleep = 1;
  +   }
  
  /*
   * We copy the assembler sleep/wakeup routines to SRAM.
  @@ -693,17 +695,15 @@ static int __init omap_pm_init(void)
  omap_pm_init_debugfs();
   #endif
  
  -#ifdef CONFIG_OMAP_32K_TIMER
  error = sysfs_create_file(power_kobj, sleep_while_idle_attr.attr);
  if (error)
  printk(KERN_ERR sysfs_create_file failed: %d\n, error);
  -#endif
  
  if (cpu_is_omap16xx()) {
  /* configure LOW_PWR pin */
  omap_cfg_reg(T20_1610_LOW_PWR);
  }
  
  -   return 0;
  +   return error;
   }
   __initcall(omap_pm_init);
  
 
 Otherwise ...
 
 Acked-by: Jon Hunter jgchun...@gmail.com
 
 Cheers Jon

Updated patch follows.


- Paul

From: Paul Walmsley p...@pwsan.com
Date: Fri, 6 Feb 2015 15:56:07 -0700
Subject: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

Building an OMAP1510-only Kconfig generates the following warnings:

arch/arm/mach-omap1/pm.c: In function ‘omap1_pm_idle’:
arch/arm/mach-omap1/pm.c:123:2: warning: #warning Enable 32kHz OS timer in 
order to allow sleep states in idle [-Wcpp]
 #warning Enable 32kHz OS timer in order to allow sleep states in idle
  ^
arch/arm/mach-omap1/pm.c: At top level:
arch/arm/mach-omap1/pm.c:76:23: warning: ‘enable_dyn_sleep’ defined but not 
used [-Wunused-variable]
 static unsigned short enable_dyn_sleep = 0;
   ^

These are not so easy to fix in an obviously correct fashion, since I
don't have these devices up and running in my testbed.  So, use
arch/arm/plat-omap/Kconfig and the existing pm.c source as a guide,
and posit that deep power saving states are only supported on OMAP16xx
chips with kernels built with both CONFIG_OMAP_DM_TIMER=y and
CONFIG_OMAP_32K_TIMER=y.

While here, clean up a few printk()s and unnecessary #ifdefs.

This second version of the patch incorporates several suggestions from
Jon Hunter jgchun...@gmail.com.

Signed-off-by: Paul Walmsley p...@pwsan.com
Cc: Jon Hunter jonath...@nvidia.com
Cc: Aaro Koskinen aaro.koski...@iki.fi
Cc: Tuukka Tikkanen tuukka.tikka...@linaro.org
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Tony Lindgren t...@atomide.com
Cc: Russell King li...@arm.linux.org.uk
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Jon Hunter jgchun...@gmail.com
---
 arch/arm/mach-omap1/pm.c | 51 
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
index 34b4c0044961..dd94567c3628 100644
--- a/arch/arm/mach-omap1/pm.c
+++ b/arch/arm/mach-omap1/pm.c
@@ -71,13 +71,7 @@ static unsigned int 
mpui7xx_sleep_save[MPUI7XX_SLEEP_SAVE_SIZE];
 static unsigned int mpui1510_sleep_save[MPUI1510_SLEEP_SAVE_SIZE];
 static unsigned int mpui1610_sleep_save[MPUI1610_SLEEP_SAVE_SIZE];
 
-#ifndef CONFIG_OMAP_32K_TIMER
-
-static unsigned short enable_dyn_sleep = 0;
-
-#else
-
-static unsigned short enable_dyn_sleep = 1;
+static unsigned short enable_dyn_sleep;
 
 static ssize_t idle_show(struct kobject *kobj, struct kobj_attribute *attr,
 char *buf)
@@ -90,8 +84,9 @@ static ssize_t idle_store(struct kobject *kobj, struct 
kobj_attribute *attr,
 {
unsigned short value;
if (sscanf(buf, %hu, value) != 1 ||
-   (value != 0  value != 1)) {
-   printk(KERN_ERR idle_sleep_store: Invalid value\n);
+   (value != 0  value != 1) ||
+   (value != 0  !IS_ENABLED(CONFIG_OMAP_32K_TIMER))) {
+   pr_err(idle_sleep_store: Invalid value\n);
return -EINVAL

Re: [PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-10 Thread Jon Hunter
Hi Paul,

On 07/02/2015 00:23, Paul Walmsley wrote:
 
 Building an OMAP1510-only Kconfig generates the following warnings:
 
 arch/arm/mach-omap1/pm.c: In function ‘omap1_pm_idle’:
 arch/arm/mach-omap1/pm.c:123:2: warning: #warning Enable 32kHz OS timer
 in order to allow sleep states in idle [-Wcpp]
  #warning Enable 32kHz OS timer in order to allow sleep states in idle
   ^
 arch/arm/mach-omap1/pm.c: At top level:
 arch/arm/mach-omap1/pm.c:76:23: warning: ‘enable_dyn_sleep’ defined but
 not used [-Wunused-variable]
  static unsigned short enable_dyn_sleep = 0;
^
 
 These are not so easy to fix in an obviously correct fashion, since I
 don't have these devices up and running in my testbed.  So, use
 arch/arm/plat-omap/Kconfig and the existing pm.c source as a guide,
 and posit that deep power saving states are only supported on OMAP16xx
 chips with kernels built with both CONFIG_OMAP_DM_TIMER=y and
 CONFIG_OMAP_32K_TIMER=y.
 
 While here, clean up a few printk()s and unnecessary #ifdefs.
 
 Signed-off-by: Paul Walmsley p...@pwsan.com
 Cc: Jon Hunter jonath...@nvidia.com
 Cc: Aaro Koskinen aaro.koski...@iki.fi
 Cc: Tuukka Tikkanen tuukka.tikka...@linaro.org
 Cc: Kevin Hilman khil...@deeprootsystems.com
 Cc: Tony Lindgren t...@atomide.com
 Cc: Russell King li...@arm.linux.org.uk
 Cc: linux-o...@vger.kernel.org
 Cc: linux-arm-ker...@lists.infradead.org
 Cc: linux-kernel@vger.kernel.org
 ---
 
 Hi folks, if anyone out there is still experimenting with OMAP1 PM, or has
 a copy of the OMAP1510 TRMs, could you please check this patch?  I'm
 unable to test it since I don't have any OMAP1 devices currently active
 in the testbed.  It at least compiles and deals with the build warnings:

You can find the omap5910 documents here [1]. The omap5910 is equivalent
to the omap1510. Unfortunately, there is not a single TRM for the
omap5910 but individual documents for each chapter in the original TRM.
Check out the OMAP5910 Dual-Core Processor Timer Reference Guide and
possibly the OMAP5910 Dual-Core Processor
Clock Generation and System Reset Management Reference Guide

The omap15xx/5910 did have a 32k timer but as you can see it appears it
was never supported by the kernel for this device (not sure why). I do
recall that there is some errata regarding the 32k timer, if you look at
the omap5910 errata document and search for 32k you should find it.

I no longer have access to omap15xx h/w to test. However, I do have
omap5912 if you want me to test.

 http://www.pwsan.com/omap/testlogs/fix-omap-warnings-v3.21/20150206154619/
 
 Non-critical; targeted for v3.20-rc1 or v3.21-rc1.
 
 
  arch/arm/mach-omap1/pm.c | 42 +-
  1 file changed, 21 insertions(+), 21 deletions(-)
 
 diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
 index 34b4c0044961..d46d8a222fbb 100644
 --- a/arch/arm/mach-omap1/pm.c
 +++ b/arch/arm/mach-omap1/pm.c
 @@ -71,13 +71,7 @@ static unsigned int
 mpui7xx_sleep_save[MPUI7XX_SLEEP_SAVE_SIZE];
  static unsigned int mpui1510_sleep_save[MPUI1510_SLEEP_SAVE_SIZE];
  static unsigned int mpui1610_sleep_save[MPUI1610_SLEEP_SAVE_SIZE];
 
 -#ifndef CONFIG_OMAP_32K_TIMER
 -
 -static unsigned short enable_dyn_sleep = 0;
 -
 -#else
 -
 -static unsigned short enable_dyn_sleep = 1;
 +static unsigned short enable_dyn_sleep;
 
  static ssize_t idle_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
 @@ -90,8 +84,9 @@ static ssize_t idle_store(struct kobject *kobj, struct
 kobj_attribute *attr,
  {
   unsigned short value;
   if (sscanf(buf, %hu, value) != 1 ||
 - (value != 0  value != 1)) {
 - printk(KERN_ERR idle_sleep_store: Invalid value\n);
 + (value != 0  value != 1) ||
 + (value != 0  !IS_ENABLED(CONFIG_OMAP_32K_TIMER))) {
 + pr_err(idle_sleep_store: Invalid value\n);
   return -EINVAL;
   }
   enable_dyn_sleep = value;
 @@ -101,7 +96,6 @@ static ssize_t idle_store(struct kobject *kobj,
 struct kobj_attribute *attr,
  static struct kobj_attribute sleep_while_idle_attr =
   __ATTR(sleep_while_idle, 0644, idle_show, idle_store);
 
 -#endif
 
  static void (*omap_sram_suspend)(unsigned long r0, unsigned long r1) =
 NULL;
 
 @@ -120,12 +114,11 @@ void omap1_pm_idle(void)
   local_fiq_disable();
 
  #if defined(CONFIG_OMAP_MPU_TIMER)  !defined(CONFIG_OMAP_DM_TIMER)
 -#warning Enable 32kHz OS timer in order to allow sleep states in idle
   use_idlect1 = use_idlect1  ~(1  9);
 -#else
 +#endif
 +
   if (enable_dyn_sleep)
   do_sleep = 1;

Do we still need this do_sleep variable now? Looking at the code, I
think we could use enable_dyn_sleep directly.

 -#endif
 
  #ifdef CONFIG_OMAP_DM_TIMER
   use_idlect1 = omap_dm_timer_modify_idlect_mask(use_idlect1);
 @@ -635,15 +628,24 @@ static const struct platform_suspend_ops
 omap_pm_ops = {
 
  static int __init omap_pm_init(void)
  {
 -
 -#ifdef 

[PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-06 Thread Paul Walmsley

Building an OMAP1510-only Kconfig generates the following warnings:

arch/arm/mach-omap1/pm.c: In function ‘omap1_pm_idle’:
arch/arm/mach-omap1/pm.c:123:2: warning: #warning Enable 32kHz OS timer in 
order to allow sleep states in idle [-Wcpp]
 #warning Enable 32kHz OS timer in order to allow sleep states in idle
  ^
arch/arm/mach-omap1/pm.c: At top level:
arch/arm/mach-omap1/pm.c:76:23: warning: ‘enable_dyn_sleep’ defined but not 
used [-Wunused-variable]
 static unsigned short enable_dyn_sleep = 0;
   ^

These are not so easy to fix in an obviously correct fashion, since I
don't have these devices up and running in my testbed.  So, use
arch/arm/plat-omap/Kconfig and the existing pm.c source as a guide,
and posit that deep power saving states are only supported on OMAP16xx
chips with kernels built with both CONFIG_OMAP_DM_TIMER=y and
CONFIG_OMAP_32K_TIMER=y.

While here, clean up a few printk()s and unnecessary #ifdefs.

Signed-off-by: Paul Walmsley 
Cc: Jon Hunter 
Cc: Aaro Koskinen 
Cc: Tuukka Tikkanen 
Cc: Kevin Hilman 
Cc: Tony Lindgren 
Cc: Russell King 
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---

Hi folks, if anyone out there is still experimenting with OMAP1 PM, or has 
a copy of the OMAP1510 TRMs, could you please check this patch?  I'm 
unable to test it since I don't have any OMAP1 devices currently active 
in the testbed.  It at least compiles and deals with the build warnings:

http://www.pwsan.com/omap/testlogs/fix-omap-warnings-v3.21/20150206154619/

Non-critical; targeted for v3.20-rc1 or v3.21-rc1.


 arch/arm/mach-omap1/pm.c | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
index 34b4c0044961..d46d8a222fbb 100644
--- a/arch/arm/mach-omap1/pm.c
+++ b/arch/arm/mach-omap1/pm.c
@@ -71,13 +71,7 @@ static unsigned int 
mpui7xx_sleep_save[MPUI7XX_SLEEP_SAVE_SIZE];
 static unsigned int mpui1510_sleep_save[MPUI1510_SLEEP_SAVE_SIZE];
 static unsigned int mpui1610_sleep_save[MPUI1610_SLEEP_SAVE_SIZE];
 
-#ifndef CONFIG_OMAP_32K_TIMER
-
-static unsigned short enable_dyn_sleep = 0;
-
-#else
-
-static unsigned short enable_dyn_sleep = 1;
+static unsigned short enable_dyn_sleep;
 
 static ssize_t idle_show(struct kobject *kobj, struct kobj_attribute *attr,
 char *buf)
@@ -90,8 +84,9 @@ static ssize_t idle_store(struct kobject *kobj, struct 
kobj_attribute *attr,
 {
unsigned short value;
if (sscanf(buf, "%hu", ) != 1 ||
-   (value != 0 && value != 1)) {
-   printk(KERN_ERR "idle_sleep_store: Invalid value\n");
+   (value != 0 && value != 1) ||
+   (value != 0 && !IS_ENABLED(CONFIG_OMAP_32K_TIMER))) {
+   pr_err("idle_sleep_store: Invalid value\n");
return -EINVAL;
}
enable_dyn_sleep = value;
@@ -101,7 +96,6 @@ static ssize_t idle_store(struct kobject *kobj, struct 
kobj_attribute *attr,
 static struct kobj_attribute sleep_while_idle_attr =
__ATTR(sleep_while_idle, 0644, idle_show, idle_store);
 
-#endif
 
 static void (*omap_sram_suspend)(unsigned long r0, unsigned long r1) = NULL;
 
@@ -120,12 +114,11 @@ void omap1_pm_idle(void)
local_fiq_disable();
 
 #if defined(CONFIG_OMAP_MPU_TIMER) && !defined(CONFIG_OMAP_DM_TIMER)
-#warning Enable 32kHz OS timer in order to allow sleep states in idle
use_idlect1 = use_idlect1 & ~(1 << 9);
-#else
+#endif
+
if (enable_dyn_sleep)
do_sleep = 1;
-#endif
 
 #ifdef CONFIG_OMAP_DM_TIMER
use_idlect1 = omap_dm_timer_modify_idlect_mask(use_idlect1);
@@ -635,15 +628,24 @@ static const struct platform_suspend_ops omap_pm_ops = {
 
 static int __init omap_pm_init(void)
 {
-
-#ifdef CONFIG_OMAP_32K_TIMER
-   int error;
-#endif
+   int error = 0;
 
if (!cpu_class_is_omap1())
return -ENODEV;
 
-   printk("Power Management for TI OMAP.\n");
+   pr_info("Power Management for TI OMAP.\n");
+
+   if (!IS_ENABLED(CONFIG_OMAP_32K_TIMER))
+   pr_info("OMAP1 PM: sleep states in idle disabled due to no 
32KiHz timer\n");
+
+   if (!IS_ENABLED(CONFIG_OMAP_DM_TIMER))
+   pr_info("OMAP1 PM: sleep states in idle disabled due to no 
DMTIMER support\n");
+
+   if (IS_ENABLED(CONFIG_OMAP_32K_TIMER) &&
+   IS_ENABLED(CONFIG_OMAP_DM_TIMER) && cpu_is_omap16xx()) {
+   pr_info("OMAP1 PM: sleep states in idle enabled\n");
+   enable_dyn_sleep = 1;
+   }
 
/*
 * We copy the assembler sleep/wakeup routines to SRAM.
@@ -693,17 +695,15 @@ static int __init omap_pm_init(void)
omap_pm_init_debugfs();
 #endif
 
-#ifdef CONFIG_OMAP_32K_TIMER
error = sysfs_create_file(power_kobj, _while_idle_attr.attr);
if (error)
printk(KERN_ERR "sysfs_create_file failed: %d\n", error);

[PATCH] ARM: OMAP1: PM: fix some build warnings on 1510-only Kconfigs

2015-02-06 Thread Paul Walmsley

Building an OMAP1510-only Kconfig generates the following warnings:

arch/arm/mach-omap1/pm.c: In function ‘omap1_pm_idle’:
arch/arm/mach-omap1/pm.c:123:2: warning: #warning Enable 32kHz OS timer in 
order to allow sleep states in idle [-Wcpp]
 #warning Enable 32kHz OS timer in order to allow sleep states in idle
  ^
arch/arm/mach-omap1/pm.c: At top level:
arch/arm/mach-omap1/pm.c:76:23: warning: ‘enable_dyn_sleep’ defined but not 
used [-Wunused-variable]
 static unsigned short enable_dyn_sleep = 0;
   ^

These are not so easy to fix in an obviously correct fashion, since I
don't have these devices up and running in my testbed.  So, use
arch/arm/plat-omap/Kconfig and the existing pm.c source as a guide,
and posit that deep power saving states are only supported on OMAP16xx
chips with kernels built with both CONFIG_OMAP_DM_TIMER=y and
CONFIG_OMAP_32K_TIMER=y.

While here, clean up a few printk()s and unnecessary #ifdefs.

Signed-off-by: Paul Walmsley p...@pwsan.com
Cc: Jon Hunter jonath...@nvidia.com
Cc: Aaro Koskinen aaro.koski...@iki.fi
Cc: Tuukka Tikkanen tuukka.tikka...@linaro.org
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Tony Lindgren t...@atomide.com
Cc: Russell King li...@arm.linux.org.uk
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---

Hi folks, if anyone out there is still experimenting with OMAP1 PM, or has 
a copy of the OMAP1510 TRMs, could you please check this patch?  I'm 
unable to test it since I don't have any OMAP1 devices currently active 
in the testbed.  It at least compiles and deals with the build warnings:

http://www.pwsan.com/omap/testlogs/fix-omap-warnings-v3.21/20150206154619/

Non-critical; targeted for v3.20-rc1 or v3.21-rc1.


 arch/arm/mach-omap1/pm.c | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
index 34b4c0044961..d46d8a222fbb 100644
--- a/arch/arm/mach-omap1/pm.c
+++ b/arch/arm/mach-omap1/pm.c
@@ -71,13 +71,7 @@ static unsigned int 
mpui7xx_sleep_save[MPUI7XX_SLEEP_SAVE_SIZE];
 static unsigned int mpui1510_sleep_save[MPUI1510_SLEEP_SAVE_SIZE];
 static unsigned int mpui1610_sleep_save[MPUI1610_SLEEP_SAVE_SIZE];
 
-#ifndef CONFIG_OMAP_32K_TIMER
-
-static unsigned short enable_dyn_sleep = 0;
-
-#else
-
-static unsigned short enable_dyn_sleep = 1;
+static unsigned short enable_dyn_sleep;
 
 static ssize_t idle_show(struct kobject *kobj, struct kobj_attribute *attr,
 char *buf)
@@ -90,8 +84,9 @@ static ssize_t idle_store(struct kobject *kobj, struct 
kobj_attribute *attr,
 {
unsigned short value;
if (sscanf(buf, %hu, value) != 1 ||
-   (value != 0  value != 1)) {
-   printk(KERN_ERR idle_sleep_store: Invalid value\n);
+   (value != 0  value != 1) ||
+   (value != 0  !IS_ENABLED(CONFIG_OMAP_32K_TIMER))) {
+   pr_err(idle_sleep_store: Invalid value\n);
return -EINVAL;
}
enable_dyn_sleep = value;
@@ -101,7 +96,6 @@ static ssize_t idle_store(struct kobject *kobj, struct 
kobj_attribute *attr,
 static struct kobj_attribute sleep_while_idle_attr =
__ATTR(sleep_while_idle, 0644, idle_show, idle_store);
 
-#endif
 
 static void (*omap_sram_suspend)(unsigned long r0, unsigned long r1) = NULL;
 
@@ -120,12 +114,11 @@ void omap1_pm_idle(void)
local_fiq_disable();
 
 #if defined(CONFIG_OMAP_MPU_TIMER)  !defined(CONFIG_OMAP_DM_TIMER)
-#warning Enable 32kHz OS timer in order to allow sleep states in idle
use_idlect1 = use_idlect1  ~(1  9);
-#else
+#endif
+
if (enable_dyn_sleep)
do_sleep = 1;
-#endif
 
 #ifdef CONFIG_OMAP_DM_TIMER
use_idlect1 = omap_dm_timer_modify_idlect_mask(use_idlect1);
@@ -635,15 +628,24 @@ static const struct platform_suspend_ops omap_pm_ops = {
 
 static int __init omap_pm_init(void)
 {
-
-#ifdef CONFIG_OMAP_32K_TIMER
-   int error;
-#endif
+   int error = 0;
 
if (!cpu_class_is_omap1())
return -ENODEV;
 
-   printk(Power Management for TI OMAP.\n);
+   pr_info(Power Management for TI OMAP.\n);
+
+   if (!IS_ENABLED(CONFIG_OMAP_32K_TIMER))
+   pr_info(OMAP1 PM: sleep states in idle disabled due to no 
32KiHz timer\n);
+
+   if (!IS_ENABLED(CONFIG_OMAP_DM_TIMER))
+   pr_info(OMAP1 PM: sleep states in idle disabled due to no 
DMTIMER support\n);
+
+   if (IS_ENABLED(CONFIG_OMAP_32K_TIMER) 
+   IS_ENABLED(CONFIG_OMAP_DM_TIMER)  cpu_is_omap16xx()) {
+   pr_info(OMAP1 PM: sleep states in idle enabled\n);
+   enable_dyn_sleep = 1;
+   }
 
/*
 * We copy the assembler sleep/wakeup routines to SRAM.
@@ -693,17 +695,15 @@ static int __init omap_pm_init(void)
omap_pm_init_debugfs();
 #endif
 
-#ifdef CONFIG_OMAP_32K_TIMER
error = sysfs_create_file(power_kobj,