Linux Power Management development
 help / color / mirror / Atom feed
* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Pavel Vasilyev @ 2012-08-06 16:00 UTC (permalink / raw)
  To: Alan Stern
  Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
	Len Brown
In-Reply-To: <Pine.LNX.4.44L0.1208061028160.1649-100000@iolanthe.rowland.org>

06.08.2012 18:36, Alan Stern пишет:
> On Mon, 6 Aug 2012, Pavel Machek wrote:
>
>> On Thu 2012-07-26 21:39:38, Len Brown wrote:
>>> ...both give the number of chars in the string
>>> without the '\0', as strncmp() wants,
>>> but sizeof() is compile-time.
>>
>> What about introducing something like streq() to do this
>> automatically? This is ugly....
>>
>> #define streq(a, b) ... if (_buildin_constant(b)) ...
>>
>> ?
>>
>>> -	if (!strncmp(val, "enable", strlen("enable"))) {
>>> +	if (!strncmp(val, "enable", sizeof("enable") - 1)) {
>
> While you're at it, there's no point using strncmp when you know the
> length of one of the strings beforehand.  Just use memcmp, and don't
> subtract 1 from the sizeof value.

http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux

:)




-- 

                                                          Pavel.

^ permalink raw reply

* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Alan Stern @ 2012-08-06 16:28 UTC (permalink / raw)
  To: Pavel Vasilyev; +Cc: Len Brown, linux-kernel, linux-acpi, linux-pm
In-Reply-To: <501FEA29.1030400@pavlinux.ru>

On Mon, 6 Aug 2012, Pavel Vasilyev wrote:

> 06.08.2012 18:36, Alan Stern пишет:
> > On Mon, 6 Aug 2012, Pavel Machek wrote:
> >
> >> On Thu 2012-07-26 21:39:38, Len Brown wrote:
> >>> ...both give the number of chars in the string
> >>> without the '\0', as strncmp() wants,
> >>> but sizeof() is compile-time.
> >>
> >> What about introducing something like streq() to do this
> >> automatically? This is ugly....
> >>
> >> #define streq(a, b) ... if (_buildin_constant(b)) ...
> >>
> >> ?
> >>
> >>> -	if (!strncmp(val, "enable", strlen("enable"))) {
> >>> +	if (!strncmp(val, "enable", sizeof("enable") - 1)) {
> >
> > While you're at it, there's no point using strncmp when you know the
> > length of one of the strings beforehand.  Just use memcmp, and don't
> > subtract 1 from the sizeof value.
> 
> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux

Interestingly, many (all?) of the changes in that patch are wrong 
because they don't try to match the terminating '\0'.  As a result, 
they will match against extensions of the target string as well as the 
target string itself.

Alan Stern

^ permalink raw reply

* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Pavel Vasilyev @ 2012-08-06 18:47 UTC (permalink / raw)
  To: Alan Stern
  Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
	Len Brown
In-Reply-To: <Pine.LNX.4.44L0.1208061226460.1649-100000@iolanthe.rowland.org>

06.08.2012 20:28, Alan Stern пишет:
> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
>
>> 06.08.2012 18:36, Alan Stern пишет:
>>> On Mon, 6 Aug 2012, Pavel Machek wrote:
>>>
>>>> On Thu 2012-07-26 21:39:38, Len Brown wrote:
>>>>> ...both give the number of chars in the string
>>>>> without the '\0', as strncmp() wants,
>>>>> but sizeof() is compile-time.
>>>>
>>>> What about introducing something like streq() to do this
>>>> automatically? This is ugly....
>>>>
>>>> #define streq(a, b) ... if (_buildin_constant(b)) ...
>>>>
>>>> ?
>>>>
>>>>> -	if (!strncmp(val, "enable", strlen("enable"))) {
>>>>> +	if (!strncmp(val, "enable", sizeof("enable") - 1)) {
>>>
>>> While you're at it, there's no point using strncmp when you know the
>>> length of one of the strings beforehand.  Just use memcmp, and don't
>>> subtract 1 from the sizeof value.
>>
>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
>
> Interestingly, many (all?) of the changes in that patch are wrong
> because they don't try to match the terminating '\0'.  As a result,
> they will match against extensions of the target string as well as the
> target string itself.
>

strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
memcmp compare N bytes  - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651

-- 

                                                          Pavel.

^ permalink raw reply

* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Alan Stern @ 2012-08-06 19:59 UTC (permalink / raw)
  To: Pavel Vasilyev; +Cc: Len Brown, linux-kernel, linux-acpi, linux-pm
In-Reply-To: <50201156.30704@pavlinux.ru>

On Mon, 6 Aug 2012, Pavel Vasilyev wrote:

> >> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
> >
> > Interestingly, many (all?) of the changes in that patch are wrong
> > because they don't try to match the terminating '\0'.  As a result,
> > they will match against extensions of the target string as well as the
> > target string itself.
> >
> 
> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
> memcmp compare N bytes  - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651

Yes.  So if s contains "abcde" then

	memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
	memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.

Alan Stern

^ permalink raw reply

* RE: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Daniel Taylor @ 2012-08-06 22:57 UTC (permalink / raw)
  To: 'Alan Stern', Pavel Vasilyev
  Cc: Pavel Machek, Len Brown, linux-acpi@vger.kernel.org,
	linux-pm@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	Len Brown
In-Reply-To: <Pine.LNX.4.44L0.1208061556010.1649-100000@iolanthe.rowland.org>

Silly question:  when did sizeof("string") get changed to be anything
other than the size of the pointer ("string" is, after all, an array
of characters)? 

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org 
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Alan Stern
> Sent: Monday, August 06, 2012 1:00 PM
> To: Pavel Vasilyev
> Cc: Pavel Machek; Len Brown; linux-acpi@vger.kernel.org; 
> linux-pm@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; Len Brown
> Subject: Re: [linux-pm] [PATCH] ACPI: replace 
> strlen("string") with sizeof("string") -1
> 
> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
> 
> > >> 
> http://www.gossamer-threads.com/lists/engine?do=post_attachmen
> t;postatt_id=41157;list=linux
> > >
> > > Interestingly, many (all?) of the changes in that patch are wrong
> > > because they don't try to match the terminating '\0'.  As 
> a result,
> > > they will match against extensions of the target string 
> as well as the
> > > target string itself.
> > >
> > 
> > strNcmp compare N bytes - 
> http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
> > memcmp compare N bytes  - 
> http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
> 
> Yes.  So if s contains "abcde" then
> 
> 	memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both 
> return 0, and
> 	memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
> 
> Alan Stern
> 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

^ permalink raw reply

* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Pavel Vasilyev @ 2012-08-07  1:07 UTC (permalink / raw)
  To: Alan Stern
  Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
	Len Brown
In-Reply-To: <Pine.LNX.4.44L0.1208061556010.1649-100000@iolanthe.rowland.org>

06.08.2012 23:59, Alan Stern пишет:
> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
>
>>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
>>>
>>> Interestingly, many (all?) of the changes in that patch are wrong
>>> because they don't try to match the terminating '\0'.  As a result,
>>> they will match against extensions of the target string as well as the
>>> target string itself.
>>>
>>
>> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
>> memcmp compare N bytes  - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
>
> Yes.  So if s contains "abcde" then
>
> 	memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
> 	memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.

No matter what is contained in *s, "abcde" or "abcxxx",
are important first N bytes. The second example, you see,
a little bit stupid, and devoid of logic. :)


-- 

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

^ permalink raw reply

* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Bernd Petrovitsch @ 2012-08-07 13:19 UTC (permalink / raw)
  To: Daniel Taylor
  Cc: Len Brown, linux-kernel@vger.kernel.org,
	linux-acpi@vger.kernel.org, Pavel Vasilyev,
	linux-pm@lists.linux-foundation.org
In-Reply-To: <5A9BC72FD5CEC94EA024CED8E31D701A2FD84FC0@wdscexmb03.sc.wdc.com>

On Mon, 2012-08-06 at 22:57 +0000, Daniel Taylor wrote:
> Silly question:  when did sizeof("string") get changed to be anything
> other than the size of the pointer ("string" is, after all, an array
> of characters)? 

It is since K&R times that way.
If you do not know the difference between a pointer and an array (and
these are vastly different), go learn something new about C.

	Bernd
-- 
Bernd Petrovitsch                  Email : bernd@petrovitsch.priv.at
                     LUGA : http://www.luga.at

^ permalink raw reply

* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Alan Stern @ 2012-08-07 17:24 UTC (permalink / raw)
  To: Pavel Vasilyev; +Cc: Len Brown, linux-kernel, linux-acpi, linux-pm
In-Reply-To: <50206A3A.8020706@pavlinux.ru>

On Tue, 7 Aug 2012, Pavel Vasilyev wrote:

> 06.08.2012 23:59, Alan Stern пишет:
> > On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
> >
> >>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
> >>>
> >>> Interestingly, many (all?) of the changes in that patch are wrong
> >>> because they don't try to match the terminating '\0'.  As a result,
> >>> they will match against extensions of the target string as well as the
> >>> target string itself.
> >>>
> >>
> >> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
> >> memcmp compare N bytes  - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
> >
> > Yes.  So if s contains "abcde" then
> >
> > 	memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
> > 	memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
> 
> No matter what is contained in *s, "abcde" or "abcxxx",
> are important first N bytes. The second example, you see,
> a little bit stupid, and devoid of logic. :)

Maybe yes, maybe no.  It all depends on what you want.

For example, if you're looking for "on" or "off", what should you do
when the user writes "onoff"?  You could accept it as meaning the same
as "on", but if you were being careful then you would want to reject it
as a meaningless value.

Alan Stern

^ permalink raw reply

* Re: [RESEND PATCH] conservative: Initialise the cpu field during governor start
From: Rafael J. Wysocki @ 2012-08-07 22:22 UTC (permalink / raw)
  To: Amit Daniel Kachhap; +Cc: cpufreq, amit.kachhap, linux-kernel, linux-pm
In-Reply-To: <1343121972-9130-1-git-send-email-amit.daniel@samsung.com>

On Tuesday, July 24, 2012, Amit Daniel Kachhap wrote:
> This change initialises the cpu id field of cs_cpu_dbs_info structure in
> conservative governor and keep this consistent with other governors.
> Similar initialisation is present in ondemand governor.
> 
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>

Applied to the linux-next branch of the linux-pm.git tree as v3.7 material.

Thanks,
Rafael


> ---
>  drivers/cpufreq/cpufreq_conservative.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index 235a340..a1563d7 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -504,6 +504,7 @@ static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
>  				j_dbs_info->prev_cpu_nice =
>  						kcpustat_cpu(j).cpustat[CPUTIME_NICE];
>  		}
> +		this_dbs_info->cpu = cpu;
>  		this_dbs_info->down_skip = 0;
>  		this_dbs_info->requested_freq = policy->cur;
>  
> 


^ permalink raw reply

* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Pavel Vasilyev @ 2012-08-07 23:23 UTC (permalink / raw)
  To: Alan Stern
  Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
	Len Brown
In-Reply-To: <Pine.LNX.4.44L0.1208071322070.2400-100000@iolanthe.rowland.org>

07.08.2012 21:24, Alan Stern пишет:
> On Tue, 7 Aug 2012, Pavel Vasilyev wrote:
>
>> 06.08.2012 23:59, Alan Stern пишет:
>>> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
>>>
>>>>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
>>>>>
>>>>> Interestingly, many (all?) of the changes in that patch are wrong
>>>>> because they don't try to match the terminating '\0'.  As a result,
>>>>> they will match against extensions of the target string as well as the
>>>>> target string itself.
>>>>>
>>>>
>>>> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
>>>> memcmp compare N bytes  - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
>>>
>>> Yes.  So if s contains "abcde" then
>>>
>>> 	memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
>>> 	memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
>>
>> No matter what is contained in *s, "abcde" or "abcxxx",
>> are important first N bytes. The second example, you see,
>> a little bit stupid, and devoid of logic. :)
>
> Maybe yes, maybe no.  It all depends on what you want.
>
> For example, if you're looking for "on" or "off", what should you do
> when the user writes "onoff"?  You could accept it as meaning the same
> as "on", but if you were being careful then you would want to reject it
> as a meaningless value.


The users should't be allowed to think!
There is "on" - the size of 2 bytes, or "off" - 3 bytes,
other variations - user error.

We do not create a kernel with artificial intelligence? ;)

