Re: [PATCH] of-thermal: Disable polling when interrupt property is found in DT

[Amit Kucheria]

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