Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-11-18 Thread Eduardo Valentin
Hello Lukasz,

On Wed, Nov 12, 2014 at 10:42:41AM +0100, Lukasz Majewski wrote:
 Hi Eduardo,
 
  
  Hello Lukasz,
  
  On Fri, Nov 07, 2014 at 11:05:51AM +0100, Lukasz Majewski wrote:
   Hi Eduardo,
   
On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski wrote:
 This patch extends the of-thermal.c to provide information about
 number of available non critical (i.e. non HW) trip points in
 the system.
 
 Signed-off-by: Lukasz Majewski l.majew...@samsung.com
 ---
  drivers/thermal/of-thermal.c   | 12 
  drivers/thermal/thermal_core.h |  5 +
  2 files changed, 17 insertions(+)
 
 diff --git a/drivers/thermal/of-thermal.c
 b/drivers/thermal/of-thermal.c index 23c8d6c..cd74e64 100644
 --- a/drivers/thermal/of-thermal.c
 +++ b/drivers/thermal/of-thermal.c
 @@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct
 thermal_zone_device *tz, int trip) return 1;
  }
  
 +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
 *tz) +{
 + struct __thermal_zone *data = tz-devdata;
 + int i;
 +
 + for (i = 0; i  data-ntrips; i++)
 + if (data-trips[i].type !=
 THERMAL_TRIP_CRITICAL)
 + continue;
 +
 + return --i;
 +}
 +



I am not against this addition. But looks like we start to spread
some specific APIs that may not be used to other drivers.
   
   Why do you think that this is a specific API? In the thermal OF we
   can define trip point as active, passive, hot and critical.
   
   With the first three we can handle them and properly react. For the
   last one SoC's PMU will power down the board.
   
   Do you know if any board (e.g. from TI) is NOT supposed to shut down
   when critical temperature is passed?
   
  
  So, my point is not really about the usage of trip points. It is just
  that the of-thermal  API will grow with in a wide way with specific
  functions to check some property on the trip point array. And not all
  drivers may be using these function, e.g. the above proposal.
  
   The real problem here is the accessibility to __thermal_trip and
   __thermal_bind arrays.
   
   Use case:
   In the Exynos driver we do need to initialize TMU registers with
   threshold temperatures.
   The temperature is read via tz-ops-get_trip_temp() [1] (from
   of-thermal.c).
   Unfortunately, the current API is not exporting the number of
   non-critical trip points to know how many times call [1].
   Of course we could by hand instantiate [1] n times, but this looks
   for me a bit clumsy.
  
  
  I understand your use case. My point was simply that this use case may
  be specific to your driver (or few drivers).
   
   Additionally, we now have implicit assumption about the order of
   defined temperatures for trip points, but I think this is not a big
   issue.
   
Maybe having a
single API to provide a read-only copy the list / array of trips
might be a better approach. I will think of a better way.
   
   Definitely. Exporting available trip points is crucial.
   
  
  Yeah, I think it is the best thing to do. Share a read-only array /
  copy of the needed data, and then drivers would check what ever
  property they need from the array. Just make sure you document that
  this is a read-only array (i.e. any possible change they do, won't
  affect the original array).
 
 So I assume that you don't mind that I will prepare such of-thermal.c
 modification?

No, please, feel free to send the proposal along with your patchset. To
me, it makes sense you do it, because you will also present a real use
case of this required change.

 
  

I also request you to document it accordingly.
   
   Ok, I will do that.
  
  Cool!
  
  
  
   


  static int of_thermal_get_trend(struct thermal_zone_device *tz,
 int trip, enum thermal_trend *trend)
  {
 diff --git a/drivers/thermal/thermal_core.h
 b/drivers/thermal/thermal_core.h index ed8ff05..334a7be 100644
 --- a/drivers/thermal/thermal_core.h
 +++ b/drivers/thermal/thermal_core.h
 @@ -83,6 +83,7 @@ int of_parse_thermal_zones(void);
  void of_thermal_destroy_zones(void);
  int of_thermal_get_ntrips(struct thermal_zone_device *);
  int of_thermal_is_trip_en(struct thermal_zone_device *, int);
 +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
 *); #else
  static inline int of_parse_thermal_zones(void) { return 0; }
  static inline void of_thermal_destroy_zones(void) { }
 @@ -94,6 +95,10 @@ int of_thermal_is_trip_en(struct
 thermal_zone_device *, int) {
   return 0;
  }
 +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
 *)
