diff mbox

[01/17] thermal: add thermal_zone_device_toggle() helper

Message ID 1523364131-31059-2-git-send-email-b.zolnierkie@samsung.com (mailing list archive)
State Superseded, archived
Delegated to: Eduardo Valentin
Headers show

Commit Message

Bartlomiej Zolnierkiewicz April 10, 2018, 12:41 p.m. UTC
Add thermal_zone_device_toggle() helper. Then update core code and
drivers to use it.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/thermal/hisi_thermal.c     | 14 ++------------
 drivers/thermal/of-thermal.c       |  2 +-
 drivers/thermal/rockchip_thermal.c | 22 +++++-----------------
 drivers/thermal/thermal_helpers.c  | 15 +++++++++++++++
 drivers/thermal/thermal_sysfs.c    |  8 +++++---
 include/linux/thermal.h            |  1 +
 6 files changed, 29 insertions(+), 33 deletions(-)

Comments

Eduardo Valentin Sept. 10, 2018, 5:16 p.m. UTC | #1
On Tue, Apr 10, 2018 at 02:41:55PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Add thermal_zone_device_toggle() helper. Then update core code and
> drivers to use it.

Cool, but I think it would be good to have some sort of rational here
at the commit message telling why this helper is being added, 
don't you agree?

> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
>  drivers/thermal/hisi_thermal.c     | 14 ++------------
>  drivers/thermal/of-thermal.c       |  2 +-
>  drivers/thermal/rockchip_thermal.c | 22 +++++-----------------
>  drivers/thermal/thermal_helpers.c  | 15 +++++++++++++++
>  drivers/thermal/thermal_sysfs.c    |  8 +++++---
>  include/linux/thermal.h            |  1 +
>  6 files changed, 29 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
> index 761d055..9428499 100644
> --- a/drivers/thermal/hisi_thermal.c
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -515,15 +515,6 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
>  };
>  MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
>  
> -static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
> -				       bool on)
> -{
> -	struct thermal_zone_device *tzd = sensor->tzd;
> -
> -	tzd->ops->set_mode(tzd,
> -		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> -}
> -
>  static int hisi_thermal_probe(struct platform_device *pdev)
>  {
>  	struct hisi_thermal_data *data;
> @@ -571,7 +562,7 @@ static int hisi_thermal_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	hisi_thermal_toggle_sensor(&data->sensor, true);
> +	thermal_zone_device_toggle((&data->sensor)->tzd, true);
>  
>  	return 0;
>  }
> @@ -579,9 +570,8 @@ static int hisi_thermal_probe(struct platform_device *pdev)
>  static int hisi_thermal_remove(struct platform_device *pdev)
>  {
>  	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
> -	struct hisi_thermal_sensor *sensor = &data->sensor;
>  
> -	hisi_thermal_toggle_sensor(sensor, false);
> +	thermal_zone_device_toggle((&data->sensor)->tzd, false);
>  
>  	data->disable_sensor(data);
>  
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index e09f035..f138b78 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -510,7 +510,7 @@ struct thermal_zone_device *
>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>  							 data, ops);
>  			if (!IS_ERR(tzd))
> -				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
> +				thermal_zone_device_toggle(tzd, true);
>  
>  			of_node_put(sensor_specs.np);
>  			of_node_put(child);
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index f36375d..c191e41 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -1022,15 +1022,6 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
>  };
>  MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
>  
> -static void
> -rockchip_thermal_toggle_sensor(struct rockchip_thermal_sensor *sensor, bool on)
> -{
> -	struct thermal_zone_device *tzd = sensor->tzd;
> -
> -	tzd->ops->set_mode(tzd,
> -		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> -}
> -
>  static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
>  {
>  	struct rockchip_thermal_data *thermal = dev;
> @@ -1292,7 +1283,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
>  	thermal->chip->control(thermal->regs, true);
>  
>  	for (i = 0; i < thermal->chip->chn_num; i++)
> -		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
> +		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, true);
>  
>  	platform_set_drvdata(pdev, thermal);
>  
> @@ -1311,11 +1302,8 @@ static int rockchip_thermal_remove(struct platform_device *pdev)
>  	struct rockchip_thermal_data *thermal = platform_get_drvdata(pdev);
>  	int i;
>  
> -	for (i = 0; i < thermal->chip->chn_num; i++) {
> -		struct rockchip_thermal_sensor *sensor = &thermal->sensors[i];
> -
> -		rockchip_thermal_toggle_sensor(sensor, false);
> -	}
> +	for (i = 0; i < thermal->chip->chn_num; i++)
> +		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, false);
>  
>  	thermal->chip->control(thermal->regs, false);
>  
> @@ -1332,7 +1320,7 @@ static int __maybe_unused rockchip_thermal_suspend(struct device *dev)
>  	int i;
>  
>  	for (i = 0; i < thermal->chip->chn_num; i++)
> -		rockchip_thermal_toggle_sensor(&thermal->sensors[i], false);
> +		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, false);
>  
>  	thermal->chip->control(thermal->regs, false);
>  
> @@ -1383,7 +1371,7 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev)
>  	thermal->chip->control(thermal->regs, true);
>  
>  	for (i = 0; i < thermal->chip->chn_num; i++)
> -		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
> +		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, true);
>  
>  	pinctrl_pm_select_default_state(dev);
>  
> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
> index eb03d7e..d5db101 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -227,3 +227,18 @@ int thermal_zone_get_offset(struct thermal_zone_device *tz)
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_get_offset);
> +
> +/**
> + * thermal_zone_device_toggle() - enables/disables thermal zone device
> + * @tz: a valid pointer to a struct thermal_zone_device
> + *
> + * Enables/Disables thermal zone device.
> + *
> + * Return: On success returns 0, an error code otherwise.
> + */
> +int thermal_zone_device_toggle(struct thermal_zone_device *tz, bool on)
> +{
> +	return tz->ops->set_mode(tz,
> +		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);

why not simply getting a mode type (enum thermal_device_mode)?

> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_device_toggle);

