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 <[email protected]>
> ---
>  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

Reply via email to