here, it is supposed to be static inline.

 +{
 + return 0;
 +}
  #endif
  
  #endif /* __THERMAL_CORE_H__ */
 -- 
 2.0.0.rc2
 
   
   
   
   -- 
   Best 

Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-11-18 Thread Lukasz Majewski
On Tue, 18 Nov 2014 11:20:26 -0400
Eduardo Valentin edubez...@gmail.com wrote:

 Hello Lukasz,
 
 On Wed, Nov 12, 2014 at 10:42:41AM +0100, Lukasz Majewski wrote:
  Hi Eduardo,
  
   
   Hello Lukasz,
   
   On Fri, Nov 07, 2014 at 11:05:51AM +0100, Lukasz Majewski wrote:
Hi Eduardo,

 On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski
 wrote:
  This patch extends the of-thermal.c to provide information
  about number of available non critical (i.e. non HW) trip
  points in the system.
  
  Signed-off-by: Lukasz Majewski l.majew...@samsung.com
  ---
   drivers/thermal/of-thermal.c   | 12 
   drivers/thermal/thermal_core.h |  5 +
   2 files changed, 17 insertions(+)
  
  diff --git a/drivers/thermal/of-thermal.c
  b/drivers/thermal/of-thermal.c index 23c8d6c..cd74e64 100644
  --- a/drivers/thermal/of-thermal.c
  +++ b/drivers/thermal/of-thermal.c
  @@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct
  thermal_zone_device *tz, int trip) return 1;
   }
   
  +int of_thermal_get_non_crit_ntrips(struct
  thermal_zone_device *tz) +{
  +   struct __thermal_zone *data = tz-devdata;
  +   int i;
  +
  +   for (i = 0; i  data-ntrips; i++)
  +   if (data-trips[i].type !=
  THERMAL_TRIP_CRITICAL)
  +   continue;
  +
  +   return --i;
  +}
  +
 
 
 
 I am not against this addition. But looks like we start to
 spread some specific APIs that may not be used to other
 drivers.

Why do you think that this is a specific API? In the thermal OF
we can define trip point as active, passive, hot and
critical.

With the first three we can handle them and properly react. For
the last one SoC's PMU will power down the board.

Do you know if any board (e.g. from TI) is NOT supposed to shut
down when critical temperature is passed?

   
   So, my point is not really about the usage of trip points. It is
   just that the of-thermal  API will grow with in a wide way with
   specific functions to check some property on the trip point
   array. And not all drivers may be using these function, e.g. the
   above proposal.
   
The real problem here is the accessibility to __thermal_trip and
__thermal_bind arrays.

Use case:
In the Exynos driver we do need to initialize TMU registers with
threshold temperatures.
The temperature is read via tz-ops-get_trip_temp() [1] (from
of-thermal.c).
Unfortunately, the current API is not exporting the number of
non-critical trip points to know how many times call [1].
Of course we could by hand instantiate [1] n times, but this
looks for me a bit clumsy.
   
   
   I understand your use case. My point was simply that this use
   case may be specific to your driver (or few drivers).

Additionally, we now have implicit assumption about the order of
defined temperatures for trip points, but I think this is not a
big issue.

 Maybe having a
 single API to provide a read-only copy the list / array of
 trips might be a better approach. I will think of a better
 way.