I know naming is always the hardest part :-), but to me, this function
is more a thermal_zone_set_mode() than device_toggle(). At least, based
on what I see in the code, we are not really toggling device per si,
just setting a mode.

> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index 23b5e0a..343f52b 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -72,17 +72,19 @@
>  {
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
>  	int result;
> +	bool on;
>  
>  	if (!tz->ops->set_mode)
>  		return -EPERM;
>  
>  	if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
> -		result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
> +		on = true;
>  	else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
> -		result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
> +		on = false;
>  	else
> -		result = -EINVAL;
> +		return -EINVAL;
>  
> +	result = thermal_zone_device_toggle(tz, on);
>  	if (result)
>  		return result;
>  
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 7834be6..4fbbabe 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -467,6 +467,7 @@ struct thermal_cooling_device *
>  int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
>  int thermal_zone_get_slope(struct thermal_zone_device *tz);
>  int thermal_zone_get_offset(struct thermal_zone_device *tz);
> +int thermal_zone_device_toggle(struct thermal_zone_device *tz, bool on);
>  

You missed the #else section of the #if IS_ENABLED(CONFIG_THERMAL),
didn't you?

We want to have a stub when config thermal is not set.

>  int get_tz_trend(struct thermal_zone_device *, int);
>  struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
> -- 
> 1.9.1
>
Bartlomiej Zolnierkiewicz Sept. 14, 2018, 11:40 a.m. UTC | #2
On 09/10/2018 07:16 PM, Eduardo Valentin wrote:
> On Tue, Apr 10, 2018 at 02:41:55PM +0200, Bartlomiej Zolnierkiewicz wrote:
>> Add thermal_zone_device_toggle() helper. Then update core code and
>> drivers to use it.
> 
> Cool, but I think it would be good to have some sort of rational here
> at the commit message telling why this helper is being added, 
> don't you agree?

Ok, I will enhance the patch description (the rationale is removing
code duplication and preparation for changes in the later patches from
the series).

>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> ---
>>  drivers/thermal/hisi_thermal.c     | 14 ++------------
>>  drivers/thermal/of-thermal.c       |  2 +-
>>  drivers/thermal/rockchip_thermal.c | 22 +++++-----------------
>>  drivers/thermal/thermal_helpers.c  | 15 +++++++++++++++
>>  drivers/thermal/thermal_sysfs.c    |  8 +++++---
>>  include/linux/thermal.h            |  1 +
>>  6 files changed, 29 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
>> index 761d055..9428499 100644
>> --- a/drivers/thermal/hisi_thermal.c
>> +++ b/drivers/thermal/hisi_thermal.c
>> @@ -515,15 +515,6 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
>>  };
>>  MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
>>  
>> -static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
>> -				       bool on)
>> -{
>> -	struct thermal_zone_device *tzd = sensor->tzd;
>> -
>> -	tzd->ops->set_mode(tzd,
>> -		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
>> -}
>> -
>>  static int hisi_thermal_probe(struct platform_device *pdev)
>>  {
>>  	struct hisi_thermal_data *data;
>> @@ -571,7 +562,7 @@ static int hisi_thermal_probe(struct platform_device *pdev)
>>  		}
>>  	}
>>  
>> -	hisi_thermal_toggle_sensor(&data->sensor, true);
>> +	thermal_zone_device_toggle((&data->sensor)->tzd, true);
>>  
>>  	return 0;
>>  }
>> @@ -579,9 +570,8 @@ static int hisi_thermal_probe(struct platform_device *pdev)
>>  static int hisi_thermal_remove(struct platform_device *pdev)
>>  {
>>  	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
>> -	struct hisi_thermal_sensor *sensor = &data->sensor;
>>  
>> -	hisi_thermal_toggle_sensor(sensor, false);
>> +	thermal_zone_device_toggle((&data->sensor)->tzd, false);
>>  
>>  	data->disable_sensor(data);
>>  
>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>> index e09f035..f138b78 100644
>> --- a/drivers/thermal/of-thermal.c
>> +++ b/drivers/thermal/of-thermal.c
>> @@ -510,7 +510,7 @@ struct thermal_zone_device *
>>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>>  							 data, ops);
>>  			if (!IS_ERR(tzd))
>> -				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
>> +				thermal_zone_device_toggle(tzd, true);
>>  
>>  			of_node_put(sensor_specs.np);
>>  			of_node_put(child);
>> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
>> index f36375d..c191e41 100644
>> --- a/drivers/thermal/rockchip_thermal.c
>> +++ b/drivers/thermal/rockchip_thermal.c
>> @@ -1022,15 +1022,6 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
>>  };
>>  MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
>>  
>> -static void
>> -rockchip_thermal_toggle_sensor(struct rockchip_thermal_sensor *sensor, bool on)
>> -{
>> -	struct thermal_zone_device *tzd = sensor->tzd;
>> -
>> -	tzd->ops->set_mode(tzd,
>> -		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
>> -}
>> -
>>  static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
>>  {
>>  	struct rockchip_thermal_data *thermal = dev;
>> @@ -1292,7 +1283,7 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
>>  	thermal->chip->control(thermal->regs, true);
>>  
>>  	for (i = 0; i < thermal->chip->chn_num; i++)
>> -		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
>> +		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, true);
>>  
>>  	platform_set_drvdata(pdev, thermal);
>>  
>> @@ -1311,11 +1302,8 @@ static int rockchip_thermal_remove(struct platform_device *pdev)
>>  	struct rockchip_thermal_data *thermal = platform_get_drvdata(pdev);
>>  	int i;
>>  
>> -	for (i = 0; i < thermal->chip->chn_num; i++) {
>> -		struct rockchip_thermal_sensor *sensor = &thermal->sensors[i];
>> -
>> -		rockchip_thermal_toggle_sensor(sensor, false);
>> -	}
>> +	for (i = 0; i < thermal->chip->chn_num; i++)
>> +		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, false);
>>  
>>  	thermal->chip->control(thermal->regs, false);
>>  
>> @@ -1332,7 +1320,7 @@ static int __maybe_unused rockchip_thermal_suspend(struct device *dev)
>>  	int i;
>>  
>>  	for (i = 0; i < thermal->chip->chn_num; i++)
>> -		rockchip_thermal_toggle_sensor(&thermal->sensors[i], false);
>> +		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, false);
>>  
>>  	thermal->chip->control(thermal->regs, false);
>>  
>> @@ -1383,7 +1371,7 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev)
>>  	thermal->chip->control(thermal->regs, true);
>>  
>>  	for (i = 0; i < thermal->chip->chn_num; i++)
>> -		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
>> +		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, true);
>>  
>>  	pinctrl_pm_select_default_state(dev);
>>  
>> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
>> index eb03d7e..d5db101 100644
>> --- a/drivers/thermal/thermal_helpers.c
>> +++ b/drivers/thermal/thermal_helpers.c
>> @@ -227,3 +227,18 @@ int thermal_zone_get_offset(struct thermal_zone_device *tz)
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL_GPL(thermal_zone_get_offset);
>> +
>> +/**
>> + * thermal_zone_device_toggle() - enables/disables thermal zone device
>> + * @tz: a valid pointer to a struct thermal_zone_device
>> + *
>> + * Enables/Disables thermal zone device.
>> + *
>> + * Return: On success returns 0, an error code otherwise.
>> + */
>> +int thermal_zone_device_toggle(struct thermal_zone_device *tz, bool on)
>> +{
>> +	return tz->ops->set_mode(tz,
>> +		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
> 
> why not simply getting a mode type (enum thermal_device_mode)?

Well, none of open-coded instances that thermal_zone_device_toggle()
replaces used enum thermal_device_mode. Using bool is simpler and
this enum looks a bit like over-engineering, though I can use it if
you insist.

>> +}
>> +EXPORT_SYMBOL_GPL(thermal_zone_device_toggle);
> 
> I know naming is always the hardest part :-), but to me, this function
> is more a thermal_zone_set_mode() than device_toggle(). At least, based
> on what I see in the code, we are not really toggling device per si,
> just setting a mode.

