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

Stefan Bader stefan.bader at canonical.com
Tue Jan 26 08:36:16 UTC 2021


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.
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);

>  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 *);
>  };
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210126/90737e25/attachment.sig>


More information about the kernel-team mailing list