Definitely. Exporting available trip points is crucial.

   
   Yeah, I think it is the best thing to do. Share a read-only
   array / copy of the needed data, and then drivers would check
   what ever property they need from the array. Just make sure you
   document that this is a read-only array (i.e. any possible change
   they do, won't affect the original array).
  
  So I assume that you don't mind that I will prepare such
  of-thermal.c modification?
 
 No, please, feel free to send the proposal along with your patchset.
 To me, it makes sense you do it, because you will also present a real
 use case of this required change.

Ok. I will rebase on top of your today's work.

 
  
   
 
 I also request you to document it accordingly.

Ok, I will do that.
   
   Cool!
   
   
   

 
 
   static int of_thermal_get_trend(struct thermal_zone_device
  *tz, int trip, enum thermal_trend *trend)
   {
  diff --git a/drivers/thermal/thermal_core.h
  b/drivers/thermal/thermal_core.h index ed8ff05..334a7be
  100644 --- a/drivers/thermal/thermal_core.h
  +++ b/drivers/thermal/thermal_core.h
  @@ -83,6 +83,7 @@ int of_parse_thermal_zones(void);
   void of_thermal_destroy_zones(void);
   int of_thermal_get_ntrips(struct thermal_zone_device *);
   int of_thermal_is_trip_en(struct thermal_zone_device *,
  int); +int of_thermal_get_non_crit_ntrips(struct
  thermal_zone_device *); #else
   static inline int of_parse_thermal_zones(void) { return
  0; } static inline void of_thermal_destroy_zones(void) { }
  @@ -94,6 +95,10 @@ int of_thermal_is_trip_en(struct
  thermal_zone_device *, int) {
  return 0;
   }
 

Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-11-12 Thread Lukasz Majewski
Hi Eduardo,

 
 Hello Lukasz,
 
 On Fri, Nov 07, 2014 at 11:05:51AM +0100, Lukasz Majewski wrote:
  Hi Eduardo,
  
   On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski wrote:
This patch extends the of-thermal.c to provide information about
number of available non critical (i.e. non HW) trip points in
the system.

Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
 drivers/thermal/of-thermal.c   | 12 
 drivers/thermal/thermal_core.h |  5 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/thermal/of-thermal.c
b/drivers/thermal/of-thermal.c index 23c8d6c..cd74e64 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct
thermal_zone_device *tz, int trip) return 1;
 }
 
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
*tz) +{
+   struct __thermal_zone *data = tz-devdata;
+   int i;
+
+   for (i = 0; i  data-ntrips; i++)
+   if (data-trips[i].type !=
THERMAL_TRIP_CRITICAL)
+   continue;
+
+   return --i;
+}
+
   
   
   
   I am not against this addition. But looks like we start to spread
   some specific APIs that may not be used to other drivers.
  
  Why do you think that this is a specific API? In the thermal OF we
  can define trip point as active, passive, hot and critical.
  
  With the first three we can handle them and properly react. For the
  last one SoC's PMU will power down the board.
  
  Do you know if any board (e.g. from TI) is NOT supposed to shut down
  when critical temperature is passed?
  
 
 So, my point is not really about the usage of trip points. It is just
 that the of-thermal  API will grow with in a wide way with specific
 functions to check some property on the trip point array. And not all
 drivers may be using these function, e.g. the above proposal.
 
  The real problem here is the accessibility to __thermal_trip and
  __thermal_bind arrays.
  
  Use case:
  In the Exynos driver we do need to initialize TMU registers with
  threshold temperatures.
  The temperature is read via tz-ops-get_trip_temp() [1] (from
  of-thermal.c).
  Unfortunately, the current API is not exporting the number of
  non-critical trip points to know how many times call [1].
  Of course we could by hand instantiate [1] n times, but this looks
  for me a bit clumsy.
 
 
 I understand your use case. My point was simply that this use case may
 be specific to your driver (or few drivers).
  
  Additionally, we now have implicit assumption about the order of
  defined temperatures for trip points, but I think this is not a big
  issue.
  
   Maybe having a
   single API to provide a read-only copy the list / array of trips
   might be a better approach. I will think of a better way.
  
  Definitely. Exporting available trip points is crucial.
  
 
 Yeah, I think it is the best thing to do. Share a read-only array /
 copy of the needed data, and then drivers would check what ever
 property they need from the array. Just make sure you document that
 this is a read-only array (i.e. any possible change they do, won't
 affect the original array).