-- 
                                                          Pavel.

^ permalink raw reply

* Re: [linux-pm] [PATCH 4/4] PM: cleanup: stop mandating that platforms export (pm_idle)()
From: Kevin Hilman @ 2012-08-07 23:26 UTC (permalink / raw)
  To: Len Brown
  Cc: x86, linux-pm, linux-kernel, Jonas Bonn, Len Brown, #, Tony Luck,
	Mike Frysinger, Michal Simek, David Howells, Mikael Starvik,
	Russell King, David S. Miller, linux-arm-kernel
In-Reply-To: <0753050fb227e5a19f0df303d1e9bf74534aabcd.1343428708.git.len.brown@intel.com>

+ linux-arm-kernel

Len Brown <lenb@kernel.org> writes:

> From: Len Brown <len.brown@intel.com>
>
> (pm_idle)() was originally used on x86 to vector bewteen
> ACPI and APM.  With the advent of CPU_IDLE, that reason
> for pm_idle to exist vanished.
>
> But x86 APM still scribbled on pm_idle from a module,
> so pm_idle didn't go away.  Worse, it was declared in pm.h,
> and so it spread to other architectures as dead code.
>
> But today, APM no longer scribbles on pm_idle, so
> x86 no longer requires pm_idle to be visible to modules,
> or global at all.
>
> Here we remove pm_idle from pm.h, to stop the mandate
> that all architectures define it.
>
> This deletes dead code from most architectures,
> while some continue using their own internal pm_idle.
>
> At the end of the day, pm_idle...
> becomes static in sh, was global
> becomes static in x86, was EXPORT_SYMBOL
> becomes as global in sparc, was EXPORT_SYMBOL
> continues as static in m32r (no pm.h use there)
> and is deleted from all other places in the kernel.
>
> Signed-off-by: Len Brown <len.brown@intel.com>
> Cc: x86@kernel.org # x86
> Cc: Mike Frysinger <vapier@gentoo.org> # blackfin
> Cc: Jonas Bonn <jonas@southpole.se> # openrisc
> Cc: Tony Luck <tony.luck@intel.com> # ia64
> Cc: David Howells <dhowells@redhat.com> # mn10300
> Cc: Mikael Starvik <starvik@axis.com> # cris
> Cc: Michal Simek <monstr@monstr.eu> # microblaze
> Cc: Paul Mundt <lethal@linux-sh.org> # sh
> Cc: David S. Miller <davem@davemloft.net> # sparc
> Cc: Russell King <linux@arm.linux.org.uk> # ARM
> Cc: Rafael J. Wysocki <rjw@sisk.pl> # PM
> ---
>  arch/arm/kernel/process.c         | 12 +++---------

Just looking at the ARM changes, it looks good to me.  I tested it with
and without CPUidle on my ARM-based TI/OMAP platforms (3430/n900,
4430/Panda)

So for the arch/arm changes:

Reviewed-by: Kevin Hilman <khilman@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>

Kevin

^ permalink raw reply

* RE: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Daniel Taylor @ 2012-08-08  0:59 UTC (permalink / raw)
  To: 'Bernd Petrovitsch'
  Cc: 'Alan Stern', Pavel Vasilyev, Pavel Machek, Len Brown,
	linux-acpi@vger.kernel.org, linux-pm@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, Len Brown
In-Reply-To: <1344345590.3975.1.camel@thorin>

Said it was a silly question.

It's funny.

I've been using "0123456789abcdef"[index] for a long time, so I "know"
that "string" is a array of char, but it never occurred to me that
"string" would work in sizeof() the same way as

char string[] = { '0', '1', '2', '3', '4', '5', '6', '7',
                  '8', '9', 'a', 'b', 'c', 'd', 'e', 'f', '\0' };

int stringlength = sizeof(string);

Learned something.

Thanks,

Dan
 

> -----Original Message-----
> From: Bernd Petrovitsch [mailto:bernd@petrovitsch.priv.at] 
> Sent: Tuesday, August 07, 2012 6:20 AM
> To: Daniel Taylor
> Cc: 'Alan Stern'; Pavel Vasilyev; Pavel Machek; Len Brown; 
> linux-acpi@vger.kernel.org; 
> linux-pm@lists.linux-foundation.org; 
> linux-kernel@vger.kernel.org; Len Brown
> Subject: RE: [linux-pm] [PATCH] ACPI: replace 
> strlen("string") with sizeof("string") -1
> 
> On Mon, 2012-08-06 at 22:57 +0000, Daniel Taylor wrote:
> > Silly question:  when did sizeof("string") get changed to 
> be anything
> > other than the size of the pointer ("string" is, after all, an array
> > of characters)? 
> 
> It is since K&R times that way.
> If you do not know the difference between a pointer and an array (and
> these are vastly different), go learn something new about C.
> 
> 	Bernd
> -- 
> Bernd Petrovitsch                  Email : bernd@petrovitsch.priv.at
>                      LUGA : http://www.luga.at
> 
> 

^ permalink raw reply

* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
From: Alan Stern @ 2012-08-08  1:27 UTC (permalink / raw)
  To: Pavel Vasilyev
  Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
	Len Brown
In-Reply-To: <5021A372.8030409@pavlinux.ru>

On Wed, 8 Aug 2012, Pavel Vasilyev wrote:

> >>> Yes.  So if s contains "abcde" then
> >>>
> >>> 	memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
> >>> 	memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
> >>
> >> No matter what is contained in *s, "abcde" or "abcxxx",
> >> are important first N bytes. The second example, you see,
> >> a little bit stupid, and devoid of logic. :)
> >
> > Maybe yes, maybe no.  It all depends on what you want.
> >
> > For example, if you're looking for "on" or "off", what should you do
> > when the user writes "onoff"?  You could accept it as meaning the same
> > as "on", but if you were being careful then you would want to reject it
> > as a meaningless value.
> 
> 
> The users should't be allowed to think!
> There is "on" - the size of 2 bytes, or "off" - 3 bytes,
> other variations - user error.
> 
> We do not create a kernel with artificial intelligence? ;)

Let me rephrase the previous statement, as it appears you did not 
understand what I meant.

If the kernel is testing for "on" or "off", what should it do when the
user writes "onoff"?  The kernel could accept this as meaning the same
as "on", but if the kernel was being careful then it should reject
"onoff" as a meaningless value.  A 2-byte comparison for "on" would
accept "onoff" whereas a 3-byte comparison would not.

Alan Stern


^ permalink raw reply

* Re: [PATCH v3 2/7] mfd: omap: control: core system control driver
From: Tony Lindgren @ 2012-08-08 13:48 UTC (permalink / raw)
  To: Konstantin Baydarov
  Cc: amit.kucheria, kishon, balbi, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <4FEB4B46.9020904@dev.rtsoft.ru>

* Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120627 11:09]:
> +
> +/* TODO: Add helpers for 16bit and byte access */
> +#ifdef CONFIG_MFD_OMAP_CONTROL
> +u32 omap_control_status_read(u16 offset);
> +#else
> +static inline u32 omap_control_status_read(u16 offset)
> +{
> +	return 0;
> +}
> +#endif

This should be an exported function instead. And it should
not need the offset as a parameter as the SCM core should know
where to find it's control_status register.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH v3 3/7] OMAP2+: use control module mfd driver in omap_type
From: Tony Lindgren @ 2012-08-08 13:50 UTC (permalink / raw)
  To: Konstantin Baydarov
  Cc: amit.kucheria, kishon, balbi, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <4FEB4B4A.1040204@dev.rtsoft.ru>

* Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120627 11:09]:
> --- a/arch/arm/mach-omap2/id.c
> +++ b/arch/arm/mach-omap2/id.c
> @@ -18,6 +18,7 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
> +#include <linux/mfd/omap_control.h>
>  
>  #include <asm/cputype.h>
>  
> @@ -43,13 +44,13 @@ int omap_type(void)
>  	u32 val = 0;
>  
>  	if (cpu_is_omap24xx()) {
> -		val = omap_ctrl_readl(OMAP24XX_CONTROL_STATUS);
> +		val = omap_control_status_read(OMAP24XX_CONTROL_STATUS);
>  	} else if (cpu_is_am33xx()) {
> -		val = omap_ctrl_readl(AM33XX_CONTROL_STATUS);
> +		val = omap_control_status_read(AM33XX_CONTROL_STATUS);
>  	} else if (cpu_is_omap34xx()) {
> -		val = omap_ctrl_readl(OMAP343X_CONTROL_STATUS);
> +		val = omap_control_status_read(OMAP343X_CONTROL_STATUS);
>  	} else if (cpu_is_omap44xx()) {
> -		val = omap_ctrl_readl(OMAP4_CTRL_MODULE_CORE_STATUS);
> +		val = omap_control_status_read(OMAP4_CTRL_MODULE_CORE_STATUS);
>  	} else {
>  		pr_err("Cannot detect omap type!\n");
>  		goto out;

I think I already commented on this.. This needs to become just:

val = omap_control_status_read();

with no need to pass the status register as a parameter.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH v3 5/7] ARM: OMAP4+: thermal: introduce bandgap temperature sensor
From: Tony Lindgren @ 2012-08-08 13:55 UTC (permalink / raw)
  To: Konstantin Baydarov
  Cc: amit.kucheria, kishon, balbi, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <4FEB4B4F.9040709@dev.rtsoft.ru>

* Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120627 11:09]:
> +
> +module_platform_driver(omap_bandgap_sensor_driver);
> +early_platform_init("early_omap_temperature", &omap_bandgap_sensor_driver);

Why does this need to be early_platform_init?

Tony

^ permalink raw reply

* Re: [PATCH v4 2/4] mfd: omap: control: core system control driver
From: Tony Lindgren @ 2012-08-08 14:05 UTC (permalink / raw)
  To: Konstantin Baydarov
  Cc: amit.kucheria, kishon, balbi, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <500FD2EC.7060208@dev.rtsoft.ru>

* Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120725 04:10]:
> +
> +u32 omap_control_status_read(void)
> +{
> +	return __raw_readl(omap_control_base);
> +}

Ah OK it's changed here.. Sorry I was looking at the older
version.

> +void __init of_omap_control_init(const struct of_device_id *matches)
> +{
> +	struct device_node *np;
> +	struct property *pp = 0;
> +
> +	for_each_matching_node(np, matches) {
> +		pp = of_find_property(np, "reg", NULL);
> +		if(pp) {
> +			omap_control_phys_base = (unsigned long)be32_to_cpup(pp->value);
> +			omap_control_mapsize = (size_t)be32_to_cpup( (void*)((char*)pp->value + 4) );
> +			/*
> +			 * Map control module register CONTROL_STATUS register:
> +			 * omap24xx - OMAP24XX_CONTROL_STATUS
> +			 * am33xx   - AM33XX_CONTROL_STATUS
> +			 * omap34xx - OMAP343X_CONTROL_STATUS
> +			 * omap44xx - OMAP4_CTRL_MODULE_CORE_STATUS
> +			 * omap54xx - OMAP5XXX_CONTROL_STATUS
> +			 */
> +			omap_control_base = ioremap(omap_control_phys_base, omap_control_mapsize);
> +		}
> +	}
> +}

You should probably add a function for setting omap_control_base
separately from *set_globals* in arch/arm/mach-omap2/common.c.
That way it's initialized early for id.c, and you can initialize
everything else later as regular device drivers.

FYI, we want to initialize as much as possible late so we have
proper debugging console set up in case things go wrong.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH v4 2/4] mfd: omap: control: core system control driver
From: Tony Lindgren @ 2012-08-08 14:10 UTC (permalink / raw)
  To: Konstantin Baydarov
  Cc: balbi, kishon, amit.kucheria, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <20120808140507.GF11011@atomide.com>

* Tony Lindgren <tony@atomide.com> [120808 07:05]:
> * Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120725 04:10]:
> > +
> > +u32 omap_control_status_read(void)
> > +{
> > +	return __raw_readl(omap_control_base);
> > +}
> 
> Ah OK it's changed here.. Sorry I was looking at the older
> version.
> 
> > +void __init of_omap_control_init(const struct of_device_id *matches)
> > +{
> > +	struct device_node *np;
> > +	struct property *pp = 0;
> > +
> > +	for_each_matching_node(np, matches) {
> > +		pp = of_find_property(np, "reg", NULL);
> > +		if(pp) {
> > +			omap_control_phys_base = (unsigned long)be32_to_cpup(pp->value);
> > +			omap_control_mapsize = (size_t)be32_to_cpup( (void*)((char*)pp->value + 4) );
> > +			/*
> > +			 * Map control module register CONTROL_STATUS register:
> > +			 * omap24xx - OMAP24XX_CONTROL_STATUS
> > +			 * am33xx   - AM33XX_CONTROL_STATUS
> > +			 * omap34xx - OMAP343X_CONTROL_STATUS
> > +			 * omap44xx - OMAP4_CTRL_MODULE_CORE_STATUS
> > +			 * omap54xx - OMAP5XXX_CONTROL_STATUS
> > +			 */
> > +			omap_control_base = ioremap(omap_control_phys_base, omap_control_mapsize);
> > +		}
> > +	}
> > +}
> 
> You should probably add a function for setting omap_control_base
> separately from *set_globals* in arch/arm/mach-omap2/common.c.
> That way it's initialized early for id.c, and you can initialize
> everything else later as regular device drivers.
> 
> FYI, we want to initialize as much as possible late so we have
> proper debugging console set up in case things go wrong.

Hmm it seems that omap_control_base is now only initialized for DT boot
case? This will break booting on almost all systems..

Tony

^ permalink raw reply

* Re: [linux-pm] [PATCH v4 2/4] mfd: omap: control: core system control driver
From: Tony Lindgren @ 2012-08-08 14:39 UTC (permalink / raw)
  To: Konstantin Baydarov
  Cc: balbi, kishon, amit.kucheria, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <20120808141051.GG11011@atomide.com>

* Tony Lindgren <tony@atomide.com> [120808 07:11]:
> * Tony Lindgren <tony@atomide.com> [120808 07:05]:
> > * Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120725 04:10]:
> > > +
> > > +u32 omap_control_status_read(void)
> > > +{
> > > +	return __raw_readl(omap_control_base);
> > > +}
> > 
> > Ah OK it's changed here.. Sorry I was looking at the older
> > version.
> > 
> > > +void __init of_omap_control_init(const struct of_device_id *matches)
> > > +{
> > > +	struct device_node *np;
> > > +	struct property *pp = 0;
> > > +
> > > +	for_each_matching_node(np, matches) {
> > > +		pp = of_find_property(np, "reg", NULL);
> > > +		if(pp) {
> > > +			omap_control_phys_base = (unsigned long)be32_to_cpup(pp->value);
> > > +			omap_control_mapsize = (size_t)be32_to_cpup( (void*)((char*)pp->value + 4) );
> > > +			/*
> > > +			 * Map control module register CONTROL_STATUS register:
> > > +			 * omap24xx - OMAP24XX_CONTROL_STATUS
> > > +			 * am33xx   - AM33XX_CONTROL_STATUS
> > > +			 * omap34xx - OMAP343X_CONTROL_STATUS
> > > +			 * omap44xx - OMAP4_CTRL_MODULE_CORE_STATUS
> > > +			 * omap54xx - OMAP5XXX_CONTROL_STATUS
> > > +			 */
> > > +			omap_control_base = ioremap(omap_control_phys_base, omap_control_mapsize);
> > > +		}
> > > +	}
> > > +}
> > 
> > You should probably add a function for setting omap_control_base
> > separately from *set_globals* in arch/arm/mach-omap2/common.c.
> > That way it's initialized early for id.c, and you can initialize
> > everything else later as regular device drivers.
> > 
> > FYI, we want to initialize as much as possible late so we have
> > proper debugging console set up in case things go wrong.
> 
> Hmm it seems that omap_control_base is now only initialized for DT boot
> case? This will break booting on almost all systems..

Looking at things more, looks like omap_type() is getting called
early from timer.c. So maybe just let set_globals initialize
omap_control_base and let omap_type() read that directly. That way
the SCM core driver can behave like a regular device driver and it
does not need to be initialized early.

Regards,

Tony

^ permalink raw reply

* Re: [linux-pm] [PATCH v4 2/4] mfd: omap: control: core system control driver
From: Konstantin Baydarov @ 2012-08-08 14:59 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: balbi, kishon, amit.kucheria, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <20120808143929.GJ11011@atomide.com>

  Hi, Tony.

