On Wed, Oct 30, 2019 at 12:21 PM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:
>
> On 16/10/2019 01:13, Amit Kucheria wrote:
> > Currently, in order to enable interrupt-only mode, one must set
> > polling-delay-passive and polling-delay properties in the DT to 0,
> > otherwise the thermal framework will continue to setup a periodic timers
> > to monitor the thermal zones.
> >
> > Change the behaviour, so that on DT-based systems, we no longer have to
> > set the properties to zero if we find an 'interrupt' property in the
> > sensor.
> >
> > Following data shows the number of times
> > thermal_zone_device_set_polling() is invoked with and without this
> > patch. So the patch achieves the same behaviour as setting the delay
> > properties to 0.
> >
> > Current behaviour (without setting delay properties to 0):
> > FUNC COUNT
> > thermal_zone_device_update 302
> > thermal_zone_device_set_pollin 7911
> >
> > Current behaviour (with delay properties set to 0):
> > FUNC COUNT
> > thermal_zone_device_update 3
> > thermal_zone_device_set_pollin 6
> >
> > With this patch (without setting delay properties to 0):
> > FUNC COUNT
> > thermal_zone_device_update 3
> > thermal_zone_device_set_pollin 6
> >
> > Suggested-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> > Signed-off-by: Amit Kucheria <amit.kucheria@xxxxxxxxxx>
> > ---
> > drivers/thermal/of-thermal.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> > index dc5093be553e..79ad587462b1 100644
> > --- a/drivers/thermal/of-thermal.c
> > +++ b/drivers/thermal/of-thermal.c
> > @@ -412,7 +412,8 @@ static struct thermal_zone_device_ops of_thermal_ops = {
> > static struct thermal_zone_device *
> > thermal_zone_of_add_sensor(struct device_node *zone,
> > struct device_node *sensor, void *data,
> > - const struct thermal_zone_of_device_ops *ops)
> > + const struct thermal_zone_of_device_ops *ops,
> > + bool force_interrupts)
> > {
> > struct thermal_zone_device *tzd;
> > struct __thermal_zone *tz;
> > @@ -433,6 +434,11 @@ thermal_zone_of_add_sensor(struct device_node *zone,
> > tzd->ops->get_temp = of_thermal_get_temp;
> > tzd->ops->get_trend = of_thermal_get_trend;
> >
> > + if (force_interrupts) {
> > + tz->passive_delay = 0;
> > + tz->polling_delay = 0;
> > + }
> > +
> > /*
> > * The thermal zone core will calculate the window if they have set the
> > * optional set_trips pointer.
> > @@ -486,6 +492,7 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
> > {
> > struct device_node *np, *child, *sensor_np;
> > struct thermal_zone_device *tzd = ERR_PTR(-ENODEV);
> > + bool force_interrupts = false;
> >
> > np = of_find_node_by_name(NULL, "thermal-zones");
> > if (!np)
> > @@ -498,6 +505,9 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
> >
> > sensor_np = of_node_get(dev->of_node);
> >
> > + if (of_find_property(sensor_np, "interrupts", NULL))
> > + force_interrupts = true;
> > +
>
> This is hackish. It does cover only DT drivers.
OK.
> It would be cleaner to add a specific flag describing the thermal sensor
> driver mode and then in the core code do not start the timers if the
> associated works in interrupt.
>
> Moreover the interrupt mode can be used to activate faster the passive
> delay monitoring after reaching a threshold like the hikey and hikey960.
> So this patch will disable the mitigation on those boards. This is
> another argument to add flags for the thermal sensor mode.
>
So each driver then needs to set this flag at interrupt registration
time. Or do you want to set that automatically in core code with some
wrapper function?
Regards,
Amit
