thermal: Hunt zero trip points thermal zones usage

  Some drivers are declaring a thermal zone without any thermal trip
points.

On the other side, we are introducing the function
thermal_zone_device_register_with_trips() which provides an array of
generic thermal trip points. When all the drivers will be converted to
the generic trip points, keeping two functions will be useless.

Most of the drivers are now using
thermal_zone_device_register_with_trips() with the generic trip
points. As soon as the remaining drivers are merged, the
thermal_zone_device_register_with_trips() will be renamed to
thermal_zone_device_register().

Obviously this renaming can only happen if there are no more user of
the thermal_zone_device_register() function.

This change uses thermal_zone_device_register_with_trips() with a NULL
parameter for the trip point array instead of
thermal_zone_device_register().

No functional change intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/power/supply/power_supply_core.c                | 2 +-
 drivers/thermal/armada_thermal.c                        | 4 ++--
 drivers/thermal/dove_thermal.c                          | 4 ++--
 drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6 +++---
 drivers/thermal/kirkwood_thermal.c                      | 4 ++--
 drivers/thermal/spear_thermal.c                         | 5 +++--
 6 files changed, 13 insertions(+), 12 deletions(-)
  

Hi Daniel,

daniel.lezcano@linaro.org wrote on Fri,  3 Feb 2023 18:58:31 +0100:

> Some drivers are declaring a thermal zone without any thermal trip
> points.
> 
> On the other side, we are introducing the function
> thermal_zone_device_register_with_trips() which provides an array of
> generic thermal trip points. When all the drivers will be converted to
> the generic trip points, keeping two functions will be useless.
> 
> Most of the drivers are now using
> thermal_zone_device_register_with_trips() with the generic trip
> points. As soon as the remaining drivers are merged, the
> thermal_zone_device_register_with_trips() will be renamed to
> thermal_zone_device_register().
> 
> Obviously this renaming can only happen if there are no more user of
> the thermal_zone_device_register() function.
> 
> This change uses thermal_zone_device_register_with_trips() with a NULL
> parameter for the trip point array instead of
> thermal_zone_device_register().
> 
> No functional change intended.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/power/supply/power_supply_core.c                | 2 +-
>  drivers/thermal/armada_thermal.c                        | 4 ++--
>  drivers/thermal/dove_thermal.c                          | 4 ++--
>  drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6 +++---
>  drivers/thermal/kirkwood_thermal.c                      | 4 ++--
>  drivers/thermal/spear_thermal.c                         | 5 +++--
>  6 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 7c790c41e2fe..208a849a71d9 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -1166,7 +1166,7 @@ static int psy_register_thermal(struct power_supply *psy)
>  
>  	/* Register battery zone device psy reports temperature */
>  	if (psy_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) {
> -		psy->tzd = thermal_zone_device_register(psy->desc->name,
> +		psy->tzd = thermal_zone_device_register_with_trips(psy->desc->name, NULL,
>  				0, 0, psy, &psy_tzd_ops, NULL, 0, 0);
>  		if (IS_ERR(psy->tzd))
>  			return PTR_ERR(psy->tzd);
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 99e86484a55c..83a4080bffc7 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -856,8 +856,8 @@ static int armada_thermal_probe(struct platform_device *pdev)
>  		/* Wait the sensors to be valid */
>  		armada_wait_sensor_validity(priv);
>  
> -		tz = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
> -						  &legacy_ops, NULL, 0, 0);
> +		tz = thermal_zone_device_register_with_trips(priv->zone_name, NULL, 0, 0, priv,
> +							     &legacy_ops, NULL, 0, 0);
>  		if (IS_ERR(tz)) {
>  			dev_err(&pdev->dev,
>  				"Failed to register thermal zone device\n");

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl
  

On 03/02/2023 18:58, Daniel Lezcano wrote:
> Some drivers are declaring a thermal zone without any thermal trip
> points.
> 
> On the other side, we are introducing the function
> thermal_zone_device_register_with_trips() which provides an array of
> generic thermal trip points. When all the drivers will be converted to
> the generic trip points, keeping two functions will be useless.
> 
> Most of the drivers are now using
> thermal_zone_device_register_with_trips() with the generic trip
> points. As soon as the remaining drivers are merged, the
> thermal_zone_device_register_with_trips() will be renamed to
> thermal_zone_device_register().
> 
> Obviously this renaming can only happen if there are no more user of
> the thermal_zone_device_register() function.
> 
> This change uses thermal_zone_device_register_with_trips() with a NULL
> parameter for the trip point array instead of
> thermal_zone_device_register().
> 
> No functional change intended.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>   drivers/power/supply/power_supply_core.c                | 2 +-

Sebastian,

are ok with this change and if it goes through the thermal tree ?


>   drivers/thermal/armada_thermal.c                        | 4 ++--
>   drivers/thermal/dove_thermal.c                          | 4 ++--
>   drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6 +++---
>   drivers/thermal/kirkwood_thermal.c                      | 4 ++--
>   drivers/thermal/spear_thermal.c                         | 5 +++--
>   6 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 7c790c41e2fe..208a849a71d9 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -1166,7 +1166,7 @@ static int psy_register_thermal(struct power_supply *psy)
>   
>   	/* Register battery zone device psy reports temperature */
>   	if (psy_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) {
> -		psy->tzd = thermal_zone_device_register(psy->desc->name,
> +		psy->tzd = thermal_zone_device_register_with_trips(psy->desc->name, NULL,
>   				0, 0, psy, &psy_tzd_ops, NULL, 0, 0);
>   		if (IS_ERR(psy->tzd))
>   			return PTR_ERR(psy->tzd);
> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
> index 99e86484a55c..83a4080bffc7 100644
> --- a/drivers/thermal/armada_thermal.c
> +++ b/drivers/thermal/armada_thermal.c
> @@ -856,8 +856,8 @@ static int armada_thermal_probe(struct platform_device *pdev)
>   		/* Wait the sensors to be valid */
>   		armada_wait_sensor_validity(priv);
>   
> -		tz = thermal_zone_device_register(priv->zone_name, 0, 0, priv,
> -						  &legacy_ops, NULL, 0, 0);
> +		tz = thermal_zone_device_register_with_trips(priv->zone_name, NULL, 0, 0, priv,
> +							     &legacy_ops, NULL, 0, 0);
>   		if (IS_ERR(tz)) {
>   			dev_err(&pdev->dev,
>   				"Failed to register thermal zone device\n");
> diff --git a/drivers/thermal/dove_thermal.c b/drivers/thermal/dove_thermal.c
> index 056622a58d00..fce15af5a7f6 100644
> --- a/drivers/thermal/dove_thermal.c
> +++ b/drivers/thermal/dove_thermal.c
> @@ -142,8 +142,8 @@ static int dove_thermal_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>   
> -	thermal = thermal_zone_device_register("dove_thermal", 0, 0,
> -					       priv, &ops, NULL, 0, 0);
> +	thermal = thermal_zone_device_register_with_trips("dove_thermal", NULL, 0, 0,
> +							  priv, &ops, NULL, 0, 0);
>   	if (IS_ERR(thermal)) {
>   		dev_err(&pdev->dev,
>   			"Failed to register thermal zone device\n");
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index d0295123cc3e..dac60b6a281c 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -609,9 +609,9 @@ static int int3400_thermal_probe(struct platform_device *pdev)
>   
>   	evaluate_odvp(priv);
>   
> -	priv->thermal = thermal_zone_device_register("INT3400 Thermal", 0, 0,
> -						priv, &int3400_thermal_ops,
> -						&int3400_thermal_params, 0, 0);
> +	priv->thermal = thermal_zone_device_register_with_trips("INT3400 Thermal", NULL, 0, 0,
> +								priv, &int3400_thermal_ops,
> +								&int3400_thermal_params, 0, 0);
>   	if (IS_ERR(priv->thermal)) {
>   		result = PTR_ERR(priv->thermal);
>   		goto free_art_trt;
> diff --git a/drivers/thermal/kirkwood_thermal.c b/drivers/thermal/kirkwood_thermal.c
> index bec7ec20e79d..4506b7dfb474 100644
> --- a/drivers/thermal/kirkwood_thermal.c
> +++ b/drivers/thermal/kirkwood_thermal.c
> @@ -74,8 +74,8 @@ static int kirkwood_thermal_probe(struct platform_device *pdev)
>   	if (IS_ERR(priv->sensor))
>   		return PTR_ERR(priv->sensor);
>   
> -	thermal = thermal_zone_device_register("kirkwood_thermal", 0, 0,
> -					       priv, &ops, NULL, 0, 0);
> +	thermal = thermal_zone_device_register_with_trips("kirkwood_thermal", NULL, 0, 0,
> +							  priv, &ops, NULL, 0, 0);
>   	if (IS_ERR(thermal)) {
>   		dev_err(&pdev->dev,
>   			"Failed to register thermal zone device\n");
> diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
> index 6a722b10d738..0d20bc9c5c0b 100644
> --- a/drivers/thermal/spear_thermal.c
> +++ b/drivers/thermal/spear_thermal.c
> @@ -122,8 +122,9 @@ static int spear_thermal_probe(struct platform_device *pdev)
>   	stdev->flags = val;
>   	writel_relaxed(stdev->flags, stdev->thermal_base);
>   
> -	spear_thermal = thermal_zone_device_register("spear_thermal", 0, 0,
> -				stdev, &ops, NULL, 0, 0);
> +	spear_thermal = thermal_zone_device_register_with_trips("spear_thermal",
> +								NULL, 0, 0,
> +								stdev, &ops, NULL, 0, 0);
>   	if (IS_ERR(spear_thermal)) {
>   		dev_err(&pdev->dev, "thermal zone device is NULL\n");
>   		ret = PTR_ERR(spear_thermal);
  

On Fri, Feb 3, 2023 at 6:59 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> Some drivers are declaring a thermal zone without any thermal trip
> points.
>
> On the other side, we are introducing the function
> thermal_zone_device_register_with_trips() which provides an array of
> generic thermal trip points. When all the drivers will be converted to
> the generic trip points, keeping two functions will be useless.
>
> Most of the drivers are now using
> thermal_zone_device_register_with_trips() with the generic trip
> points. As soon as the remaining drivers are merged, the
> thermal_zone_device_register_with_trips() will be renamed to
> thermal_zone_device_register().

So why is this the first time I'm learning about this plan?

> Obviously this renaming can only happen if there are no more user of
> the thermal_zone_device_register() function.
>
> This change uses thermal_zone_device_register_with_trips() with a NULL
> parameter for the trip point array instead of
> thermal_zone_device_register().

And later it will be renamed to thermal_zone_device_register() again?

Can we just stop confusing people this way?

What would be wrong with changing both
thermal_zone_device_register_with_trips() and
thermal_zone_device_register() together when we are ready?  And why
can't the both be replaced with something line thermal_zone_register()
doing all of the necessary things in one go?  Why do we have to make
confusing and redundant changes?
  

On Mon, Feb 6, 2023 at 5:08 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Feb 3, 2023 at 6:59 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >
> > Some drivers are declaring a thermal zone without any thermal trip
> > points.
> >
> > On the other side, we are introducing the function
> > thermal_zone_device_register_with_trips() which provides an array of
> > generic thermal trip points. When all the drivers will be converted to
> > the generic trip points, keeping two functions will be useless.
> >
> > Most of the drivers are now using
> > thermal_zone_device_register_with_trips() with the generic trip
> > points. As soon as the remaining drivers are merged, the
> > thermal_zone_device_register_with_trips() will be renamed to
> > thermal_zone_device_register().
>
> So why is this the first time I'm learning about this plan?
>
> > Obviously this renaming can only happen if there are no more user of
> > the thermal_zone_device_register() function.
> >
> > This change uses thermal_zone_device_register_with_trips() with a NULL
> > parameter for the trip point array instead of
> > thermal_zone_device_register().
>
> And later it will be renamed to thermal_zone_device_register() again?
>
> Can we just stop confusing people this way?
>
> What would be wrong with changing both
> thermal_zone_device_register_with_trips() and
> thermal_zone_device_register() together when we are ready?  And why
> can't the both be replaced with something line thermal_zone_register()
> doing all of the necessary things in one go?  Why do we have to make
> confusing and redundant changes?

Or you can define a thermal_register_tripless_zone() wrapper around
thermal_zone_device_register_with_trips() and make the drivers in
question use that, and modify them just once.
  

On 06/02/2023 17:08, Rafael J. Wysocki wrote:
> On Fri, Feb 3, 2023 at 6:59 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> Some drivers are declaring a thermal zone without any thermal trip
>> points.
>>
>> On the other side, we are introducing the function
>> thermal_zone_device_register_with_trips() which provides an array of
>> generic thermal trip points. When all the drivers will be converted to
>> the generic trip points, keeping two functions will be useless.
>>
>> Most of the drivers are now using
>> thermal_zone_device_register_with_trips() with the generic trip
>> points. As soon as the remaining drivers are merged, the
>> thermal_zone_device_register_with_trips() will be renamed to
>> thermal_zone_device_register().
> 
> So why is this the first time I'm learning about this plan?

Well it is not a plan, it looked purely logical to me that the 
_with_trips variant was added to support the generic trip points in 
addition to the specific trips. As soon as all the drivers are 
converted, there is no need to have these two functions anymore and we 
can fall back to the previous name (or a shorter one).

>> Obviously this renaming can only happen if there are no more user of
>> the thermal_zone_device_register() function.
>>
>> This change uses thermal_zone_device_register_with_trips() with a NULL
>> parameter for the trip point array instead of
>> thermal_zone_device_register().
> 
> And later it will be renamed to thermal_zone_device_register() again?

Yes, that was the idea, unify the name and then use a cocci script to 
rename them all.

> Can we just stop confusing people this way?
> 
> What would be wrong with changing both
> thermal_zone_device_register_with_trips() and
> thermal_zone_device_register() together when we are ready?  And why
> can't the both be replaced with something line thermal_zone_register()
> doing all of the necessary things in one go?  Why do we have to make
> confusing and redundant changes?

For me the result will be the same, if you prefer to wait for all the 
drivers to be converted then it is fine for me.
  

On Mon, Feb 6, 2023 at 7:00 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 06/02/2023 17:08, Rafael J. Wysocki wrote:
> > On Fri, Feb 3, 2023 at 6:59 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >>
> >> Some drivers are declaring a thermal zone without any thermal trip
> >> points.
> >>
> >> On the other side, we are introducing the function
> >> thermal_zone_device_register_with_trips() which provides an array of
> >> generic thermal trip points. When all the drivers will be converted to
> >> the generic trip points, keeping two functions will be useless.
> >>
> >> Most of the drivers are now using
> >> thermal_zone_device_register_with_trips() with the generic trip
> >> points. As soon as the remaining drivers are merged, the
> >> thermal_zone_device_register_with_trips() will be renamed to
> >> thermal_zone_device_register().
> >
> > So why is this the first time I'm learning about this plan?
>
> Well it is not a plan, it looked purely logical to me that the
> _with_trips variant was added to support the generic trip points in
> addition to the specific trips. As soon as all the drivers are
> converted, there is no need to have these two functions anymore and we
> can fall back to the previous name (or a shorter one).
>
> >> Obviously this renaming can only happen if there are no more user of
> >> the thermal_zone_device_register() function.
> >>
> >> This change uses thermal_zone_device_register_with_trips() with a NULL
> >> parameter for the trip point array instead of
> >> thermal_zone_device_register().
> >
> > And later it will be renamed to thermal_zone_device_register() again?
>
> Yes, that was the idea, unify the name and then use a cocci script to
> rename them all.
>
> > Can we just stop confusing people this way?
> >
> > What would be wrong with changing both
> > thermal_zone_device_register_with_trips() and
> > thermal_zone_device_register() together when we are ready?  And why
> > can't the both be replaced with something line thermal_zone_register()
> > doing all of the necessary things in one go?  Why do we have to make
> > confusing and redundant changes?
>
> For me the result will be the same, if you prefer to wait for all the
> drivers to be converted then it is fine for me.

Yes, please.
  

Hi,

On Fri, Feb 03, 2023 at 06:58:31PM +0100, Daniel Lezcano wrote:
> Some drivers are declaring a thermal zone without any thermal trip
> points.
> 
> On the other side, we are introducing the function
> thermal_zone_device_register_with_trips() which provides an array of
> generic thermal trip points. When all the drivers will be converted to
> the generic trip points, keeping two functions will be useless.
> 
> Most of the drivers are now using
> thermal_zone_device_register_with_trips() with the generic trip
> points. As soon as the remaining drivers are merged, the
> thermal_zone_device_register_with_trips() will be renamed to
> thermal_zone_device_register().
> 
> Obviously this renaming can only happen if there are no more user of
> the thermal_zone_device_register() function.
> 
> This change uses thermal_zone_device_register_with_trips() with a NULL
> parameter for the trip point array instead of
> thermal_zone_device_register().
> 
> No functional change intended.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/power/supply/power_supply_core.c                | 2 +-

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

[...]

> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 7c790c41e2fe..208a849a71d9 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -1166,7 +1166,7 @@ static int psy_register_thermal(struct power_supply *psy)
>  
>  	/* Register battery zone device psy reports temperature */
>  	if (psy_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) {
> -		psy->tzd = thermal_zone_device_register(psy->desc->name,
> +		psy->tzd = thermal_zone_device_register_with_trips(psy->desc->name, NULL,
>  				0, 0, psy, &psy_tzd_ops, NULL, 0, 0);
>  		if (IS_ERR(psy->tzd))
>  			return PTR_ERR(psy->tzd);

[...]

-- Sebastian