On 08/08/2012 06:39 PM, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [120808 07:11]:
>> * Tony Lindgren <tony@atomide.com> [120808 07:05]:
>>> * Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120725 04:10]:
>>>> +
>>>> +u32 omap_control_status_read(void)
>>>> +{
>>>> +	return __raw_readl(omap_control_base);
>>>> +}
>>> Ah OK it's changed here.. Sorry I was looking at the older
>>> version.
>>>
>>>> +void __init of_omap_control_init(const struct of_device_id *matches)
>>>> +{
>>>> +	struct device_node *np;
>>>> +	struct property *pp = 0;
>>>> +
>>>> +	for_each_matching_node(np, matches) {
>>>> +		pp = of_find_property(np, "reg", NULL);
>>>> +		if(pp) {
>>>> +			omap_control_phys_base = (unsigned long)be32_to_cpup(pp->value);
>>>> +			omap_control_mapsize = (size_t)be32_to_cpup( (void*)((char*)pp->value + 4) );
>>>> +			/*
>>>> +			 * Map control module register CONTROL_STATUS register:
>>>> +			 * omap24xx - OMAP24XX_CONTROL_STATUS
>>>> +			 * am33xx   - AM33XX_CONTROL_STATUS
>>>> +			 * omap34xx - OMAP343X_CONTROL_STATUS
>>>> +			 * omap44xx - OMAP4_CTRL_MODULE_CORE_STATUS
>>>> +			 * omap54xx - OMAP5XXX_CONTROL_STATUS
>>>> +			 */
>>>> +			omap_control_base = ioremap(omap_control_phys_base, omap_control_mapsize);
>>>> +		}
>>>> +	}
>>>> +}
>>> You should probably add a function for setting omap_control_base
>>> separately from *set_globals* in arch/arm/mach-omap2/common.c.
>>> That way it's initialized early for id.c, and you can initialize
>>> everything else later as regular device drivers.
>>>
>>> FYI, we want to initialize as much as possible late so we have
>>> proper debugging console set up in case things go wrong.
>> Hmm it seems that omap_control_base is now only initialized for DT boot
>> case? This will break booting on almost all systems..
> Looking at things more, looks like omap_type() is getting called
> early from timer.c. So maybe just let set_globals initialize
> omap_control_base and let omap_type() read that directly. That way
> the SCM core driver can behave like a regular device driver and it
> does not need to be initialized early.

  Yes, omap_type() is called very early , that is why I'm using early_initcall for omap_control_base initialization.

Do you mean following?:
void __init omap2_set_globals_control(struct omap_globals *omap2_globals)
{
    if (omap2_globals->ctrl)
        omap2_ctrl_base = omap2_globals->ctrl;

    if (omap2_globals->ctrl_pad)
        omap4_ctrl_pad_base = omap2_globals->ctrl_pad;

    omap_control_base = omap2_ctrl_base;  // this line is added
}

  Or you suggest to move
void __init of_omap_control_init(const struct of_device_id *matches)
{
    struct device_node *np;
    struct property *pp = 0;

    for_each_matching_node(np, matches) {
        pp = of_find_property(np, "reg", NULL);
        if(pp) {
            omap_control_phys_base = (unsigned long)be32_to_cpup(pp->value);
            omap_control_mapsize = (size_t)be32_to_cpup( (void*)((char*)pp->value + 4) );
            /*
             * Map control module register CONTROL_STATUS register:
             * omap24xx - OMAP24XX_CONTROL_STATUS
             * am33xx   - AM33XX_CONTROL_STATUS
             * omap34xx - OMAP343X_CONTROL_STATUS
             * omap44xx - OMAP4_CTRL_MODULE_CORE_STATUS
             * omap54xx - OMAP5XXX_CONTROL_STATUS
             */
            omap_control_base = ioremap(omap_control_phys_base, omap_control_mapsize);
        }
    }
}

into omap2_set_globals_control() ?

Also if we move omap_control_base initialization to omap2_set_globals_control(), than the initialization routine of drivers/mfd/omap-control-core.c become empty, because omap_control_base is the only thing that is initialized in omap-control-core driver.

  BR,
    Konstantin Baydarov.

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


^ permalink raw reply

* Re: [linux-pm] [PATCH v4 2/4] mfd: omap: control: core system control driver
From: Tony Lindgren @ 2012-08-09  6:18 UTC (permalink / raw)
  To: Konstantin Baydarov
  Cc: balbi, kishon, amit.kucheria, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <50227ED9.3010508@dev.rtsoft.ru>

* Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120808 07:59]:
> On 08/08/2012 06:39 PM, Tony Lindgren wrote:
> 
> Yes, omap_type() is called very early , that is why I'm using early_initcall
> for omap_control_base initialization.
> 
> Do you mean following?:
> void __init omap2_set_globals_control(struct omap_globals *omap2_globals)
> {
>     if (omap2_globals->ctrl)
>         omap2_ctrl_base = omap2_globals->ctrl;
> 
>     if (omap2_globals->ctrl_pad)
>         omap4_ctrl_pad_base = omap2_globals->ctrl_pad;
> 
>     omap_control_base = omap2_ctrl_base;  // this line is added
> }

OK so we already have set_globals_control, but we're not using it..
No need for this line is added above.

Let's do the attached clean-up patch and just leave omap_control_base
out of the driver for now as it's not currently needed there.

If omap_control_base is needed in the driver, then we need to pass
it in the platform_data to the driver for the non-DT boot case, or
parse it from DT like you're doing.

Regards,

Tony


From: Tony Lindgren <tony@atomide.com>
Date: Wed, 8 Aug 2012 23:13:03 -0700
Subject: [PATCH] ARM: OMAP2+: Change omap_type() to use omap_ctrl_base_get()

We have the SoC specific ctrl_base already initialized in
set_globals.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -42,28 +42,20 @@ int omap_type(void)
 {
 	u32 val = 0;
 
-	if (cpu_is_omap24xx()) {
-		val = omap_ctrl_readl(OMAP24XX_CONTROL_STATUS);
-	} else if (soc_is_am33xx()) {
-		val = omap_ctrl_readl(AM33XX_CONTROL_STATUS);
-	} else if (cpu_is_omap34xx()) {
-		val = omap_ctrl_readl(OMAP343X_CONTROL_STATUS);
-	} else if (cpu_is_omap44xx()) {
-		val = omap_ctrl_readl(OMAP4_CTRL_MODULE_CORE_STATUS);
-	} else if (soc_is_omap54xx()) {
-		val = omap_ctrl_readl(OMAP5XXX_CONTROL_STATUS);
+	val = __raw_readl(omap_ctrl_base_get());
+	if (!val) {
+		pr_err("Cannot detect omap type!\n");
+		return 0;
+	}
+
+	if (soc_is_omap54xx()) {
 		val &= OMAP5_DEVICETYPE_MASK;
 		val >>= 6;
-		goto out;
 	} else {
-		pr_err("Cannot detect omap type!\n");
-		goto out;
+		val &= OMAP2_DEVICETYPE_MASK;
+		val >>= 8;
 	}
 
-	val &= OMAP2_DEVICETYPE_MASK;
-	val >>= 8;
-
-out:
 	return val;
 }
 EXPORT_SYMBOL(omap_type);

^ permalink raw reply

* Re: [linux-pm] [PATCH v4 2/4] mfd: omap: control: core system control driver
From: Konstantin Baydarov @ 2012-08-09 11:00 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: balbi, kishon, amit.kucheria, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <50227ED9.3010508@dev.rtsoft.ru>

  Hi, Tony.

On 08/08/2012 06:59 PM, Konstantin Baydarov wrote:
>   Yes, omap_type() is called very early , that is why I'm using early_initcall for omap_control_base initialization.
>
> Do you mean following?:
> void __init omap2_set_globals_control(struct omap_globals *omap2_globals)
> {
>     if (omap2_globals->ctrl)
>         omap2_ctrl_base = omap2_globals->ctrl;
>
>     if (omap2_globals->ctrl_pad)
>         omap4_ctrl_pad_base = omap2_globals->ctrl_pad;
>
>     omap_control_base = omap2_ctrl_base;  // this line is added
> }
  Sorry for the confusion - the code above isn't correct. First, as omap-control-core.c driver maps only control module status register the omap_control_base should be renamed to omap_control_status_reg:

--- a/drivers/mfd/omap-control-core.c
+++ b/drivers/mfd/omap-control-core.c
@@ -35,13 +35,15 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-void __iomem *omap_control_base;
+void __iomem *omap_control_status_reg;
 unsigned long omap_control_phys_base;
 size_t omap_control_mapsize;
 
 u32 omap_control_status_read(void)


Then, if you want to move omap_control_status_reg(omap_control_base) initialization to the
omap2_set_globals_control(), it can be done following way:

--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -149,6 +149,8 @@ static struct omap3_control_regs control_context;
 #define OMAP_CTRL_REGADDR(reg)		(omap2_ctrl_base + (reg))
 #define OMAP4_CTRL_PAD_REGADDR(reg)	(omap4_ctrl_pad_base + (reg))
 
+extern void __iomem *omap_control_status_reg;
+
 void __init omap2_set_globals_control(struct omap_globals *omap2_globals)
 {
 	if (omap2_globals->ctrl)
@@ -156,6 +158,20 @@ void __init omap2_set_globals_control(struct omap_globals *omap2_globals)
 
 	if (omap2_globals->ctrl_pad)
 		omap4_ctrl_pad_base = omap2_globals->ctrl_pad;
+
+	omap_control_status_reg = omap2_ctrl_base;
+	if (cpu_is_omap24xx())
+		omap_control_status_reg += OMAP24XX_CONTROL_STATUS;
+	else if (soc_is_am33xx())
+		omap_control_status_reg += AM33XX_CONTROL_STATUS;
+	else if (cpu_is_omap34xx())
+		omap_control_status_reg += OMAP343X_CONTROL_STATUS;
+	else if (cpu_is_omap44xx())
+		omap_control_status_reg += OMAP4_CTRL_MODULE_CORE_STATUS;
+	else if (soc_is_omap54xx())
+		omap_control_status_reg += OMAP5XXX_CONTROL_STATUS;
+	else
+		omap_control_status_reg = 0;
 }
 
 void __iomem *omap_ctrl_base_get(void)


Then omap_type() can be changed:

diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 8018cad..916b3f6 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -39,31 +39,27 @@ unsigned int omap_rev(void)
 }
 EXPORT_SYMBOL(omap_rev);
 
