| Message ID | 1251303445-25317-6-git-send-email-elendil@planet.nl (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Wed, Aug 26, 2009 at 06:17:24PM +0200, Frans Pop wrote: > Setting polling_delay is useless as passive_delay has priority, > so the value shown in proc isn't the actual polling delay. It > also gives the impression to the user that he can change the > polling interval through proc, while in fact he can't. > > Also, unset passive_delay when the forced passive trip point is > unbound to allow polling to be disabled. > > Signed-off-by: Frans Pop <elendil@planet.nl> > Cc: Matthew Garrett <mjg@redhat.com> > Cc: Zhang Rui <rui.zhang@intel.com> I'll look over this - I seem to remember having some reason to set that, but it escapes me now.
Hi Matthew, On Wednesday 26 August 2009, Matthew Garrett wrote: > On Wed, Aug 26, 2009 at 06:17:24PM +0200, Frans Pop wrote: > > Setting polling_delay is useless as passive_delay has priority, > > so the value shown in proc isn't the actual polling delay. It > > also gives the impression to the user that he can change the > > polling interval through proc, while in fact he can't. > > > > Also, unset passive_delay when the forced passive trip point is > > unbound to allow polling to be disabled. > > > > Signed-off-by: Frans Pop <elendil@planet.nl> > > Cc: Matthew Garrett <mjg@redhat.com> > > Cc: Zhang Rui <rui.zhang@intel.com> > > I'll look over this - I seem to remember having some reason to set > that, but it escapes me now. Have you had a chance to check this? Cheers, FJP -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 10, 2009 at 06:07:12PM +0200, Frans Pop wrote: > Have you had a chance to check this? I think this was just an artifact of me modifying the existing logic rather than it being meaningful, so as long as this works for you: Acked-by: Matthew Garrett <mjg@redhat.com>
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c index 2d13d0d..ceda0f1 100644 --- a/drivers/thermal/thermal_sys.c +++ b/drivers/thermal/thermal_sys.c @@ -241,6 +241,8 @@ passive_store(struct device *dev, struct device_attribute *attr, cdev); } mutex_unlock(&thermal_list_lock); + if (!tz->passive_delay) + tz->passive_delay = 1000; } else if (!state && tz->forced_passive) { mutex_lock(&thermal_list_lock); list_for_each_entry(cdev, &thermal_cdev_list, node) { @@ -251,17 +253,12 @@ passive_store(struct device *dev, struct device_attribute *attr, cdev); } mutex_unlock(&thermal_list_lock); + tz->passive_delay = 0; } tz->tc1 = 1; tz->tc2 = 1; - if (!tz->passive_delay) - tz->passive_delay = 1000; - - if (!tz->polling_delay) - tz->polling_delay = 10000; - tz->forced_passive = state; thermal_zone_device_update(tz);
Setting polling_delay is useless as passive_delay has priority, so the value shown in proc isn't the actual polling delay. It also gives the impression to the user that he can change the polling interval through proc, while in fact he can't. Also, unset passive_delay when the forced passive trip point is unbound to allow polling to be disabled. Signed-off-by: Frans Pop <elendil@planet.nl> Cc: Matthew Garrett <mjg@redhat.com> Cc: Zhang Rui <rui.zhang@intel.com> --- I'm not sure why polling_delay was getting set here. Possibly the intention was to set polling_frequency instead, which is in deci- seconds and would thus explain the factor 10 between the values. But even for polling_frequency there is IMO no reason to set it. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html