On Tue, 18 Nov 2014 11:20:26 -0400
Eduardo Valentin [off-list ref] wrote:

Hello Lukasz,

On Wed, Nov 12, 2014 at 10:42:41AM +0100, Lukasz Majewski wrote:

quoted

Hi Eduardo,

quoted

Hello Lukasz,

On Fri, Nov 07, 2014 at 11:05:51AM +0100, Lukasz Majewski wrote:

quoted

Hi Eduardo,

quoted

On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski
wrote:

quoted

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 <redacted>
---
 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.

quoted

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).

quoted

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

quoted

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.

quoted

quoted

quoted

quoted

I also request you to document it accordingly.

Ok, I will do that.

Cool!

quoted

quoted

quoted

 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.

quoted

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

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141118/bfb70aa8/attachment.sig>