[PATCH v4 08/11] thermal: Explicitly enable non-changing thermal zone devices

[Bartlomiej Zolnierkiewicz]

On 5/28/20 9:20 PM, Andrzej Pietrasiewicz wrote:
> Some thermal zone devices never change their state, so they should be
> always enabled.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p at collabora.com>
> ---
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c     |  8 ++++++++
>  drivers/net/wireless/intel/iwlwifi/mvm/tt.c            |  9 ++++++++-
>  drivers/platform/x86/intel_mid_thermal.c               |  6 ++++++
>  drivers/power/supply/power_supply_core.c               |  9 +++++++--
>  drivers/thermal/armada_thermal.c                       |  6 ++++++
>  drivers/thermal/dove_thermal.c                         |  6 ++++++
>  .../thermal/intel/int340x_thermal/int3400_thermal.c    |  5 +++++
>  .../intel/int340x_thermal/int340x_thermal_zone.c       |  5 +++++
>  drivers/thermal/intel/intel_pch_thermal.c              |  5 +++++
>  drivers/thermal/intel/intel_soc_dts_iosf.c             |  3 +++
>  drivers/thermal/intel/x86_pkg_temp_thermal.c           |  6 ++++++
>  drivers/thermal/kirkwood_thermal.c                     |  7 +++++++
>  drivers/thermal/rcar_thermal.c                         |  9 ++++++++-
>  drivers/thermal/spear_thermal.c                        |  7 +++++++
>  drivers/thermal/st/st_thermal.c                        |  5 +++++
>  drivers/thermal/thermal_of.c                           | 10 +++++++++-
>  16 files changed, 101 insertions(+), 5 deletions(-)

[...]

> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 3c0397a29b8c..8e8c9af7e5f4 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -485,6 +485,10 @@ static int int3400_thermal_probe(struct platform_device *pdev)
>  		goto free_art_trt;
>  	}
>  
> +	result = thermal_zone_device_enable(priv->thermal);

I'm not sure about correctness of this addition.

This driver contains ->set_mode but doesn't call it on initialization
(in v3 it was using THERMAL_DEVICE_DISABLED as .initial_mode parameter).

> +	if (result)
> +		goto free_tzd;
> +
>  	priv->rel_misc_dev_res = acpi_thermal_rel_misc_device_add(
>  							priv->adev->handle);
>  
> @@ -518,6 +522,7 @@ static int int3400_thermal_probe(struct platform_device *pdev)
>  free_rel_misc:
>  	if (!priv->rel_misc_dev_res)
>  		acpi_thermal_rel_misc_device_remove(priv->adev->handle);
> +free_tzd:
>  	thermal_zone_device_unregister(priv->thermal);
>  free_art_trt:
>  	kfree(priv->trts);

[...]

> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 43a516a35d64..011fd7f0a01e 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -1066,7 +1066,7 @@ int __init of_parse_thermal_zones(void)
>  	for_each_available_child_of_node(np, child) {
>  		struct thermal_zone_device *zone;
>  		struct thermal_zone_params *tzp;
> -		int i, mask = 0;
> +		int i, ret, mask = 0;
>  		u32 prop;
>  
>  		tz = thermal_of_build_thermal_zone(child);
> @@ -1113,6 +1113,14 @@ int __init of_parse_thermal_zones(void)
>  			of_thermal_free_zone(tz);
>  			/* attempting to build remaining zones still */
>  		}
> +		ret = thermal_zone_device_enable(zone);

This doesn't seem correct as it is done too early and
there is already proper thermal_zone_device_enable() call
in thermal_zone_of_sensor_register().

> +		if (ret) {
> +			thermal_zone_device_unregister(zone);
> +			pr_err("Failed to enable thermal zone\n");
> +			kfree(tzp);
> +			kfree(ops);
> +			of_thermal_free_zone(tz);
> +		}
>  	}
>  	of_node_put(np);
>  

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics