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@xxxxxxxxx>:
> >
> > 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@xxxxxxxxx>
> > 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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxx>
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
