Hi Rui,
On 12/5/18 4:09 PM, Zhang Rui wrote:
> On 三, 2018-11-07 at 18:09 +0100, Lukasz Luba wrote:
>> This patch adds support irq mode in trip point.
>> When that flag is set in DT, there is no need for polling
>> in thermal framework. Crossing the trip point will rise an IRQ.
>> The naming convention for tip point 'type' can be confussing
>> and 'passive' (whic is passive cooling) might be interpretted
>> wrongly.
>>
>> This mechanism prevents from missue and adds explicit setting
>> for hardware which support interrupts for pre-configured temperature
>> threshold.
>>
>> Cc: Zhang Rui <rui.zh...@intel.com>
>> Cc: Eduardo Valentin <edubez...@gmail.com>
>> Cc: Daniel Lezcano <daniel.lezc...@linaro.org>
>> Signed-off-by: Lukasz Luba <l.l...@partner.samsung.com>
>> ---
>> drivers/thermal/of-thermal.c | 17 +++++++++++++++++
>> drivers/thermal/thermal_core.c | 10 ++++++++--
>> include/linux/thermal.h | 5 +++++
>> 3 files changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-
>> thermal.c
>> index 4bfdb4a..1a75946a 100644
>> --- a/drivers/thermal/of-thermal.c
>> +++ b/drivers/thermal/of-thermal.c
>> @@ -312,6 +312,20 @@ static int of_thermal_get_trip_type(struct
>> thermal_zone_device *tz, int trip,
>> return 0;
>> }
>>
>> +static int
>> +of_thermal_get_trip_irq_mode(struct thermal_zone_device *tz, int
>> trip,
>> + bool *mode)
>> +{
>> + struct __thermal_zone *data = tz->devdata;
>> +
>> + if (trip >= data->ntrips || trip < 0)
>> + return -EDOM;
>> +
>> + *mode = data->trips[trip].irq_mode;
>> +
>> + return 0;
>> +}
>> +
>> static int of_thermal_get_trip_temp(struct thermal_zone_device *tz,
>> int trip,
>> int *temp)
>> {
>> @@ -394,6 +408,7 @@ static struct thermal_zone_device_ops
>> of_thermal_ops = {
>> .set_mode = of_thermal_set_mode,
>>
>> .get_trip_type = of_thermal_get_trip_type,
>> + .get_trip_irq_mode = of_thermal_get_trip_irq_mode,
>> .get_trip_temp = of_thermal_get_trip_temp,
>> .set_trip_temp = of_thermal_set_trip_temp,
>> .get_trip_hyst = of_thermal_get_trip_hyst,
>> @@ -827,6 +842,8 @@ static int thermal_of_populate_trip(struct
>> device_node *np,
>> return ret;
>> }
>>
>> + trip->irq_mode = of_property_read_bool(np, "irq-mode");
>> +
>> /* Required for cooling map matching */
>> trip->np = np;
>> of_node_get(np);
>> diff --git a/drivers/thermal/thermal_core.c
>> b/drivers/thermal/thermal_core.c
>> index 39fc812..6d41e08 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -406,6 +406,7 @@ static void handle_critical_trips(struct
>> thermal_zone_device *tz,
>> static void handle_thermal_trip(struct thermal_zone_device *tz, int
>> trip)
>> {
>> enum thermal_trip_type type;
>> + bool irq_mode = false;
>>
>> /* Ignore disabled trip points */
>> if (test_bit(trip, &tz->trips_disabled))
>> @@ -419,9 +420,14 @@ static void handle_thermal_trip(struct
>> thermal_zone_device *tz, int trip)
>> handle_non_critical_trips(tz, trip);
>> /*
>> * Alright, we handled this trip successfully.
>> - * So, start monitoring again.
>> + * So, start monitoring in polling mode if
>> + * trip is not using irq HW support.
>> */
>> - monitor_thermal_zone(tz);
>> + if (tz->ops->get_trip_irq_mode)
>> + tz->ops->get_trip_irq_mode(tz, trip, &irq_mode);
>> +
>> + if (!irq_mode)
>> + monitor_thermal_zone(tz);
>> }
>>
> handle_thermal_trip() is called from thermal_zone_device_update(), and
> it is invoked for every trip points.
> say, you have a passive trip point 1 that supports irq_mode, and
> another passive trip point 2 that does not support irq_mode,
> monitor_thermal_zone() is still called in handle_thermal_trip(tz, 2),
> and the passive timer will be activated anyway, do I miss something?
Yes, the passive timer will be activated in your example. It is correct
behavior and does not break anything.
case 1: there is 'k' passive trip points which have irq_mode and 1
additional which does not have 'irq_mode', the framework will start
polling but skip check for those 'k' trip points.
case 2: all of the passive trip points have irq_mode set, the framework
will not register 'scheduled_work' because it will not call
'monitor_thermal_zone()'.
This is the case of most Exynos platforms, but there is one exception
which has 'case 1' with 2 trip points not supporting irq_mode.
Regards,
Lukasz
>
> thanks,
> rui
>
>> static void update_temperature(struct thermal_zone_device *tz)
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 5f4705f..b064565 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -103,6 +103,7 @@ struct thermal_zone_device_ops {
>> enum thermal_device_mode);
>> int (*get_trip_type) (struct thermal_zone_device *, int,
>> enum thermal_trip_type *);
>> + int (*get_trip_irq_mode) (struct thermal_zone_device *, int,
>> bool *);
>> int (*get_trip_temp) (struct thermal_zone_device *, int, int
>> *);
>> int (*set_trip_temp) (struct thermal_zone_device *, int,
>> int);
>> int (*get_trip_hyst) (struct thermal_zone_device *, int, int
>> *);
>> @@ -196,6 +197,7 @@ struct thermal_zone_device {
>> struct thermal_attr *trip_temp_attrs;
>> struct thermal_attr *trip_type_attrs;
>> struct thermal_attr *trip_hyst_attrs;
>> + struct thermal_attr *trip_irq_mode_attrs;
>> void *devdata;
>> int trips;
>> unsigned long trips_disabled; /* bitmap for disabled
>> trips */
>> @@ -364,6 +366,8 @@ struct thermal_zone_of_device_ops {
>> * @temperature: temperature value in miliCelsius
>> * @hysteresis: relative hysteresis in miliCelsius
>> * @type: trip point type
>> + * @irq_mode: to not use polling in framework set support of HW irq
>> (which will
>> + * be triggered when temperature reaches this level).
>> */
>>
>> struct thermal_trip {
>> @@ -371,6 +375,7 @@ struct thermal_trip {
>> int temperature;
>> int hysteresis;
>> enum thermal_trip_type type;
>> + bool irq_mode;
>> };
>>
>> /* Function declarations */
>
>