Ok, I will change it (the naming was based on existing helpers from
hisi_thermal.c and rockchip_thermal.c).

>> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
>> index 23b5e0a..343f52b 100644
>> --- a/drivers/thermal/thermal_sysfs.c
>> +++ b/drivers/thermal/thermal_sysfs.c
>> @@ -72,17 +72,19 @@
>>  {
>>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
>>  	int result;
>> +	bool on;
>>  
>>  	if (!tz->ops->set_mode)
>>  		return -EPERM;
>>  
>>  	if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
>> -		result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
>> +		on = true;
>>  	else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
>> -		result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
>> +		on = false;
>>  	else
>> -		result = -EINVAL;
>> +		return -EINVAL;
>>  
>> +	result = thermal_zone_device_toggle(tz, on);
>>  	if (result)
>>  		return result;
>>  
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 7834be6..4fbbabe 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -467,6 +467,7 @@ struct thermal_cooling_device *
>>  int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
>>  int thermal_zone_get_slope(struct thermal_zone_device *tz);
>>  int thermal_zone_get_offset(struct thermal_zone_device *tz);
>> +int thermal_zone_device_toggle(struct thermal_zone_device *tz, bool on);
>>  
> 
> You missed the #else section of the #if IS_ENABLED(CONFIG_THERMAL),
> didn't you?

No, this function shouldn't be used outside of CONFIG_THERMAL.

> We want to have a stub when config thermal is not set.

Actually it seems that a lot of these existing stubs don't make sense
outside of CONFIG_THERMAL and should be cleaned up.

>>  int get_tz_trend(struct thermal_zone_device *, int);
>>  struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Bartlomiej Zolnierkiewicz Sept. 17, 2018, 9:33 a.m. UTC | #3
On 09/14/2018 01:40 PM, Bartlomiej Zolnierkiewicz wrote:

[...]

>>> --- a/include/linux/thermal.h
>>> +++ b/include/linux/thermal.h
>>> @@ -467,6 +467,7 @@ struct thermal_cooling_device *
>>>  int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
>>>  int thermal_zone_get_slope(struct thermal_zone_device *tz);
>>>  int thermal_zone_get_offset(struct thermal_zone_device *tz);
>>> +int thermal_zone_device_toggle(struct thermal_zone_device *tz, bool on);
>>>  
>>
>> You missed the #else section of the #if IS_ENABLED(CONFIG_THERMAL),
>> didn't you?
> 
> No, this function shouldn't be used outside of CONFIG_THERMAL.

Please disregard my previous comment. I see now that the CONFIG_THERMAL=n
stub is needed and I will add it in the next patch revision.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Eduardo Valentin Sept. 24, 2018, 4:46 p.m. UTC | #4
On Mon, Sep 17, 2018 at 11:33:59AM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> On 09/14/2018 01:40 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> [...]
> 
> >>> --- a/include/linux/thermal.h
> >>> +++ b/include/linux/thermal.h
> >>> @@ -467,6 +467,7 @@ struct thermal_cooling_device *
> >>>  int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
> >>>  int thermal_zone_get_slope(struct thermal_zone_device *tz);
> >>>  int thermal_zone_get_offset(struct thermal_zone_device *tz);
> >>> +int thermal_zone_device_toggle(struct thermal_zone_device *tz, bool on);
> >>>  
> >>
> >> You missed the #else section of the #if IS_ENABLED(CONFIG_THERMAL),
> >> didn't you?
> > 
> > No, this function shouldn't be used outside of CONFIG_THERMAL.
> 
> Please disregard my previous comment. I see now that the CONFIG_THERMAL=n
> stub is needed and I will add it in the next patch revision.

Thanks Bartolomiej for keeping this up.

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

Patch

diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index 761d055..9428499 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -515,15 +515,6 @@  static int hisi_thermal_register_sensor(struct platform_device *pdev,
 };
 MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
 
-static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
-				       bool on)
-{
-	struct thermal_zone_device *tzd = sensor->tzd;
-
-	tzd->ops->set_mode(tzd,
-		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
-}
-
 static int hisi_thermal_probe(struct platform_device *pdev)
 {
 	struct hisi_thermal_data *data;
@@ -571,7 +562,7 @@  static int hisi_thermal_probe(struct platform_device *pdev)
 		}
 	}
 