So I assume that you don't mind that I will prepare such of-thermal.c
modification?

 
   
   I also request you to document it accordingly.
  
  Ok, I will do that.
 
 Cool!
 
 
 
  
   
   
 static int of_thermal_get_trend(struct thermal_zone_device *tz,
int trip, enum thermal_trend *trend)
 {
diff --git a/drivers/thermal/thermal_core.h
b/drivers/thermal/thermal_core.h index ed8ff05..334a7be 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -83,6 +83,7 @@ int of_parse_thermal_zones(void);
 void of_thermal_destroy_zones(void);
 int of_thermal_get_ntrips(struct thermal_zone_device *);
 int of_thermal_is_trip_en(struct thermal_zone_device *, int);
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
*); #else
 static inline int of_parse_thermal_zones(void) { return 0; }
 static inline void of_thermal_destroy_zones(void) { }
@@ -94,6 +95,10 @@ int of_thermal_is_trip_en(struct
thermal_zone_device *, int) {
return 0;
 }
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
*)
   here, it is supposed to be static inline.
   
+{
+   return 0;
+}
 #endif
 
 #endif /* __THERMAL_CORE_H__ */
-- 
2.0.0.rc2

  
  
  
  -- 
  Best regards,
  
  Lukasz Majewski
  
  Samsung RD Institute Poland (SRPOL) | Linux Platform Group



-- 
Best regards,

Lukasz Majewski

Samsung RD Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-11-07 Thread Lukasz Majewski
Hi Eduardo,

 On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski wrote:
  This patch extends the of-thermal.c to provide information about
  number of available non critical (i.e. non HW) trip points in the
  system.
  
  Signed-off-by: Lukasz Majewski l.majew...@samsung.com
  ---
   drivers/thermal/of-thermal.c   | 12 
   drivers/thermal/thermal_core.h |  5 +
   2 files changed, 17 insertions(+)
  
  diff --git a/drivers/thermal/of-thermal.c
  b/drivers/thermal/of-thermal.c index 23c8d6c..cd74e64 100644
  --- a/drivers/thermal/of-thermal.c
  +++ b/drivers/thermal/of-thermal.c
  @@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct
  thermal_zone_device *tz, int trip) return 1;
   }
   
  +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *tz)
  +{
  +   struct __thermal_zone *data = tz-devdata;
  +   int i;
  +
  +   for (i = 0; i  data-ntrips; i++)
  +   if (data-trips[i].type != THERMAL_TRIP_CRITICAL)
  +   continue;
  +
  +   return --i;
  +}
  +
 
 
 
 I am not against this addition. But looks like we start to spread some
 specific APIs that may not be used to other drivers.

Why do you think that this is a specific API? In the thermal OF we can
define trip point as active, passive, hot and critical.

With the first three we can handle them and properly react. For the last
one SoC's PMU will power down the board.

Do you know if any board (e.g. from TI) is NOT supposed to shut down
when critical temperature is passed?

The real problem here is the accessibility to __thermal_trip and
__thermal_bind arrays.

Use case:
In the Exynos driver we do need to initialize TMU registers with
threshold temperatures.
The temperature is read via tz-ops-get_trip_temp() [1] (from
of-thermal.c).
Unfortunately, the current API is not exporting the number of
non-critical trip points to know how many times call [1].
Of course we could by hand instantiate [1] n times, but this looks for
me a bit clumsy.

Additionally, we now have implicit assumption about the order of defined
temperatures for trip points, but I think this is not a big issue.

 Maybe having a
 single API to provide a read-only copy the list / array of trips might
 be a better approach. I will think of a better way.

Definitely. Exporting available trip points is crucial.

 
 I also request you to document it accordingly.

Ok, I will do that.

 
 
   static int of_thermal_get_trend(struct thermal_zone_device *tz,
  int trip, enum thermal_trend *trend)
   {
  diff --git a/drivers/thermal/thermal_core.h
  b/drivers/thermal/thermal_core.h index ed8ff05..334a7be 100644
  --- a/drivers/thermal/thermal_core.h
  +++ b/drivers/thermal/thermal_core.h
  @@ -83,6 +83,7 @@ int of_parse_thermal_zones(void);
   void of_thermal_destroy_zones(void);
   int of_thermal_get_ntrips(struct thermal_zone_device *);
   int of_thermal_is_trip_en(struct thermal_zone_device *, int);
  +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *);
   #else
   static inline int of_parse_thermal_zones(void) { return 0; }
   static inline void of_thermal_destroy_zones(void) { }
  @@ -94,6 +95,10 @@ int of_thermal_is_trip_en(struct
  thermal_zone_device *, int) {
  return 0;
   }
  +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *)
 here, it is supposed to be static inline.
 
  +{
  +   return 0;
  +}
   #endif
   
   #endif /* __THERMAL_CORE_H__ */
  -- 
  2.0.0.rc2
  



-- 
Best regards,

Lukasz Majewski

Samsung RD Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-11-07 Thread Eduardo Valentin

Hello Lukasz,

On Fri, Nov 07, 2014 at 11:05:51AM +0100, Lukasz Majewski wrote:
 Hi Eduardo,
 
  On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski wrote:
   This patch extends the of-thermal.c to provide information about
   number of available non critical (i.e. non HW) trip points in the
   system.
   
   Signed-off-by: Lukasz Majewski l.majew...@samsung.com
   ---
drivers/thermal/of-thermal.c   | 12 
drivers/thermal/thermal_core.h |  5 +
2 files changed, 17 insertions(+)
   
   diff --git a/drivers/thermal/of-thermal.c
   b/drivers/thermal/of-thermal.c index 23c8d6c..cd74e64 100644
   --- a/drivers/thermal/of-thermal.c
   +++ b/drivers/thermal/of-thermal.c
   @@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct
   thermal_zone_device *tz, int trip) return 1;
}

   +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *tz)
   +{
   + struct __thermal_zone *data = tz-devdata;
   + int i;
   +
   + for (i = 0; i  data-ntrips; i++)
   + if (data-trips[i].type != THERMAL_TRIP_CRITICAL)
   + continue;
   +
   + return --i;
   +}
   +
  
  
  
  I am not against this addition. But looks like we start to spread some
  specific APIs that may not be used to other drivers.
 
 Why do you think that this is a specific API? In the thermal OF we can
 define trip point as active, passive, hot and critical.
 
 With the first three we can handle them and properly react. For the last
 one SoC's PMU will power down the board.
 
 Do you know if any board (e.g. from TI) is NOT supposed to shut down
 when critical temperature is passed?
 

So, my point is not really about the usage of trip points. It is just
that the of-thermal  API will grow with in a wide way with specific
functions to check some property on the trip point array. And not all
drivers may be using these function, e.g. the above proposal.

 The real problem here is the accessibility to __thermal_trip and
 __thermal_bind arrays.
 
 Use case:
 In the Exynos driver we do need to initialize TMU registers with
 threshold temperatures.
 The temperature is read via tz-ops-get_trip_temp() [1] (from
 of-thermal.c).
 Unfortunately, the current API is not exporting the number of
 non-critical trip points to know how many times call [1].
 Of course we could by hand instantiate [1] n times, but this looks for
 me a bit clumsy.


I understand your use case. My point was simply that this use case may
be specific to your driver (or few drivers).
 
 Additionally, we now have implicit assumption about the order of defined
 temperatures for trip points, but I think this is not a big issue.
 
  Maybe having a
  single API to provide a read-only copy the list / array of trips might
  be a better approach. I will think of a better way.
 
 Definitely. Exporting available trip points is crucial.
 

