NACK: [G] [PATCH 5/7] thermal/core: Remove notify ops

[Kai-Heng Feng]

On Tue, Jan 26, 2021 at 4:36 PM Stefan Bader <stefan.bader at canonical.com> wrote:
>
> On 21.01.21 09:49, Kai-Heng Feng wrote:
> > From: Daniel Lezcano <daniel.lezcano at linaro.org>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1906168
> >
> > With the removal of the notifys user in a previous patches, the ops is no
> > longer needed, remove it.
> >
> > Signed-off-by: Daniel Lezcano <daniel.lezcano at linaro.org>
> > Reviewed-by: Lukasz Luba <lukasz.luba at arm.com>
> > Link: https://lore.kernel.org/r/20201210121514.25760-5-daniel.lezcano@linaro.org
> > (cherry picked from commit 04f111130e9afa41c10d7bcec14e00e3be8b6977 linux-next)
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng at canonical.com>
> > ---
>
> Removing interfaces like this in a backport sounds rather dangerous. Upstream
> usually does this after fixing all drivers which make use of it and do not care
> about anything else. For stable releases we have to care about 3rd party drivers
> as well as potentially still having usage in the kernel. I suppose the latter
> has to some degree be tested by test compiling it but there might be drivers not
> enabled. Not that I saw any mention of a test compile.

I didn't think of the case of 3rd party module, thanks for pointing that out.

> Instead of removing the call completely, I would use something like this:
>
>         if ((!tz-ops->hot && !tz-ops->critical) && tz->ops->notify)
>                 ts->ops->notify(tz, trip, trip_type);

Actually, it makes more sense to keep the code as is.
So we can drop this one and merge the other patches.

Kai-Heng

>
> >  drivers/thermal/thermal_core.c | 3 ---
> >  include/linux/thermal.h        | 2 --
> >  2 files changed, 5 deletions(-)
> >
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index aba84e1a4396..b53305b8d643 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -401,9 +401,6 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
> >
> >       trace_thermal_zone_trip(tz, trip, trip_type);
> >
> > -     if (tz->ops->notify)
> > -             tz->ops->notify(tz, trip, trip_type);
> > -
> >       if (trip_type == THERMAL_TRIP_HOT && tz->ops->hot)
> >               tz->ops->hot(tz);
> >       else if (trip_type == THERMAL_TRIP_CRITICAL)
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index e7989cec090a..5777a9e4ea54 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -90,8 +90,6 @@ struct thermal_zone_device_ops {
> >       int (*set_emul_temp) (struct thermal_zone_device *, int);
> >       int (*get_trend) (struct thermal_zone_device *, int,
> >                         enum thermal_trend *);
> > -     int (*notify) (struct thermal_zone_device *, int,
> > -                    enum thermal_trip_type);
> >       void (*hot)(struct thermal_zone_device *);
> >       void (*critical)(struct thermal_zone_device *);
> >  };
> >
>
>