+extern void __iomem *omap_control_status_reg;
+
 int omap_type(void)
 {
 	u32 val = 0;
 
-	if (cpu_is_omap24xx()) {
-		val = omap_control_status_read();
-	} else if (soc_is_am33xx()) {
-		val = omap_control_status_read();
-	} else if (cpu_is_omap34xx()) {
-		val = omap_control_status_read();
-	} else if (cpu_is_omap44xx()) {
+	if(!omap_control_status_reg) {
+		pr_err("Cannot detect omap type!\n");
+		goto out;
+	}
+
+	if(!soc_is_omap54xx()) {
 		val = omap_control_status_read();
-	} else if (soc_is_omap54xx()) {
+		val &= OMAP2_DEVICETYPE_MASK;
+		val >>= 8;
+	} else {
 		val = omap_control_status_read();
 		val &= OMAP5_DEVICETYPE_MASK;
 		val >>= 6;
-		goto out;
-	} else {
-		pr_err("Cannot detect omap type!\n");
-		goto out;
 	}
 
-	val &= OMAP2_DEVICETYPE_MASK;
-	val >>= 8;
-
 out:
 	return val;
 }


  BR,
    Konstantin Baydarov.

>   Or you suggest to move
> void __init of_omap_control_init(const struct of_device_id *matches)
> {
>     struct device_node *np;
>     struct property *pp = 0;
>
>     for_each_matching_node(np, matches) {
>         pp = of_find_property(np, "reg", NULL);
>         if(pp) {
>             omap_control_phys_base = (unsigned long)be32_to_cpup(pp->value);
>             omap_control_mapsize = (size_t)be32_to_cpup( (void*)((char*)pp->value + 4) );
>             /*
>              * Map control module register CONTROL_STATUS register:
>              * omap24xx - OMAP24XX_CONTROL_STATUS
>              * am33xx   - AM33XX_CONTROL_STATUS
>              * omap34xx - OMAP343X_CONTROL_STATUS
>              * omap44xx - OMAP4_CTRL_MODULE_CORE_STATUS
>              * omap54xx - OMAP5XXX_CONTROL_STATUS
>              */
>             omap_control_base = ioremap(omap_control_phys_base, omap_control_mapsize);
>         }
>     }
> }
>
> into omap2_set_globals_control() ?
>
> Also if we move omap_control_base initialization to omap2_set_globals_control(), than the initialization routine of drivers/mfd/omap-control-core.c become empty, because omap_control_base is the only thing that is initialized in omap-control-core driver.
>
>   BR,
>     Konstantin Baydarov.
>
>> Regards,
>>
>> Tony
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply related

* [REGRESSION] hard lockup on resume from suspend on ThinkPad T23
From: Martin Steigerwald @ 2012-08-09 16:43 UTC (permalink / raw)
  To: linux-pm; +Cc: linux-kernel

Hello!

I thought I report it here – although unless it rings a bell I won´t 
probably be doing much about it like a git-bisect since that machine is 
slow and it could take ages…

With Debian kernel 3.3.0-trunk-686-pae the machine suspends and resume for 
lots of times for 30-40 days uptime and more if I let it.

With Debian kernel 3.4-trunk-686-pae as well as 3.5-trunk-686-pae tried 
out today often not always the kernel locks up hard after resume. That is, 
the image is read back but then when switching to graphics or shortly 
afterwards the mouse pointer is frozen and the machine does not respond to 
ping anymore.

Graphics is ancient:

deepdance:~> lspci -nn | grep VGA
01:00.0 VGA compatible controller [0300]: S3 Inc. SuperSavage IX/C SDR 
[5333:8c2e] (rev 05)

CPU is Mobile Intel(R) Pentium(R) III CPU - M  1133MHz, the machine has 
768 MiB of RAM.

Filesystems are (aged) BTRFS (which contributes to the extreme slowness of 
the machine it seems – apt-get (dist-)upgrade without eatmydata takes 
long).

Kernel parameters are:

linux /vmlinuz-3.3.0-trunk-686-pae root=/dev/mapper/deepdance-debian ro 
vga=791 threadirqs init=/bin/systemd resume=/dev/mapper/deepdance-swap


I didn´t report this with 3.4 cause I hoped that difficult to debug bug 
would just go away with 3.5, but it seems it didn´t. If it could just dump 
out something useful before it crashes… I am using threadirqs since quite 
some time on various machines, but I could try disabling it if it seems to 
be suspicious.

I am back on 3.3 right now. It doesn´t matter that much on this machine.


Unless any brilliant ideas I might just keep it at this report. Maybe it 
rings a bell for someone who has a suggestion to try out.

I have 3.4 on ThinkPad T42 with some radeon chip and 3.4 and since some 
while 3.5 on ThinkPad T520 with Intel Sandybridge graphics – no issues 
there. So it only seems to affect this machine. T520 uses threadirqs, I 
think the T42 as well.

Thanks,
-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

^ permalink raw reply

* Re: [PATCH v4 2/4] mfd: omap: control: core system control driver
From: Tony Lindgren @ 2012-08-10  8:50 UTC (permalink / raw)
  To: Konstantin Baydarov
  Cc: amit.kucheria, kishon, balbi, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <5023985F.3090606@dev.rtsoft.ru>

* Konstantin Baydarov <kbaidarov@dev.rtsoft.ru> [120809 04:00]:
>   Hi, Tony.
> 
> On 08/08/2012 06:59 PM, Konstantin Baydarov wrote:
> >   Yes, omap_type() is called very early , that is why I'm using early_initcall for omap_control_base initialization.
> >
> > Do you mean following?:
> > void __init omap2_set_globals_control(struct omap_globals *omap2_globals)
> > {
> >     if (omap2_globals->ctrl)
> >         omap2_ctrl_base = omap2_globals->ctrl;
> >
> >     if (omap2_globals->ctrl_pad)
> >         omap4_ctrl_pad_base = omap2_globals->ctrl_pad;
> >
> >     omap_control_base = omap2_ctrl_base;  // this line is added
> > }
>   Sorry for the confusion - the code above isn't correct. First, as omap-control-core.c driver maps only control module status register the omap_control_base should be renamed to omap_control_status_reg:

Heh right :) And that makes the patch I posted wrong too.
 
> @@ -156,6 +158,20 @@ void __init omap2_set_globals_control(struct omap_globals *omap2_globals)
>  
>  	if (omap2_globals->ctrl_pad)
>  		omap4_ctrl_pad_base = omap2_globals->ctrl_pad;
> +
> +	omap_control_status_reg = omap2_ctrl_base;
> +	if (cpu_is_omap24xx())
> +		omap_control_status_reg += OMAP24XX_CONTROL_STATUS;
> +	else if (soc_is_am33xx())
> +		omap_control_status_reg += AM33XX_CONTROL_STATUS;
> +	else if (cpu_is_omap34xx())
> +		omap_control_status_reg += OMAP343X_CONTROL_STATUS;
> +	else if (cpu_is_omap44xx())
> +		omap_control_status_reg += OMAP4_CTRL_MODULE_CORE_STATUS;
> +	else if (soc_is_omap54xx())
> +		omap_control_status_reg += OMAP5XXX_CONTROL_STATUS;
> +	else
> +		omap_control_status_reg = 0;
>  }

We can get rid of the if else SoC check here too. And we can limit the
control_status tinkering to id.c. Here's an updated clean-up patch.

Regards,

Tony
 

From: Tony Lindgren <tony@atomide.com>
Date: Wed, 8 Aug 2012 23:13:03 -0700
Subject: [PATCH] ARM: OMAP2+: Initialize things for omap_type() to simplify
 SoC detection

Let's rename omap2_set_globals_tap() to omap2_set_globals_id() and update
the comments to remove old comments about map_io() as we don't need SoC
detection for map_io() any longer.

Let's also initialize control_status_reg with omap2_set_globals_id()
so we can avoid SoC tests every time omap_type() gets called.