Yeah, I think it is the best thing to do. Share a read-only array / copy
of the needed data, and then drivers would check what ever property they
need from the array. Just make sure you document that this is a
read-only array (i.e. any possible change they do, won't affect the
original array).

  
  I also request you to document it accordingly.
 
 Ok, I will do that.

Cool!



 
  
  
static int of_thermal_get_trend(struct thermal_zone_device *tz,
   int trip, enum thermal_trend *trend)
{
   diff --git a/drivers/thermal/thermal_core.h
   b/drivers/thermal/thermal_core.h index ed8ff05..334a7be 100644
   --- a/drivers/thermal/thermal_core.h
   +++ b/drivers/thermal/thermal_core.h
   @@ -83,6 +83,7 @@ int of_parse_thermal_zones(void);
void of_thermal_destroy_zones(void);
int of_thermal_get_ntrips(struct thermal_zone_device *);
int of_thermal_is_trip_en(struct thermal_zone_device *, int);
   +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *);
#else
static inline int of_parse_thermal_zones(void) { return 0; }
static inline void of_thermal_destroy_zones(void) { }
   @@ -94,6 +95,10 @@ int of_thermal_is_trip_en(struct
   thermal_zone_device *, int) {
 return 0;
}
   +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *)
  here, it is supposed to be static inline.
  
   +{
   + return 0;
   +}
#endif

#endif /* __THERMAL_CORE_H__ */
   -- 
   2.0.0.rc2
   
 
 
 
 -- 
 Best regards,
 
 Lukasz Majewski
 
 Samsung RD Institute Poland (SRPOL) | Linux Platform Group


signature.asc
Description: Digital signature


Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-11-07 Thread Lukasz Majewski
Hi Eduardo,

 
 Hello Lukasz,
 
 On Fri, Nov 07, 2014 at 11:05:51AM +0100, Lukasz Majewski wrote:
  Hi Eduardo,
  
   On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski wrote:
This patch extends the of-thermal.c to provide information about
number of available non critical (i.e. non HW) trip points in
the system.

Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
 drivers/thermal/of-thermal.c   | 12 
 drivers/thermal/thermal_core.h |  5 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/thermal/of-thermal.c
b/drivers/thermal/of-thermal.c index 23c8d6c..cd74e64 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct
thermal_zone_device *tz, int trip) return 1;
 }
 
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
*tz) +{
+   struct __thermal_zone *data = tz-devdata;
+   int i;
+
+   for (i = 0; i  data-ntrips; i++)
+   if (data-trips[i].type !=
THERMAL_TRIP_CRITICAL)
+   continue;
+
+   return --i;
+}
+
   
   
   
   I am not against this addition. But looks like we start to spread
   some specific APIs that may not be used to other drivers.
  
  Why do you think that this is a specific API? In the thermal OF we
  can define trip point as active, passive, hot and critical.
  
  With the first three we can handle them and properly react. For the
  last one SoC's PMU will power down the board.
  
  Do you know if any board (e.g. from TI) is NOT supposed to shut down
  when critical temperature is passed?
  
 
 So, my point is not really about the usage of trip points. It is just
 that the of-thermal  API will grow with in a wide way with specific
 functions to check some property on the trip point array. And not all
 drivers may be using these function, e.g. the above proposal.
 
  The real problem here is the accessibility to __thermal_trip and
  __thermal_bind arrays.
  
  Use case:
  In the Exynos driver we do need to initialize TMU registers with
  threshold temperatures.
  The temperature is read via tz-ops-get_trip_temp() [1] (from
  of-thermal.c).
  Unfortunately, the current API is not exporting the number of
  non-critical trip points to know how many times call [1].
  Of course we could by hand instantiate [1] n times, but this looks
  for me a bit clumsy.
 
 
 I understand your use case. My point was simply that this use case may
 be specific to your driver (or few drivers).
  
  Additionally, we now have implicit assumption about the order of
  defined temperatures for trip points, but I think this is not a big
  issue.
  
   Maybe having a
   single API to provide a read-only copy the list / array of trips
   might be a better approach. I will think of a better way.
  
  Definitely. Exporting available trip points is crucial.
  
 
 Yeah, I think it is the best thing to do. Share a read-only array /
 copy of the needed data, and then drivers would check what ever
 property they need from the array. Just make sure you document that
 this is a read-only array (i.e. any possible change they do, won't
 affect the original array).

