devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] thermal: Fix cdev registration with THERMAL_NO_LIMIT on 64bit
@ 2014-12-09 12:22 Punit Agrawal
  2014-12-10  1:57 ` Zhang Rui
  0 siblings, 1 reply; 3+ messages in thread
From: Punit Agrawal @ 2014-12-09 12:22 UTC (permalink / raw)
  To: edubezval; +Cc: rui.zhang, linux-pm, devicetree, Punit Agrawal

The size of unsigned long varies between 32 and 64 bit systems while
the size of phandle arguments is always 32 bits per parameter.

On 64-bit systems, cooling devices registered via of-thermal apis fail
to bind when the min/max cooling state is specified as
THERMAL_NO_LIMIT (-1UL) as there is a mis-match between the value read
from the device tree (32bit) and the pre-processor define (64bit).

As we're unlikely to need cooling states larger than 32 bits, and for
consistency with the size of phandle arguments, explicitly limit
THERMAL_NO_LIMIT to 32 bits.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 include/dt-bindings/thermal/thermal.h | 2 +-
 include/linux/thermal.h               | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/dt-bindings/thermal/thermal.h b/include/dt-bindings/thermal/thermal.h
index 59822a9..b5e6b00 100644
--- a/include/dt-bindings/thermal/thermal.h
+++ b/include/dt-bindings/thermal/thermal.h
@@ -11,7 +11,7 @@
 #define _DT_BINDINGS_THERMAL_THERMAL_H
 
 /* On cooling devices upper and lower limits */
-#define THERMAL_NO_LIMIT		(-1UL)
+#define THERMAL_NO_LIMIT		(~0)
 
 #endif
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index ef90838..005586f 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -38,7 +38,7 @@
 #define THERMAL_CSTATE_INVALID -1UL
 
 /* No upper/lower limit requirement */
-#define THERMAL_NO_LIMIT	THERMAL_CSTATE_INVALID
+#define THERMAL_NO_LIMIT	((u32)~0)
 
 /* Unit conversion macros */
 #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
-- 
2.1.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] thermal: Fix cdev registration with THERMAL_NO_LIMIT on 64bit
  2014-12-09 12:22 [PATCH] thermal: Fix cdev registration with THERMAL_NO_LIMIT on 64bit Punit Agrawal
@ 2014-12-10  1:57 ` Zhang Rui
  2014-12-10 14:42   ` Eduardo Valentin
  0 siblings, 1 reply; 3+ messages in thread
From: Zhang Rui @ 2014-12-10  1:57 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: edubezval, linux-pm, devicetree

On Tue, 2014-12-09 at 12:22 +0000, Punit Agrawal wrote:
> The size of unsigned long varies between 32 and 64 bit systems while
> the size of phandle arguments is always 32 bits per parameter.
> 
> On 64-bit systems, cooling devices registered via of-thermal apis fail
> to bind when the min/max cooling state is specified as
> THERMAL_NO_LIMIT (-1UL) as there is a mis-match between the value read
> from the device tree (32bit) and the pre-processor define (64bit).
> 
> As we're unlikely to need cooling states larger than 32 bits, and for
> consistency with the size of phandle arguments, explicitly limit
> THERMAL_NO_LIMIT to 32 bits.
> 
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
>  include/dt-bindings/thermal/thermal.h | 2 +-
>  include/linux/thermal.h               | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/dt-bindings/thermal/thermal.h b/include/dt-bindings/thermal/thermal.h
> index 59822a9..b5e6b00 100644
> --- a/include/dt-bindings/thermal/thermal.h
> +++ b/include/dt-bindings/thermal/thermal.h
> @@ -11,7 +11,7 @@
>  #define _DT_BINDINGS_THERMAL_THERMAL_H
>  
>  /* On cooling devices upper and lower limits */
> -#define THERMAL_NO_LIMIT		(-1UL)
> +#define THERMAL_NO_LIMIT		(~0)
>  
I'm okay with the other change, so Eduardo, what do you think of this?

thanks,
rui
>  #endif
>  
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index ef90838..005586f 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -38,7 +38,7 @@
>  #define THERMAL_CSTATE_INVALID -1UL
>  
>  /* No upper/lower limit requirement */
> -#define THERMAL_NO_LIMIT	THERMAL_CSTATE_INVALID
> +#define THERMAL_NO_LIMIT	((u32)~0)
>  
>  /* Unit conversion macros */
>  #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] thermal: Fix cdev registration with THERMAL_NO_LIMIT on 64bit
  2014-12-10  1:57 ` Zhang Rui
@ 2014-12-10 14:42   ` Eduardo Valentin
  0 siblings, 0 replies; 3+ messages in thread
From: Eduardo Valentin @ 2014-12-10 14:42 UTC (permalink / raw)
  To: Zhang Rui; +Cc: Punit Agrawal, linux-pm, devicetree

[-- Attachment #1: Type: text/plain, Size: 2118 bytes --]

On Wed, Dec 10, 2014 at 09:57:37AM +0800, Zhang Rui wrote:
> On Tue, 2014-12-09 at 12:22 +0000, Punit Agrawal wrote:
> > The size of unsigned long varies between 32 and 64 bit systems while
> > the size of phandle arguments is always 32 bits per parameter.
> > 
> > On 64-bit systems, cooling devices registered via of-thermal apis fail
> > to bind when the min/max cooling state is specified as
> > THERMAL_NO_LIMIT (-1UL) as there is a mis-match between the value read
> > from the device tree (32bit) and the pre-processor define (64bit).
> > 
> > As we're unlikely to need cooling states larger than 32 bits, and for
> > consistency with the size of phandle arguments, explicitly limit
> > THERMAL_NO_LIMIT to 32 bits.
> > 
> > Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> > ---
> >  include/dt-bindings/thermal/thermal.h | 2 +-
> >  include/linux/thermal.h               | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/dt-bindings/thermal/thermal.h b/include/dt-bindings/thermal/thermal.h
> > index 59822a9..b5e6b00 100644
> > --- a/include/dt-bindings/thermal/thermal.h
> > +++ b/include/dt-bindings/thermal/thermal.h
> > @@ -11,7 +11,7 @@
> >  #define _DT_BINDINGS_THERMAL_THERMAL_H
> >  
> >  /* On cooling devices upper and lower limits */
> > -#define THERMAL_NO_LIMIT		(-1UL)
> > +#define THERMAL_NO_LIMIT		(~0)
> >  
> I'm okay with the other change, so Eduardo, what do you think of this?

Adding to my fixes branch.


Prunit, thanks for spotting and fixing this issue.

> 
> thanks,
> rui
> >  #endif
> >  
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index ef90838..005586f 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -38,7 +38,7 @@
> >  #define THERMAL_CSTATE_INVALID -1UL
> >  
> >  /* No upper/lower limit requirement */
> > -#define THERMAL_NO_LIMIT	THERMAL_CSTATE_INVALID
> > +#define THERMAL_NO_LIMIT	((u32)~0)
> >  
> >  /* Unit conversion macros */
> >  #define KELVIN_TO_CELSIUS(t)	(long)(((long)t-2732 >= 0) ?	\
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-12-10 14:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-09 12:22 [PATCH] thermal: Fix cdev registration with THERMAL_NO_LIMIT on 64bit Punit Agrawal
2014-12-10  1:57 ` Zhang Rui
2014-12-10 14:42   ` Eduardo Valentin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).