-	hisi_thermal_toggle_sensor(&data->sensor, true);
+	thermal_zone_device_toggle((&data->sensor)->tzd, true);
 
 	return 0;
 }
@@ -579,9 +570,8 @@  static int hisi_thermal_probe(struct platform_device *pdev)
 static int hisi_thermal_remove(struct platform_device *pdev)
 {
 	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
-	struct hisi_thermal_sensor *sensor = &data->sensor;
 
-	hisi_thermal_toggle_sensor(sensor, false);
+	thermal_zone_device_toggle((&data->sensor)->tzd, false);
 
 	data->disable_sensor(data);
 
diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index e09f035..f138b78 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -510,7 +510,7 @@  struct thermal_zone_device *
 			tzd = thermal_zone_of_add_sensor(child, sensor_np,
 							 data, ops);
 			if (!IS_ERR(tzd))
-				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
+				thermal_zone_device_toggle(tzd, true);
 
 			of_node_put(sensor_specs.np);
 			of_node_put(child);
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index f36375d..c191e41 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -1022,15 +1022,6 @@  static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
 };
 MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
 
-static void
-rockchip_thermal_toggle_sensor(struct rockchip_thermal_sensor *sensor, bool on)
-{
-	struct thermal_zone_device *tzd = sensor->tzd;
-
-	tzd->ops->set_mode(tzd,
-		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
-}
-
 static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
 {
 	struct rockchip_thermal_data *thermal = dev;
@@ -1292,7 +1283,7 @@  static int rockchip_thermal_probe(struct platform_device *pdev)
 	thermal->chip->control(thermal->regs, true);
 
 	for (i = 0; i < thermal->chip->chn_num; i++)
-		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
+		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, true);
 
 	platform_set_drvdata(pdev, thermal);
 
@@ -1311,11 +1302,8 @@  static int rockchip_thermal_remove(struct platform_device *pdev)
 	struct rockchip_thermal_data *thermal = platform_get_drvdata(pdev);
 	int i;
 
-	for (i = 0; i < thermal->chip->chn_num; i++) {
-		struct rockchip_thermal_sensor *sensor = &thermal->sensors[i];
-
-		rockchip_thermal_toggle_sensor(sensor, false);
-	}
+	for (i = 0; i < thermal->chip->chn_num; i++)
+		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, false);
 
 	thermal->chip->control(thermal->regs, false);
 
@@ -1332,7 +1320,7 @@  static int __maybe_unused rockchip_thermal_suspend(struct device *dev)
 	int i;
 
 	for (i = 0; i < thermal->chip->chn_num; i++)
-		rockchip_thermal_toggle_sensor(&thermal->sensors[i], false);
+		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, false);
 
 	thermal->chip->control(thermal->regs, false);
 
@@ -1383,7 +1371,7 @@  static int __maybe_unused rockchip_thermal_resume(struct device *dev)
 	thermal->chip->control(thermal->regs, true);
 
 	for (i = 0; i < thermal->chip->chn_num; i++)
-		rockchip_thermal_toggle_sensor(&thermal->sensors[i], true);
+		thermal_zone_device_toggle((&thermal->sensors[i])->tzd, true);
 
 	pinctrl_pm_select_default_state(dev);
 
diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
index eb03d7e..d5db101 100644
--- a/drivers/thermal/thermal_helpers.c
+++ b/drivers/thermal/thermal_helpers.c
@@ -227,3 +227,18 @@  int thermal_zone_get_offset(struct thermal_zone_device *tz)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_offset);
+
+/**
+ * thermal_zone_device_toggle() - enables/disables thermal zone device
+ * @tz: a valid pointer to a struct thermal_zone_device
+ *
+ * Enables/Disables thermal zone device.
+ *
+ * Return: On success returns 0, an error code otherwise.
+ */
+int thermal_zone_device_toggle(struct thermal_zone_device *tz, bool on)
+{
+	return tz->ops->set_mode(tz,
+		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
+}
+EXPORT_SYMBOL_GPL(thermal_zone_device_toggle);
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 23b5e0a..343f52b 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -72,17 +72,19 @@ 
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
 	int result;
+	bool on;
 
 	if (!tz->ops->set_mode)
 		return -EPERM;
 
 	if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
-		result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
+		on = true;
 	else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
-		result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
+		on = false;
 	else
-		result = -EINVAL;
+		return -EINVAL;
 
+	result = thermal_zone_device_toggle(tz, on);
 	if (result)
 		return result;
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 7834be6..4fbbabe 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -467,6 +467,7 @@  struct thermal_cooling_device *
 int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
 int thermal_zone_get_slope(struct thermal_zone_device *tz);
 int thermal_zone_get_offset(struct thermal_zone_device *tz);
+int thermal_zone_device_toggle(struct thermal_zone_device *tz, bool on);
 
 int get_tz_trend(struct thermal_zone_device *, int);
 struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,