I agree on that.

 
   
   I also request you to document it accordingly.
  
  Ok, I will do that.
 
 Cool!
 
 
 
  
   
   
 static int of_thermal_get_trend(struct thermal_zone_device *tz,
int trip, enum thermal_trend *trend)
 {
diff --git a/drivers/thermal/thermal_core.h
b/drivers/thermal/thermal_core.h index ed8ff05..334a7be 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -83,6 +83,7 @@ int of_parse_thermal_zones(void);
 void of_thermal_destroy_zones(void);
 int of_thermal_get_ntrips(struct thermal_zone_device *);
 int of_thermal_is_trip_en(struct thermal_zone_device *, int);
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
*); #else
 static inline int of_parse_thermal_zones(void) { return 0; }
 static inline void of_thermal_destroy_zones(void) { }
@@ -94,6 +95,10 @@ int of_thermal_is_trip_en(struct
thermal_zone_device *, int) {
return 0;
 }
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device
*)
   here, it is supposed to be static inline.
   
+{
+   return 0;
+}
 #endif
 
 #endif /* __THERMAL_CORE_H__ */
-- 
2.0.0.rc2

  
  
  
  -- 
  Best regards,
  
  Lukasz Majewski
  
  Samsung RD Institute Poland (SRPOL) | Linux Platform Group



-- 
Best regards,

Lukasz Majewski

Samsung RD Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-11-07 Thread Dmitry Torokhov
Hi Lukasz,

On Fri, Nov 7, 2014 at 2:05 AM, Lukasz Majewski l.majew...@samsung.com wrote:
 Hi Eduardo,

 On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski wrote:
  This patch extends the of-thermal.c to provide information about
  number of available non critical (i.e. non HW) trip points in the
  system.
 
  Signed-off-by: Lukasz Majewski l.majew...@samsung.com
  ---
   drivers/thermal/of-thermal.c   | 12 
   drivers/thermal/thermal_core.h |  5 +
   2 files changed, 17 insertions(+)
 
  diff --git a/drivers/thermal/of-thermal.c
  b/drivers/thermal/of-thermal.c index 23c8d6c..cd74e64 100644
  --- a/drivers/thermal/of-thermal.c
  +++ b/drivers/thermal/of-thermal.c
  @@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct
  thermal_zone_device *tz, int trip) return 1;
   }
 
  +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *tz)
  +{
  +   struct __thermal_zone *data = tz-devdata;
  +   int i;
  +
  +   for (i = 0; i  data-ntrips; i++)
  +   if (data-trips[i].type != THERMAL_TRIP_CRITICAL)
  +   continue;
  +
  +   return --i;
  +}
  +



 I am not against this addition. But looks like we start to spread some
 specific APIs that may not be used to other drivers.

 Why do you think that this is a specific API? In the thermal OF we can
 define trip point as active, passive, hot and critical.

 With the first three we can handle them and properly react. For the last
 one SoC's PMU will power down the board.

 Do you know if any board (e.g. from TI) is NOT supposed to shut down
 when critical temperature is passed?

 The real problem here is the accessibility to __thermal_trip and
 __thermal_bind arrays.

 Use case:
 In the Exynos driver we do need to initialize TMU registers with
 threshold temperatures.

Is this indeed plural or you want to program the next trip point as your
alarm temperature? If so, there was another patch floating around adding
set_trips() callback that would get passed in the low and high trip points for
the current temperature reading and you can use that data to program
alarms. Rockchip thermal driver uses it.

