[PATCH v2 1/2] acpi: thermal: update thermal_zone after enable the driver.

[PATCH v2 1/2] acpi: thermal: update thermal_zone after enable the driver.

[Enric Balletbo i Serra]

Use thermal_set_mode instead of just set the tz_enable variable when
enabling the ACPI thermal driver. The purpose of this change is trigger
a thermal_zone_device_update when driver switches from disabled to
enabled mode so thermal_zone data is up-to-date.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
This patch is new from v1 [1]

[1] https://patchwork.kernel.org/patch/9804229/

 drivers/acpi/thermal.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 1d0417b..9949458 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -930,7 +930,9 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	tz->tz_enabled = 1;
+	result = thermal_set_mode(tz->thermal_zone, THERMAL_DEVICE_ENABLED);
+	if (result)
+		return result;
 
 	dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
 		 tz->thermal_zone->id);
-- 
2.9.3

[PATCH v2 2/2] thermal: core: Allow to disable polling when disabling thermal zone.

[Enric Balletbo i Serra]

Under each thermal zone there is a optional file called "mode". Writing
enabled or disabled to this file allows a given thermal zone to be enabled
or disabled, but in current code, the monitoring queue doesn't stops. Add
the code to disable polling when disabling thermal zone and enable polling
when enabling the thermal zone.

This patch is based on the original Sameer Nanda <snanda@chromium.org>
patch that implemented this idea for the ACPI thermal driver.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v1:
 - Implement in thermal subsystem instead of ACPI thermal driver (Zhang Rui)

v1: https://patchwork.kernel.org/patch/9804229/

 drivers/thermal/thermal_core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 5a51c74..869c8f3 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -292,6 +292,17 @@ static void thermal_unregister_governors(void)
 static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
 					    int delay)
 {
+	enum thermal_device_mode mode;
+
+	if (tz->ops->get_mode) {
+		/* When the thermal zone is disabled stop the polling */
+		tz->ops->get_mode(tz, &mode);
+		if (mode == THERMAL_DEVICE_DISABLED) {
+			cancel_delayed_work(&tz->poll_queue);
+			return;
+		}
+	}
+
 	if (delay > 1000)
 		mod_delayed_work(system_freezable_wq, &tz->poll_queue,
 				 round_jiffies(msecs_to_jiffies(delay)));
-- 
2.9.3

Re: [PATCH v2 1/2] acpi: thermal: update thermal_zone after enable the driver.

[Zhang Rui]

On Thu, 2017-06-29 at 18:50 +0200, Enric Balletbo i Serra wrote:
> Use thermal_set_mode instead of just set the tz_enable variable when
> enabling the ACPI thermal driver. The purpose of this change is
> trigger
> a thermal_zone_device_update when driver switches from disabled to
> enabled mode so thermal_zone data is up-to-date.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> This patch is new from v1 [1]
> 
> [1] https://patchwork.kernel.org/patch/9804229/
> 
>  drivers/acpi/thermal.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 1d0417b..9949458 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -930,7 +930,9 @@ static int
> acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>  	if (ACPI_FAILURE(status))
>  		return -ENODEV;
>  
> -	tz->tz_enabled = 1;
> +	result = thermal_set_mode(tz->thermal_zone,
> THERMAL_DEVICE_ENABLED);
> +	if (result)
> +		return result;

thermal core is responsible for checking the thermal zone "mode", and
set the polling properly, right after thermal_zone_device_being
registered.

Thus we need to do nothing, but just make sure tz->tz_enabled is set
properly before registering the zone.

thanks,
rui
>  
>  	dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
>  		 tz->thermal_zone->id);

Re: [PATCH v2 2/2] thermal: core: Allow to disable polling when disabling thermal zone.

[Zhang Rui]

On Thu, 2017-06-29 at 18:50 +0200, Enric Balletbo i Serra wrote:
> Under each thermal zone there is a optional file called "mode".
> Writing
> enabled or disabled to this file allows a given thermal zone to be
> enabled
> or disabled, but in current code, the monitoring queue doesn't stops.
> Add
> the code to disable polling when disabling thermal zone and enable
> polling
> when enabling the thermal zone.
> 
> This patch is based on the original Sameer Nanda <snanda@chromium.org
> >
> patch that implemented this idea for the ACPI thermal driver.
> 

Before these two patches, only platform thermal driver cares about
"mode", thermal core does nothing but invokes platform .set_mode()
callback upon sysfs I/F write.
But after this patch set, "mode" becomes something that we should
take into account in thermal core as well.
Thus, IMO, we have a couple of things more to do, like the prototype
patch attached, which I have not tested yet.

>From 8bf51fe65bd386b7e8c3c2ca8b9f7d321c92b22f Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Fri, 30 Jun 2017 11:11:45 +0800
Subject: [RFC PATCH] Thermal: introduce thermal zone device mode control

Thermal "mode" sysfs attribute is introduced to enable/disable a thermal zone,
and .get_mode()/.set_mode() callback is introduced for platform thermal driver
to enable/disable the hardware thermal control logic. And thermal core takes
no action upon thermal zone enable/disable.

Actually, this is not quite right because thermal core still pokes those
disabled thermal zones occasionally, e.g. upon system resume.

To fix this, a new flag 'mode' is introduced in struct thermal_zone_device
to represent the thermal zone mode, and several decisions have been made
based on this flag, including
1. check the thermal zone mode right after it's registered.
2. skip updating thermal zone if the zone is disabled
3. stop the polling timer when the thermal zone is disabled

Note: basically, this patch doesn't affect the existing always-enabled
thermal zones much, with just one exception -
thermal zone .get_mode() must be well prepared to reflect the real thermal
zone status upon the thermal zone registration.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal_core.c  | 37 +++++++++++++++++++++++++++++++++++--
 drivers/thermal/thermal_sysfs.c | 22 ++++++----------------
 include/linux/thermal.h         |  3 +++
 3 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 5a51c74..89b2254 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -306,9 +306,9 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz)
 {
 	mutex_lock(&tz->lock);
 
-	if (tz->passive)
+	if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->passive)
 		thermal_zone_device_set_polling(tz, tz->passive_delay);
-	else if (tz->polling_delay)
+	else if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->polling_delay)
 		thermal_zone_device_set_polling(tz, tz->polling_delay);
 	else
 		thermal_zone_device_set_polling(tz, 0);
@@ -464,11 +464,35 @@ static void thermal_zone_device_reset(struct thermal_zone_device *tz)
 		pos->initialized = false;
 }
 