Note that this patch sets the ti81xx to always return GP mode as
the mode bits seem to be marked as reserved in ti81xx TRM.

Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/mach-omap2/common.c b/arch/arm/mach-omap2/common.c
index 069f972..64afeee 100644
--- a/arch/arm/mach-omap2/common.c
+++ b/arch/arm/mach-omap2/common.c
@@ -31,7 +31,7 @@
 
 static void __init __omap2_set_globals(struct omap_globals *omap2_globals)
 {
-	omap2_set_globals_tap(omap2_globals);
+	omap2_set_globals_id(omap2_globals);
 	omap2_set_globals_sdrc(omap2_globals);
 	omap2_set_globals_control(omap2_globals);
 	omap2_set_globals_prcm(omap2_globals);
@@ -45,6 +45,8 @@ static struct omap_globals omap242x_globals = {
 	.sdrc	= OMAP2_L3_IO_ADDRESS(OMAP2420_SDRC_BASE),
 	.sms	= OMAP2_L3_IO_ADDRESS(OMAP2420_SMS_BASE),
 	.ctrl	= OMAP2_L4_IO_ADDRESS(OMAP242X_CTRL_BASE),
+	.ctrl_status	= OMAP2_L4_IO_ADDRESS(OMAP242X_CTRL_BASE) +
+				OMAP24XX_CONTROL_STATUS,
 	.prm	= OMAP2_L4_IO_ADDRESS(OMAP2420_PRM_BASE),
 	.cm	= OMAP2_L4_IO_ADDRESS(OMAP2420_CM_BASE),
 };
@@ -68,6 +70,8 @@ static struct omap_globals omap243x_globals = {
 	.sdrc	= OMAP2_L3_IO_ADDRESS(OMAP243X_SDRC_BASE),
 	.sms	= OMAP2_L3_IO_ADDRESS(OMAP243X_SMS_BASE),
 	.ctrl	= OMAP2_L4_IO_ADDRESS(OMAP243X_CTRL_BASE),
+	.ctrl_status	= OMAP2_L4_IO_ADDRESS(OMAP243X_CTRL_BASE) +
+				OMAP24XX_CONTROL_STATUS,
 	.prm	= OMAP2_L4_IO_ADDRESS(OMAP2430_PRM_BASE),
 	.cm	= OMAP2_L4_IO_ADDRESS(OMAP2430_CM_BASE),
 };
@@ -91,6 +95,8 @@ static struct omap_globals omap3_globals = {
 	.sdrc	= OMAP2_L3_IO_ADDRESS(OMAP343X_SDRC_BASE),
 	.sms	= OMAP2_L3_IO_ADDRESS(OMAP343X_SMS_BASE),
 	.ctrl	= OMAP2_L4_IO_ADDRESS(OMAP343X_CTRL_BASE),
+	.ctrl_status	= OMAP2_L4_IO_ADDRESS(OMAP343X_CTRL_BASE) +
+				OMAP343X_CONTROL_STATUS,
 	.prm	= OMAP2_L4_IO_ADDRESS(OMAP3430_PRM_BASE),
 	.cm	= OMAP2_L4_IO_ADDRESS(OMAP3430_CM_BASE),
 };
@@ -117,6 +123,8 @@ static struct omap_globals ti81xx_globals = {
 	.class  = OMAP343X_CLASS,
 	.tap    = OMAP2_L4_IO_ADDRESS(TI81XX_TAP_BASE),
 	.ctrl   = OMAP2_L4_IO_ADDRESS(TI81XX_CTRL_BASE),
+	.ctrl_status   = OMAP2_L4_IO_ADDRESS(TI81XX_CTRL_BASE) +
+				TI81XX_CONTROL_STATUS,
 	.prm    = OMAP2_L4_IO_ADDRESS(TI81XX_PRCM_BASE),
 	.cm     = OMAP2_L4_IO_ADDRESS(TI81XX_PRCM_BASE),
 };
@@ -140,6 +148,8 @@ static struct omap_globals am33xx_globals = {
 	.class  = AM335X_CLASS,
 	.tap    = AM33XX_L4_WK_IO_ADDRESS(AM33XX_TAP_BASE),
 	.ctrl   = AM33XX_L4_WK_IO_ADDRESS(AM33XX_CTRL_BASE),
+	.ctrl_status = AM33XX_L4_WK_IO_ADDRESS(AM33XX_CTRL_BASE) +
+			AM33XX_CONTROL_STATUS,
 	.prm    = AM33XX_L4_WK_IO_ADDRESS(AM33XX_PRCM_BASE),
 	.cm     = AM33XX_L4_WK_IO_ADDRESS(AM33XX_PRCM_BASE),
 };
@@ -160,6 +170,8 @@ static struct omap_globals omap4_globals = {
 	.class	= OMAP443X_CLASS,
 	.tap	= OMAP2_L4_IO_ADDRESS(OMAP443X_SCM_BASE),
 	.ctrl	= OMAP2_L4_IO_ADDRESS(OMAP443X_SCM_BASE),
+	.ctrl_status	= OMAP2_L4_IO_ADDRESS(OMAP443X_SCM_BASE) +
+				OMAP4_CTRL_MODULE_CORE_STATUS,
 	.ctrl_pad	= OMAP2_L4_IO_ADDRESS(OMAP443X_CTRL_BASE),
 	.prm	= OMAP2_L4_IO_ADDRESS(OMAP4430_PRM_BASE),
 	.cm	= OMAP2_L4_IO_ADDRESS(OMAP4430_CM_BASE),
@@ -183,6 +195,8 @@ static struct omap_globals omap5_globals = {
 	.class	= OMAP54XX_CLASS,
 	.tap	= OMAP2_L4_IO_ADDRESS(OMAP54XX_SCM_BASE),
 	.ctrl	= OMAP2_L4_IO_ADDRESS(OMAP54XX_SCM_BASE),
+	.ctrl_status	= OMAP2_L4_IO_ADDRESS(OMAP54XX_SCM_BASE) +
+				OMAP5XXX_CONTROL_STATUS,
 	.ctrl_pad	= OMAP2_L4_IO_ADDRESS(OMAP54XX_CTRL_BASE),
 	.prm	= OMAP2_L4_IO_ADDRESS(OMAP54XX_PRM_BASE),
 	.cm	= OMAP2_L4_IO_ADDRESS(OMAP54XX_CM_CORE_AON_BASE),
@@ -192,7 +206,7 @@ static struct omap_globals omap5_globals = {
 
 void __init omap2_set_globals_5xxx(void)
 {
-	omap2_set_globals_tap(&omap5_globals);
+	omap2_set_globals_id(&omap5_globals);
 	omap2_set_globals_control(&omap5_globals);
 	omap2_set_globals_prcm(&omap5_globals);
 }
diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index 1f65b18..a3cee8c 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -168,6 +168,7 @@ struct omap_globals {
 	void __iomem	*sdrc;           /* SDRAM Controller */
 	void __iomem	*sms;            /* SDRAM Memory Scheduler */
 	void __iomem	*ctrl;           /* System Control Module */
+	void __iomem	*ctrl_status;	/* Control status register */
 	void __iomem	*ctrl_pad;	/* PAD Control Module */
 	void __iomem	*prm;            /* Power and Reset Management */
 	void __iomem	*cm;             /* Clock Management */
@@ -184,7 +185,7 @@ void omap2_set_globals_ti81xx(void);
 void omap2_set_globals_am33xx(void);
 
 /* These get called from omap2_set_globals_xxxx(), do not call these */
-void omap2_set_globals_tap(struct omap_globals *);
+void omap2_set_globals_id(struct omap_globals *);
 #if defined(CONFIG_SOC_HAS_OMAP2_SDRC)
 void omap2_set_globals_sdrc(struct omap_globals *);
 #else
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index b8cdc85..cf92e06 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -59,6 +59,7 @@
 
 /* TI81XX spefic control submodules */
 #define TI81XX_CONTROL_DEVCONF		0x600
+#define TI81XX_CONTROL_STATUS		0x040
 
 /* Control register offsets - read/write with omap_ctrl_{read,write}{bwl}() */
 
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 40373db..19cc66d 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -29,6 +29,7 @@
 #include "control.h"
 
 static unsigned int omap_revision;
+static unsigned int control_status;
 static const char *cpu_rev;
 u32 omap_features;
 
@@ -40,31 +41,13 @@ EXPORT_SYMBOL(omap_rev);
 
 int omap_type(void)
 {
-	u32 val = 0;
-
-	if (cpu_is_omap24xx()) {
-		val = omap_ctrl_readl(OMAP24XX_CONTROL_STATUS);
-	} else if (soc_is_am33xx()) {
-		val = omap_ctrl_readl(AM33XX_CONTROL_STATUS);
-	} else if (cpu_is_omap34xx()) {
-		val = omap_ctrl_readl(OMAP343X_CONTROL_STATUS);
-	} else if (cpu_is_omap44xx()) {
-		val = omap_ctrl_readl(OMAP4_CTRL_MODULE_CORE_STATUS);
-	} else if (soc_is_omap54xx()) {
-		val = omap_ctrl_readl(OMAP5XXX_CONTROL_STATUS);
-		val &= OMAP5_DEVICETYPE_MASK;
-		val >>= 6;
-		goto out;
-	} else {
-		pr_err("Cannot detect omap type!\n");
-		goto out;
+	if (!control_status) {
+		pr_err("Uninitialized control_status!\n");
+		WARN_ON(1);
+		return 0;
 	}
 
-	val &= OMAP2_DEVICETYPE_MASK;
-	val >>= 8;
-
-out:
-	return val;
+	return control_status;
 }
 EXPORT_SYMBOL(omap_type);
 
@@ -554,13 +537,9 @@ void __init omap5xxx_check_revision(void)
 }
 
 /*
- * Set up things for map_io and processor detection later on. Gets called
- * pretty much first thing from board init. For multi-omap, this gets
- * cpu_is_omapxxxx() working accurately enough for map_io. Then we'll try to
- * detect the exact revision later on in omap2_detect_revision() once map_io
- * is done.
+ * Initialize things for SoC detecttion. Gets called from init_early.
  */
-void __init omap2_set_globals_tap(struct omap_globals *omap2_globals)
+void __init omap2_set_globals_id(struct omap_globals *omap2_globals)
 {
 	omap_revision = omap2_globals->class;
 	tap_base = omap2_globals->tap;
@@ -569,4 +548,16 @@ void __init omap2_set_globals_tap(struct omap_globals *omap2_globals)
 		tap_prod_id = 0x0210;
 	else
 		tap_prod_id = 0x0208;
+
+	control_status = __raw_readl(omap2_globals->ctrl_status);
+	if (cpu_is_ti81xx()) {
+		/* At least ti81xx TRM sprugx8.pdf lists type bits as reserved */
+		control_status = OMAP2_DEVICE_TYPE_GP;
+	} else if (soc_is_omap54xx()) {
+		control_status &= OMAP5_DEVICETYPE_MASK;
+		control_status >>= 6;
+	} else {
+		control_status &= OMAP2_DEVICETYPE_MASK;
+		control_status >>= 8;
+	}
 }

^ permalink raw reply related

* Re: [PATCH v4 4/5] thermal: exynos: Register the tmu sensor with the kernel thermal layer
From: Hongbo Zhang @ 2012-08-10 11:49 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	durgadoss.r-ral2JQCrhuEAvxtiuMwx3w,
	patches-QSEj5FYQhm4dnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	rui.zhang-ral2JQCrhuEAvxtiuMwx3w, khali-PUYAD+kWke1g9hUCZPvPmw,
	linaro-dev-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	lenb-DgEjT+Ai2ygdnm+yROfE0A,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	guenter.roeck-IzeFyvvaP7pWk0Htik3J/w
In-Reply-To: <1336815645-29625-5-git-send-email-amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 16719 bytes --]

On 12 May 2012 17:40, Amit Daniel Kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> This code added creates a link between temperature sensors, linux thermal
> framework and cooling devices for samsung exynos platform. This layer
> monitors the temperature from the sensor and informs the generic thermal
> layer to take the necessary cooling action.
>
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/thermal/exynos_thermal.c             |  344
> +++++++++++++++++++++++++-
>  include/linux/platform_data/exynos_thermal.h |    6 +
>  2 files changed, 348 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/exynos_thermal.c
> b/drivers/thermal/exynos_thermal.c
> index cfe4aeb..48106d8 100644
> --- a/drivers/thermal/exynos_thermal.c
> +++ b/drivers/thermal/exynos_thermal.c
> @@ -35,6 +35,9 @@
>  #include <linux/mutex.h>
>  #include <linux/err.h>
>  #include <linux/platform_data/exynos_thermal.h>
> +#include <linux/thermal.h>
> +#include <linux/cpufreq.h>
> +#include <linux/cpu_cooling.h>
>  #include <linux/of.h>
>
>  #include <plat/cpu.h>
> @@ -95,6 +98,7 @@
>
>  #define ACTIVE_INTERVAL 500
>  #define IDLE_INTERVAL 10000
> +#define MCELSIUS       1000
>
>  /* CPU Zone information */
>  #define PANIC_ZONE      4
> @@ -105,6 +109,8 @@
>  #define GET_ZONE(trip) (trip + 2)
>  #define GET_TRIP(zone) (zone - 2)
>
> +#define EXYNOS_ZONE_COUNT      3
> +
>  struct exynos_tmu_data {
>         struct exynos_tmu_platform_data *pdata;
>         struct resource *mem;
> @@ -117,6 +123,309 @@ struct exynos_tmu_data {
>         u8 temp_error1, temp_error2;
>  };
>
> +struct thermal_trip_point_conf {
> +       int trip_val[MAX_TRIP_COUNT];
> +       int trip_count;
> +};
> +
> +struct thermal_cooling_conf {
> +       struct freq_clip_table freq_data[MAX_TRIP_COUNT];
> +       int freq_clip_count;
> +};
> +
> +struct thermal_sensor_conf {
> +       char name[SENSOR_NAME_LEN];
> +       int (*read_temperature)(void *data);
> +       struct thermal_trip_point_conf trip_data;
> +       struct thermal_cooling_conf cooling_data;
> +       void *private_data;
> +};
> +
> +struct exynos_thermal_zone {
> +       enum thermal_device_mode mode;
> +       struct thermal_zone_device *therm_dev;
> +       struct thermal_cooling_device *cool_dev[MAX_COOLING_DEVICE];
> +       unsigned int cool_dev_size;
> +       struct platform_device *exynos4_dev;
> +       struct thermal_sensor_conf *sensor_conf;
> +};
> +
> +static struct exynos_thermal_zone *th_zone;
> +static void exynos_unregister_thermal(void);
> +static int exynos_register_thermal(struct thermal_sensor_conf
> *sensor_conf);
> +
> +/* Get mode callback functions for thermal zone */
> +static int exynos_get_mode(struct thermal_zone_device *thermal,
> +                       enum thermal_device_mode *mode)
> +{
> +       if (th_zone)
> +               *mode = th_zone->mode;
> +       return 0;
> +}
> +
> +/* Set mode callback functions for thermal zone */
> +static int exynos_set_mode(struct thermal_zone_device *thermal,
> +                       enum thermal_device_mode mode)
> +{
> +       if (!th_zone->therm_dev) {
> +               pr_notice("thermal zone not registered\n");
> +               return 0;
> +       }
> +
> +       mutex_lock(&th_zone->therm_dev->lock);
> +
> +       if (mode == THERMAL_DEVICE_ENABLED)
> +               th_zone->therm_dev->polling_delay = IDLE_INTERVAL;
> +       else
> +               th_zone->therm_dev->polling_delay = 0;
> +
> +       mutex_unlock(&th_zone->therm_dev->lock);
> +
> +       th_zone->mode = mode;
> +       thermal_zone_device_update(th_zone->therm_dev);
>
I think it should be like this:
if (mode == THERMAL_DEVICE_ENABLED)
    thermal_zone_device_update(th_zone->therm_dev);
else
    disable cooling device;

Imagine that when CPU goes high and the frequency is limited to low,
and the you think it is safe I want a high speed CPU, then you disable
the thermal mode by sysfs, but the CPU frequency is still low.

 +       pr_info("thermal polling set for duration=%d msec\n",
> +                               th_zone->therm_dev->polling_delay);
> +       return 0;
> +}
> +
> +/*
> + * This function may be called from interrupt based temperature sensor
> + * when threshold is changed.
> + */
> +static void exynos_report_trigger(void)
> +{
> +       unsigned int i;
> +       char data[10];
> +       char *envp[] = { data, NULL };
> +
> +       if (!th_zone || !th_zone->therm_dev)
> +               return;
> +
> +       thermal_zone_device_update(th_zone->therm_dev);
> +
> +       mutex_lock(&th_zone->therm_dev->lock);
> +       /* Find the level for which trip happened */
> +       for (i = 0; i < th_zone->sensor_conf->trip_data.trip_count; i++) {
> +               if (th_zone->therm_dev->last_temperature <
> +                       th_zone->sensor_conf->trip_data.trip_val[i] *
> MCELSIUS)
> +                       break;
> +       }
> +
> +       if (th_zone->mode == THERMAL_DEVICE_ENABLED) {
> +               if (i > 0)
> +                       th_zone->therm_dev->polling_delay =
> ACTIVE_INTERVAL;
> +               else
> +                       th_zone->therm_dev->polling_delay = IDLE_INTERVAL;
> +       }
> +
> +       snprintf(data, sizeof(data), "%u", i);
> +       kobject_uevent_env(&th_zone->therm_dev->device.kobj, KOBJ_CHANGE,
> envp);
> +       mutex_unlock(&th_zone->therm_dev->lock);
> +}
> +
> +/* Get trip type callback functions for thermal zone */
> +static int exynos_get_trip_type(struct thermal_zone_device *thermal, int
> trip,
> +                                enum thermal_trip_type *type)
> +{
> +       switch (GET_ZONE(trip)) {
> +       case MONITOR_ZONE:
> +       case WARN_ZONE:
> +               *type = THERMAL_TRIP_ACTIVE;
> +               break;
> +       case PANIC_ZONE:
> +               *type = THERMAL_TRIP_CRITICAL;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +       return 0;
> +}
> +
> +/* Get trip temperature callback functions for thermal zone */
> +static int exynos_get_trip_temp(struct thermal_zone_device *thermal, int
> trip,
> +                               unsigned long *temp)
> +{
> +       if (trip < GET_TRIP(MONITOR_ZONE) || trip > GET_TRIP(PANIC_ZONE))
> +               return -EINVAL;
> +
> +       *temp = th_zone->sensor_conf->trip_data.trip_val[trip];
> +       /* convert the temperature into millicelsius */
> +       *temp = *temp * MCELSIUS;
> +
> +       return 0;
> +}
> +
> +/* Get critical temperature callback functions for thermal zone */
> +static int exynos_get_crit_temp(struct thermal_zone_device *thermal,
> +                               unsigned long *temp)
> +{
> +       int ret;
> +       /* Panic zone */
> +       ret = exynos_get_trip_temp(thermal, GET_TRIP(PANIC_ZONE), temp);
> +       return ret;
> +}
> +
> +/* Bind callback functions for thermal zone */
> +static int exynos_bind(struct thermal_zone_device *thermal,
> +                       struct thermal_cooling_device *cdev)
> +{
> +       int ret = 0, i;
> +
> +       /* find the cooling device registered*/
> +       for (i = 0; i < th_zone->cool_dev_size; i++)
> +               if (cdev == th_zone->cool_dev[i])
> +                       break;
> +
> +       /*No matching cooling device*/
> +       if (i == th_zone->cool_dev_size)
> +               return 0;
> +
> +       switch (GET_ZONE(i)) {
> +       case MONITOR_ZONE:
> +       case WARN_ZONE:
> +               if (thermal_zone_bind_cooling_device(thermal, i, cdev)) {
> +                       pr_err("error binding cooling dev inst 0\n");
> +                       ret = -EINVAL;
> +               }
> +               break;
> +       default:
> +               ret = -EINVAL;
> +       }
> +
> +       return ret;
> +}
> +
> +/* Unbind callback functions for thermal zone */
> +static int exynos_unbind(struct thermal_zone_device *thermal,
> +                       struct thermal_cooling_device *cdev)
> +{
> +       int ret = 0, i;
> +
> +       /* find the cooling device registered*/
> +       for (i = 0; i < th_zone->cool_dev_size; i++)
> +               if (cdev == th_zone->cool_dev[i])
> +                       break;
> +
> +       /*No matching cooling device*/
> +       if (i == th_zone->cool_dev_size)
> +               return 0;
> +
> +       switch (GET_ZONE(i)) {
> +       case MONITOR_ZONE:
> +       case WARN_ZONE:
> +               if (thermal_zone_unbind_cooling_device(thermal, i, cdev))
> {
>
Cooling devices should be disabled before unbinding?

+                       pr_err("error unbinding cooling dev\n");
> +                       ret = -EINVAL;
> +               }
> +               break;
> +       default:
> +               ret = -EINVAL;
> +       }
> +       return ret;
> +}
> +
> +/* Get temperature callback functions for thermal zone */
> +static int exynos_get_temp(struct thermal_zone_device *thermal,
> +                       unsigned long *temp)
> +{
> +       void *data;
> +
> +       if (!th_zone->sensor_conf) {
> +               pr_info("Temperature sensor not initialised\n");
> +               return -EINVAL;
> +       }
> +       data = th_zone->sensor_conf->private_data;
> +       *temp = th_zone->sensor_conf->read_temperature(data);
> +       /* convert the temperature into millicelsius */
> +       *temp = *temp * MCELSIUS;
> +       return 0;
> +}
> +
> +/* Operation callback functions for thermal zone */
> +static struct thermal_zone_device_ops const exynos_dev_ops = {
> +       .bind = exynos_bind,
> +       .unbind = exynos_unbind,
> +       .get_temp = exynos_get_temp,
> +       .get_mode = exynos_get_mode,
> +       .set_mode = exynos_set_mode,
> +       .get_trip_type = exynos_get_trip_type,
> +       .get_trip_temp = exynos_get_trip_temp,
> +       .get_crit_temp = exynos_get_crit_temp,
> +};
> +
> +/* Register with the in-kernel thermal management */
> +static int exynos_register_thermal(struct thermal_sensor_conf
> *sensor_conf)
> +{
> +       int ret, count, tab_size;
> +       struct freq_clip_table *tab_ptr, *clip_data;
> +
> +       if (!sensor_conf || !sensor_conf->read_temperature) {
> +               pr_err("Temperature sensor not initialised\n");
> +               return -EINVAL;
> +       }
> +
> +       th_zone = kzalloc(sizeof(struct exynos_thermal_zone), GFP_KERNEL);
> +       if (!th_zone)
> +               return -ENOMEM;
> +
> +       th_zone->sensor_conf = sensor_conf;
> +
> +       tab_ptr = (struct freq_clip_table
> *)sensor_conf->cooling_data.freq_data;
> +       tab_size = sensor_conf->cooling_data.freq_clip_count;
> +
> +       /* Register the cpufreq cooling device */
> +       for (count = 0; count < tab_size; count++) {
> +               clip_data = (struct freq_clip_table *)&(tab_ptr[count]);
> +               clip_data->mask_val = cpumask_of(0);
> +               th_zone->cool_dev[count] = cpufreq_cooling_register(
> +                                               clip_data, 1);
> +               if (IS_ERR(th_zone->cool_dev[count])) {
> +                       pr_err("Failed to register cpufreq cooling
> device\n");
> +                       ret = -EINVAL;
> +                       th_zone->cool_dev_size = count;
> +                       goto err_unregister;
> +               }
> +       }
> +       th_zone->cool_dev_size = count;
> +
> +       th_zone->therm_dev =
> thermal_zone_device_register(sensor_conf->name,
> +                       EXYNOS_ZONE_COUNT, NULL, &exynos_dev_ops, 0, 0, 0,
> +                       IDLE_INTERVAL);
> +
> +       if (IS_ERR(th_zone->therm_dev)) {
> +               pr_err("Failed to register thermal zone device\n");
> +               ret = -EINVAL;
> +               goto err_unregister;
> +       }
> +       th_zone->mode = THERMAL_DEVICE_ENABLED;
> +
> +       pr_info("Exynos: Kernel Thermal management registered\n");
> +
> +       return 0;
> +
> +err_unregister:
> +       exynos_unregister_thermal();
> +       return ret;
> +}
> +
> +/* Un-Register with the in-kernel thermal management */
> +static void exynos_unregister_thermal(void)
> +{
> +       int i;
> +
> +       for (i = 0; i < th_zone->cool_dev_size; i++) {
> +               if (th_zone && th_zone->cool_dev[i])
> +                       cpufreq_cooling_unregister(th_zone->cool_dev[i]);
> +       }
> +
> +       if (th_zone && th_zone->therm_dev)
> +               thermal_zone_device_unregister(th_zone->therm_dev);
> +
> +       kfree(th_zone);
> +
> +       pr_info("Exynos: Kernel Thermal management unregistered\n");
> +}
> +
>  /*
>   * TMU treats temperature as a mapped temperature code.
>   * The temperature is converted differently depending on the calibration
> type.
> @@ -337,6 +646,7 @@ static void exynos_tmu_work(struct work_struct *work)
>
>         clk_disable(data->clk);
>         mutex_unlock(&data->lock);
> +       exynos_report_trigger();
>         enable_irq(data->irq);
>  }
>
> @@ -349,12 +659,16 @@ static irqreturn_t exynos_tmu_irq(int irq, void *id)
>
>         return IRQ_HANDLED;
>  }
> -
> +static struct thermal_sensor_conf exynos_sensor_conf = {
> +       .name                   = "exynos-therm",
> +       .read_temperature       = (int (*)(void *))exynos_tmu_read,
> +}
> +;
>  static int __devinit exynos_tmu_probe(struct platform_device *pdev)
>  {
>         struct exynos_tmu_data *data;
>         struct exynos_tmu_platform_data *pdata = pdev->dev.platform_data;
> -       int ret;
> +       int ret, i;
>
>         if (!pdata) {
>                 dev_err(&pdev->dev, "No platform init data supplied.\n");
> @@ -432,6 +746,30 @@ static int __devinit exynos_tmu_probe(struct
> platform_device *pdev)
>
>         exynos_tmu_control(pdev, true);
>
> +       /*Register the sensor with thermal management interface*/
> +       (&exynos_sensor_conf)->private_data = data;
> +       exynos_sensor_conf.trip_data.trip_count = pdata->trigger_level0_en
> +
> +                       pdata->trigger_level1_en +
> pdata->trigger_level2_en +
> +                       pdata->trigger_level3_en;
> +
> +       for (i = 0; i < exynos_sensor_conf.trip_data.trip_count; i++)
> +               exynos_sensor_conf.trip_data.trip_val[i] =
> +                       pdata->threshold + pdata->trigger_levels[i];
> +
> +       exynos_sensor_conf.cooling_data.freq_clip_count =
> +                                               pdata->freq_tab_count;
> +       for (i = 0; i < pdata->freq_tab_count; i++) {
> +               exynos_sensor_conf.cooling_data.freq_data[i].freq_clip_max
> =
> +                                       pdata->freq_tab[i].freq_clip_max;
> +               exynos_sensor_conf.cooling_data.freq_data[i].temp_level =
> +                                       pdata->freq_tab[i].temp_level;
> +       }
> +
> +       ret = exynos_register_thermal(&exynos_sensor_conf);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Failed to register thermal
> interface\n");
> +               goto err_clk;
> +       }
>         return 0;
>  err_clk:
>         platform_set_drvdata(pdev, NULL);
> @@ -454,6 +792,8 @@ static int __devexit exynos_tmu_remove(struct
> platform_device *pdev)
>
>         exynos_tmu_control(pdev, false);
>
> +       exynos_unregister_thermal();
> +
>         clk_put(data->clk);
>
>         free_irq(data->irq, data);
> diff --git a/include/linux/platform_data/exynos_thermal.h
> b/include/linux/platform_data/exynos_thermal.h
> index c980af6..858eaca 100644
> --- a/include/linux/platform_data/exynos_thermal.h
> +++ b/include/linux/platform_data/exynos_thermal.h
> @@ -21,6 +21,7 @@
>
>  #ifndef _LINUX_EXYNOS_THERMAL_H
>  #define _LINUX_EXYNOS_THERMAL_H
> +#include <linux/cpu_cooling.h>
>
>  enum calibration_type {
>         TYPE_ONE_POINT_TRIMMING,
> @@ -72,6 +73,9 @@ enum soc_type {
>   * @type: determines the type of SOC
>   * @efuse_value: platform defined fuse value
>   * @cal_type: calibration type for temperature
> + * @freq_clip_table: Table representing frequency reduction percentage.
> + * @freq_tab_count: Count of the above table as frequency reduction may
> + *     applicable to only some of the trigger levels.
>   *
>   * This structure is required for configuration of exynos_tmu driver.
>   */
> @@ -90,5 +94,7 @@ struct exynos_tmu_platform_data {
>
>         enum calibration_type cal_type;
>         enum soc_type type;
> +       struct freq_clip_table freq_tab[4];
> +       unsigned int freq_tab_count;
>  };
>  #endif /* _LINUX_EXYNOS_THERMAL_H */
> --
> 1.7.1
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>

[-- Attachment #1.2: Type: text/html, Size: 19488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 175 bytes --]

_______________________________________________
linaro-dev mailing list
linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox