Re: [PATCH v2 02/11] thermal: add irq-mode configuration for trip point

From: Zhang Rui
Date: Wed Dec 05 2018 - 10:10:09 EST


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.zhang@xxxxxxxxx>
> Cc: Eduardo Valentin <edubezval@xxxxxxxxx>
> Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> Signed-off-by: Lukasz Luba <l.luba@xxxxxxxxxxxxxxxxxxx>
> ---
> Â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?

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 */