+int thermal_zone_set_mode(struct thermal_zone_device *tz,
+				 enum thermal_device_mode mode)
+{
+	int result;
+
+	if (!tz->ops->set_mode)
+		return -EPERM;
+
+	result = tz->ops->set_mode(tz, mode);
+	if (result)
+		return result;
+
+	if (tz->mode != mode) {
+		tz->mode = mode;
+		monitor_thermal_zone(tz);
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(thermal_zone_set_mode);
+
 void thermal_zone_device_update(struct thermal_zone_device *tz,
 				enum thermal_notify_event event)
 {
 	int count;
 
+	/* Do nothing if the thermal zone is disabled */
+	if (tz->mode == THERMAL_DEVICE_DISABLED)
+		return;
+
 	if (atomic_read(&in_suspend))
 		return;
 
@@ -1287,6 +1311,15 @@ thermal_zone_device_register(const char *type, int trips, int mask,
 	INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check);
 
 	thermal_zone_device_reset(tz);
+
+	if (tz->ops->get_mode()) {
+		enum thermal_device_mode mode;
+
+		result = tz->ops->get_mode(tz, &mode);
+		tz->mode = result ? THERMAL_DEVICE_ENABLED : mode;
+	} else
+		tz->mode = THERMAL_DEVICE_ENABLED;
+
 	/* Update the new thermal zone and mark it as already updated. */
 	if (atomic_cmpxchg(&tz->need_update, 1, 0))
 		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index a694de9..95d2587 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -51,17 +51,8 @@ static ssize_t
 mode_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	enum thermal_device_mode mode;
-	int result;
-
-	if (!tz->ops->get_mode)
-		return -EPERM;
 
-	result = tz->ops->get_mode(tz, &mode);
-	if (result)
-		return result;
-
-	return sprintf(buf, "%s\n", mode == THERMAL_DEVICE_ENABLED ? "enabled"
+	return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ? "enabled"
 		       : "disabled");
 }
 
@@ -70,18 +61,17 @@ mode_store(struct device *dev, struct device_attribute *attr,
 	   const char *buf, size_t count)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	enum thermal_device_mode mode;
 	int result;
 
-	if (!tz->ops->set_mode)
-		return -EPERM;
-
 	if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
-		result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
+		mode = THERMAL_DEVICE_ENABLED;
 	else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
-		result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
+		mode = THERMAL_DEVICE_DISABLED;
 	else
-		result = -EINVAL;
+		return -EINVAL;
 
+	result = thermal_zone_set_mode(tz, mode)
 	if (result)
 		return result;
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index dab11f9..2f427de 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -210,6 +210,7 @@ struct thermal_zone_device {
 	struct thermal_attr *trip_type_attrs;
 	struct thermal_attr *trip_hyst_attrs;
 	void *devdata;
+	enum thermal_device_mode mode;
 	int trips;
 	unsigned long trips_disabled;	/* bitmap for disabled trips */
 	int passive_delay;
@@ -465,6 +466,8 @@ struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
 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_set_mode(struct thermal_zone_device *tz,
+			  enum thermal_device_mode mode);
 
 int get_tz_trend(struct thermal_zone_device *, int);
 struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
-- 
2.7.4

Re: [PATCH v2 2/2] thermal: core: Allow to disable polling when disabling thermal zone.

[Enric Balletbo Serra]

Hi Rui,

2017-06-30 7:05 GMT+02:00 Zhang Rui <rui.zhang@intel.com>:
> On Thu, 2017-06-29 at 18:50 +0200, Enric Balletbo i Serra wrote:
>> Under each thermal zone there is a optional file called "mode".
>> Writing
>> enabled or disabled to this file allows a given thermal zone to be
>> enabled
>> or disabled, but in current code, the monitoring queue doesn't stops.
>> Add
>> the code to disable polling when disabling thermal zone and enable
>> polling
>> when enabling the thermal zone.
>>
>> This patch is based on the original Sameer Nanda <snanda@chromium.org
>> >
>> patch that implemented this idea for the ACPI thermal driver.
>>
>
> Before these two patches, only platform thermal driver cares about
> "mode", thermal core does nothing but invokes platform .set_mode()
> callback upon sysfs I/F write.
> But after this patch set, "mode" becomes something that we should
> take into account in thermal core as well.
> Thus, IMO, we have a couple of things more to do, like the prototype
> patch attached, which I have not tested yet.
>
> From 8bf51fe65bd386b7e8c3c2ca8b9f7d321c92b22f Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Fri, 30 Jun 2017 11:11:45 +0800
> Subject: [RFC PATCH] Thermal: introduce thermal zone device mode control
>
> Thermal "mode" sysfs attribute is introduced to enable/disable a thermal zone,
> and .get_mode()/.set_mode() callback is introduced for platform thermal driver
> to enable/disable the hardware thermal control logic. And thermal core takes
> no action upon thermal zone enable/disable.
>
> Actually, this is not quite right because thermal core still pokes those
> disabled thermal zones occasionally, e.g. upon system resume.
>
> To fix this, a new flag 'mode' is introduced in struct thermal_zone_device
> to represent the thermal zone mode, and several decisions have been made
> based on this flag, including
> 1. check the thermal zone mode right after it's registered.
> 2. skip updating thermal zone if the zone is disabled
> 3. stop the polling timer when the thermal zone is disabled
>
> Note: basically, this patch doesn't affect the existing always-enabled
> thermal zones much, with just one exception -
> thermal zone .get_mode() must be well prepared to reflect the real thermal
> zone status upon the thermal zone registration.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/thermal/thermal_core.c  | 37 +++++++++++++++++++++++++++++++++++--
>  drivers/thermal/thermal_sysfs.c | 22 ++++++----------------
>  include/linux/thermal.h         |  3 +++
>  3 files changed, 44 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 5a51c74..89b2254 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -306,9 +306,9 @@ static void monitor_thermal_zone(struct thermal_zone_device *tz)
>  {
>         mutex_lock(&tz->lock);
>
> -       if (tz->passive)
> +       if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->passive)

tz->mode

>                 thermal_zone_device_set_polling(tz, tz->passive_delay);
> -       else if (tz->polling_delay)
> +       else if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->polling_delay)

tz->mode

>                 thermal_zone_device_set_polling(tz, tz->polling_delay);
>         else
>                 thermal_zone_device_set_polling(tz, 0);
> @@ -464,11 +464,35 @@ static void thermal_zone_device_reset(struct thermal_zone_device *tz)
>                 pos->initialized = false;
>  }
>
> +int thermal_zone_set_mode(struct thermal_zone_device *tz,
> +                                enum thermal_device_mode mode)
> +{
> +       int result;
> +
> +       if (!tz->ops->set_mode)
> +               return -EPERM;
> +
> +       result = tz->ops->set_mode(tz, mode);
> +       if (result)
> +               return result;
> +
> +       if (tz->mode != mode) {
> +               tz->mode = mode;
> +               monitor_thermal_zone(tz);
> +       }
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_set_mode);
> +
>  void thermal_zone_device_update(struct thermal_zone_device *tz,
>                                 enum thermal_notify_event event)
>  {
>         int count;
>
> +       /* Do nothing if the thermal zone is disabled */
> +       if (tz->mode == THERMAL_DEVICE_DISABLED)
> +               return;
> +
>         if (atomic_read(&in_suspend))
>                 return;
>
> @@ -1287,6 +1311,15 @@ thermal_zone_device_register(const char *type, int trips, int mask,
>         INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check);
>
>         thermal_zone_device_reset(tz);
> +
> +       if (tz->ops->get_mode()) {

remove the ()

> +               enum thermal_device_mode mode;
> +
> +               result = tz->ops->get_mode(tz, &mode);
> +               tz->mode = result ? THERMAL_DEVICE_ENABLED : mode;
> +       } else
> +               tz->mode = THERMAL_DEVICE_ENABLED;
> +
>         /* Update the new thermal zone and mark it as already updated. */
>         if (atomic_cmpxchg(&tz->need_update, 1, 0))
>                 thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index a694de9..95d2587 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -51,17 +51,8 @@ static ssize_t
>  mode_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
> -       enum thermal_device_mode mode;
> -       int result;
> -
> -       if (!tz->ops->get_mode)
> -               return -EPERM;
>
> -       result = tz->ops->get_mode(tz, &mode);
> -       if (result)
> -               return result;
> -
> -       return sprintf(buf, "%s\n", mode == THERMAL_DEVICE_ENABLED ? "enabled"
> +       return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ? "enabled"
>                        : "disabled");
>  }
>
> @@ -70,18 +61,17 @@ mode_store(struct device *dev, struct device_attribute *attr,
>            const char *buf, size_t count)
>  {
>         struct thermal_zone_device *tz = to_thermal_zone(dev);
> +       enum thermal_device_mode mode;
>         int result;
>
> -       if (!tz->ops->set_mode)
> -               return -EPERM;
> -
>         if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
> -               result = tz->ops->set_mode(tz, THERMAL_DEVICE_ENABLED);
> +               mode = THERMAL_DEVICE_ENABLED;
>         else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
> -               result = tz->ops->set_mode(tz, THERMAL_DEVICE_DISABLED);
> +               mode = THERMAL_DEVICE_DISABLED;
>         else
> -               result = -EINVAL;
> +               return -EINVAL;
>
> +       result = thermal_zone_set_mode(tz, mode)
>         if (result)
>                 return result;
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index dab11f9..2f427de 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -210,6 +210,7 @@ struct thermal_zone_device {
>         struct thermal_attr *trip_type_attrs;
>         struct thermal_attr *trip_hyst_attrs;
>         void *devdata;
> +       enum thermal_device_mode mode;
>         int trips;
>         unsigned long trips_disabled;   /* bitmap for disabled trips */
>         int passive_delay;
> @@ -465,6 +466,8 @@ struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
>  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_set_mode(struct thermal_zone_device *tz,
> +                         enum thermal_device_mode mode);
>
>  int get_tz_trend(struct thermal_zone_device *, int);
>  struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
> --
> 2.7.4
>

There are some build issues but once fixed the patch works as
expected, many thanks.

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Rui, do you want I send a fixed version of this patch? Or do you want
to fix yourself?

Best regards,
  Enric

Re: [PATCH v2 2/2] thermal: core: Allow to disable polling when disabling thermal zone.

[Zhang Rui]

Hi, Enric,

On Fri, 2017-06-30 at 10:15 +0200, Enric Balletbo Serra wrote:
> Hi Rui,
> 
> 2017-06-30 7:05 GMT+02:00 Zhang Rui <rui.zhang@intel.com>:
> > 
> > On Thu, 2017-06-29 at 18:50 +0200, Enric Balletbo i Serra wrote:
> > > 
> > > Under each thermal zone there is a optional file called "mode".
> > > Writing
> > > enabled or disabled to this file allows a given thermal zone to
> > > be
> > > enabled
> > > or disabled, but in current code, the monitoring queue doesn't
> > > stops.
> > > Add
> > > the code to disable polling when disabling thermal zone and
> > > enable
> > > polling
> > > when enabling the thermal zone.
> > > 
> > > This patch is based on the original Sameer Nanda <snanda@chromium
> > > .org
> > > > 
> > > > 
> > > patch that implemented this idea for the ACPI thermal driver.
> > > 
> > Before these two patches, only platform thermal driver cares about
> > "mode", thermal core does nothing but invokes platform .set_mode()
> > callback upon sysfs I/F write.
> > But after this patch set, "mode" becomes something that we should
> > take into account in thermal core as well.
> > Thus, IMO, we have a couple of things more to do, like the
> > prototype
> > patch attached, which I have not tested yet.
> > 
> > From 8bf51fe65bd386b7e8c3c2ca8b9f7d321c92b22f Mon Sep 17 00:00:00
> > 2001
> > From: Zhang Rui <rui.zhang@intel.com>
> > Date: Fri, 30 Jun 2017 11:11:45 +0800
> > Subject: [RFC PATCH] Thermal: introduce thermal zone device mode
> > control
> > 
> > Thermal "mode" sysfs attribute is introduced to enable/disable a
> > thermal zone,
> > and .get_mode()/.set_mode() callback is introduced for platform
> > thermal driver
> > to enable/disable the hardware thermal control logic. And thermal
> > core takes
> > no action upon thermal zone enable/disable.
> > 
> > Actually, this is not quite right because thermal core still pokes
> > those
> > disabled thermal zones occasionally, e.g. upon system resume.
> > 
> > To fix this, a new flag 'mode' is introduced in struct
> > thermal_zone_device
> > to represent the thermal zone mode, and several decisions have been
> > made
> > based on this flag, including
> > 1. check the thermal zone mode right after it's registered.
> > 2. skip updating thermal zone if the zone is disabled
> > 3. stop the polling timer when the thermal zone is disabled
> > 
> > Note: basically, this patch doesn't affect the existing always-
> > enabled
> > thermal zones much, with just one exception -
> > thermal zone .get_mode() must be well prepared to reflect the real
> > thermal
> > zone status upon the thermal zone registration.
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/thermal/thermal_core.c  | 37
> > +++++++++++++++++++++++++++++++++++--
> >  drivers/thermal/thermal_sysfs.c | 22 ++++++----------------
> >  include/linux/thermal.h         |  3 +++
> >  3 files changed, 44 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/thermal/thermal_core.c
> > b/drivers/thermal/thermal_core.c
> > index 5a51c74..89b2254 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -306,9 +306,9 @@ static void monitor_thermal_zone(struct
> > thermal_zone_device *tz)
> >  {
> >         mutex_lock(&tz->lock);
> > 
> > -       if (tz->passive)
> > +       if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->passive)
> tz->mode
> 
> > 
> >                 thermal_zone_device_set_polling(tz, tz-
> > >passive_delay);
> > -       else if (tz->polling_delay)
> > +       else if (tz->enabled == THERMAL_DEVICE_ENABLED && tz-
> > >polling_delay)
> tz->mode
> 
> > 
> >                 thermal_zone_device_set_polling(tz, tz-
> > >polling_delay);
> >         else
> >                 thermal_zone_device_set_polling(tz, 0);
> > @@ -464,11 +464,35 @@ static void thermal_zone_device_reset(struct
> > thermal_zone_device *tz)
> >                 pos->initialized = false;
> >  }
> > 
> > +int thermal_zone_set_mode(struct thermal_zone_device *tz,
> > +                                enum thermal_device_mode mode)
> > +{
> > +       int result;
> > +
> > +       if (!tz->ops->set_mode)
> > +               return -EPERM;
> > +
> > +       result = tz->ops->set_mode(tz, mode);
> > +       if (result)
> > +               return result;
> > +
> > +       if (tz->mode != mode) {
> > +               tz->mode = mode;
> > +               monitor_thermal_zone(tz);
> > +       }
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(thermal_zone_set_mode);
> > +
> >  void thermal_zone_device_update(struct thermal_zone_device *tz,
> >                                 enum thermal_notify_event event)
> >  {
> >         int count;
> > 
> > +       /* Do nothing if the thermal zone is disabled */
> > +       if (tz->mode == THERMAL_DEVICE_DISABLED)
> > +               return;
> > +
> >         if (atomic_read(&in_suspend))
> >                 return;
> > 
> > @@ -1287,6 +1311,15 @@ thermal_zone_device_register(const char
> > *type, int trips, int mask,
> >         INIT_DELAYED_WORK(&tz->poll_queue,
> > thermal_zone_device_check);
> > 
> >         thermal_zone_device_reset(tz);
> > +
> > +       if (tz->ops->get_mode()) {
> remove the ()
> 
> > 
> > +               enum thermal_device_mode mode;
> > +
> > +               result = tz->ops->get_mode(tz, &mode);
> > +               tz->mode = result ? THERMAL_DEVICE_ENABLED : mode;
> > +       } else
> > +               tz->mode = THERMAL_DEVICE_ENABLED;
> > +
> >         /* Update the new thermal zone and mark it as already
> > updated. */
> >         if (atomic_cmpxchg(&tz->need_update, 1, 0))
> >                 thermal_zone_device_update(tz,
> > THERMAL_EVENT_UNSPECIFIED);
> > diff --git a/drivers/thermal/thermal_sysfs.c
> > b/drivers/thermal/thermal_sysfs.c
> > index a694de9..95d2587 100644
> > --- a/drivers/thermal/thermal_sysfs.c
> > +++ b/drivers/thermal/thermal_sysfs.c
> > @@ -51,17 +51,8 @@ static ssize_t
> >  mode_show(struct device *dev, struct device_attribute *attr, char
> > *buf)
> >  {
> >         struct thermal_zone_device *tz = to_thermal_zone(dev);
> > -       enum thermal_device_mode mode;
> > -       int result;
> > -
> > -       if (!tz->ops->get_mode)
> > -               return -EPERM;
> > 
> > -       result = tz->ops->get_mode(tz, &mode);
> > -       if (result)
> > -               return result;
> > -
> > -       return sprintf(buf, "%s\n", mode == THERMAL_DEVICE_ENABLED
> > ? "enabled"
> > +       return sprintf(buf, "%s\n", tz->mode ==
> > THERMAL_DEVICE_ENABLED ? "enabled"
> >                        : "disabled");
> >  }
> > 
> > @@ -70,18 +61,17 @@ mode_store(struct device *dev, struct
> > device_attribute *attr,
> >            const char *buf, size_t count)
> >  {
> >         struct thermal_zone_device *tz = to_thermal_zone(dev);
> > +       enum thermal_device_mode mode;
> >         int result;
> > 
> > -       if (!tz->ops->set_mode)
> > -               return -EPERM;
> > -
> >         if (!strncmp(buf, "enabled", sizeof("enabled") - 1))
> > -               result = tz->ops->set_mode(tz,
> > THERMAL_DEVICE_ENABLED);
> > +               mode = THERMAL_DEVICE_ENABLED;
> >         else if (!strncmp(buf, "disabled", sizeof("disabled") - 1))
> > -               result = tz->ops->set_mode(tz,
> > THERMAL_DEVICE_DISABLED);
> > +               mode = THERMAL_DEVICE_DISABLED;
> >         else
> > -               result = -EINVAL;
> > +               return -EINVAL;
> > 
> > +       result = thermal_zone_set_mode(tz, mode)
> >         if (result)
> >                 return result;
> > 
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index dab11f9..2f427de 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -210,6 +210,7 @@ struct thermal_zone_device {
> >         struct thermal_attr *trip_type_attrs;
> >         struct thermal_attr *trip_hyst_attrs;
> >         void *devdata;
> > +       enum thermal_device_mode mode;
> >         int trips;
> >         unsigned long trips_disabled;   /* bitmap for disabled
> > trips */
> >         int passive_delay;
> > @@ -465,6 +466,8 @@ struct thermal_zone_device
> > *thermal_zone_get_zone_by_name(const char *name);
> >  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_set_mode(struct thermal_zone_device *tz,
> > +                         enum thermal_device_mode mode);
> > 
> >  int get_tz_trend(struct thermal_zone_device *, int);
> >  struct thermal_instance *get_thermal_instance(struct
> > thermal_zone_device *,
> > --
> > 2.7.4
> > 
> There are some build issues but once fixed the patch works as
> expected, many thanks.
> 
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
> Rui, do you want I send a fixed version of this patch?

Please send out a fixed version.
But for this patch, I'd like to leave it in linux-next for sometime
before pushing it, as it makes some functional changes. And it will be
queued for 4.14 if there is no other problems.

thanks,
rui
>  Or do you want
> to fix yourself?
> 
> Best regards,
>   Enric