https://lkml.org/lkml/2014/6/27/76

Thanks.

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


Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-11-06 Thread Eduardo Valentin
On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski wrote:
 This patch extends the of-thermal.c to provide information about number of
 available non critical (i.e. non HW) trip points in the system.
 
 Signed-off-by: Lukasz Majewski l.majew...@samsung.com
 ---
  drivers/thermal/of-thermal.c   | 12 
  drivers/thermal/thermal_core.h |  5 +
  2 files changed, 17 insertions(+)
 
 diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
 index 23c8d6c..cd74e64 100644
 --- a/drivers/thermal/of-thermal.c
 +++ b/drivers/thermal/of-thermal.c
 @@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct thermal_zone_device 
 *tz, int trip)
   return 1;
  }
  
 +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *tz)
 +{
 + struct __thermal_zone *data = tz-devdata;
 + int i;
 +
 + for (i = 0; i  data-ntrips; i++)
 + if (data-trips[i].type != THERMAL_TRIP_CRITICAL)
 + continue;
 +
 + return --i;
 +}
 +



I am not against this addition. But looks like we start to spread some
specific APIs that may not be used to other drivers. Maybe having a
single API to provide a read-only copy the list / array of trips might
be a better approach. I will think of a better way.

I also request you to document it accordingly.


  static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip,
   enum thermal_trend *trend)
  {
 diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
 index ed8ff05..334a7be 100644
 --- a/drivers/thermal/thermal_core.h
 +++ b/drivers/thermal/thermal_core.h
 @@ -83,6 +83,7 @@ int of_parse_thermal_zones(void);
  void of_thermal_destroy_zones(void);
  int of_thermal_get_ntrips(struct thermal_zone_device *);
  int of_thermal_is_trip_en(struct thermal_zone_device *, int);
 +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *);
  #else
  static inline int of_parse_thermal_zones(void) { return 0; }
  static inline void of_thermal_destroy_zones(void) { }
 @@ -94,6 +95,10 @@ int of_thermal_is_trip_en(struct thermal_zone_device *, 
 int)
  {
   return 0;
  }
 +int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *)
here, it is supposed to be static inline.

 +{
 + return 0;
 +}
  #endif
  
  #endif /* __THERMAL_CORE_H__ */
 -- 
 2.0.0.rc2
 


signature.asc
Description: Digital signature


[PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points

2014-10-09 Thread Lukasz Majewski
This patch extends the of-thermal.c to provide information about number of
available non critical (i.e. non HW) trip points in the system.

Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
 drivers/thermal/of-thermal.c   | 12 
 drivers/thermal/thermal_core.h |  5 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 23c8d6c..cd74e64 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct thermal_zone_device *tz, 
int trip)
return 1;
 }
 
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *tz)
+{
+   struct __thermal_zone *data = tz-devdata;
+   int i;
+
+   for (i = 0; i  data-ntrips; i++)
+   if (data-trips[i].type != THERMAL_TRIP_CRITICAL)
+   continue;
+
+   return --i;
+}
+
 static int of_thermal_get_trend(struct thermal_zone_device *tz, int trip,
enum thermal_trend *trend)
 {
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index ed8ff05..334a7be 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -83,6 +83,7 @@ int of_parse_thermal_zones(void);
 void of_thermal_destroy_zones(void);
 int of_thermal_get_ntrips(struct thermal_zone_device *);
 int of_thermal_is_trip_en(struct thermal_zone_device *, int);
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *);
 #else
 static inline int of_parse_thermal_zones(void) { return 0; }
 static inline void of_thermal_destroy_zones(void) { }
@@ -94,6 +95,10 @@ int of_thermal_is_trip_en(struct thermal_zone_device *, int)
 {
return 0;
 }
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *)
+{
+   return 0;
+}
 #endif
 
 #endif /* __THERMAL_CORE_H__ */
-- 
2.0.0.rc2

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