On 四, 2012-07-19 at 22:55 +0200, Rafael J. Wysocki wrote:
> On Thursday, July 19, 2012, Zhang Rui wrote:
> > set upper and lower limits when binding
> > a thermal cooling device to a thermal zone device.
> >
> > Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
> > ---
> > Documentation/thermal/sysfs-api.txt | 6 ++++-
> > drivers/acpi/thermal.c | 49 ++++++++++++++++++++++-------------
> > drivers/platform/x86/acerhdf.c | 2 +-
> > drivers/thermal/thermal_sys.c | 20 +++++++++-----
> > include/linux/thermal.h | 3 ++-
> > 5 files changed, 53 insertions(+), 27 deletions(-)
> >
> > diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> > index b7b3609..f169821 100644
> > --- a/Documentation/thermal/sysfs-api.txt
> > +++ b/Documentation/thermal/sysfs-api.txt
> > @@ -84,7 +84,7 @@ temperature) and throttle appropriate devices.
> >
> > 1.3 interface for binding a thermal zone device with a thermal cooling device
> > 1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
> > - int trip, struct thermal_cooling_device *cdev);
> > + int trip, struct thermal_cooling_device *cdev, long upper, long lower);
> >
> > This interface function bind a thermal cooling device to the certain trip
> > point of a thermal zone device.
> > @@ -93,6 +93,10 @@ temperature) and throttle appropriate devices.
> > cdev: thermal cooling device
> > trip: indicates which trip point the cooling devices is associated with
> > in this thermal zone.
> > + upper:the Maximum cooling state for this trip point.
> > + -1 means no upper limit, and the cooling device can be in max_state.
> > + lower:the Minimum cooling state can be used for this trip point.
> > + -1 means no lower limit, and the cooling device can be in cooling state 0.
>
> If we have to use -1 (or all ones for unsigned long), I'd suggest to define a
> constant called CS_NO_LIMIT or something like that and use it instead of the
> plain numbers.
>
agreed.
> And perhaps change the name of the above to
> thermal_zone_bind_cooling_device_limits() and define
> thermal_zone_bind_cooling_device() as a static inline that will not take the
> extra two arguments and make it call thermal_zone_bind_cooling_device_limits()
> with the CS_NO_LIMIT values in there.
>
then you mean there are two APIs for binding?
users can invoke thermal_zone_bind_cooling_device if they do not care
the limits?
Hmm, I do not think we need to do this. And further more, as there are
and there will be more and more parameters for binding, say, maybe
weight/influence in the near future, I'm thinking of using a struct
thermal_bind_params to pass the parameters instead.
> > 1.3.2 int thermal_zone_unbind_cooling_device(struct thermal_zone_device *tz,
> > int trip, struct thermal_cooling_device *cdev);
> > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> > index 8275e7b..a7c97f5 100644
> > --- a/drivers/acpi/thermal.c
> > +++ b/drivers/acpi/thermal.c
> > @@ -727,11 +727,9 @@ static int thermal_notify(struct thermal_zone_device *thermal, int trip,
> > return 0;
> > }
> >
> > -typedef int (*cb)(struct thermal_zone_device *, int,
> > - struct thermal_cooling_device *);
> > static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
> > struct thermal_cooling_device *cdev,
> > - cb action)
> > + int bind)
>
> Should 'bind' be a bool?
>
yep.
> > {
> > struct acpi_device *device = cdev->devdata;
> > struct acpi_thermal *tz = thermal->devdata;
> > @@ -755,11 +753,18 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
> > i++) {
> > handle = tz->trips.passive.devices.handles[i];
> > status = acpi_bus_get_device(handle, &dev);
> > - if (ACPI_SUCCESS(status) && (dev == device)) {
> > - result = action(thermal, trip, cdev);
> > - if (result)
> > - goto failed;
> > - }
> > + if (ACPI_FAILURE(status) || dev != device)
> > + continue;
> > + if (bind)
> > + result =
> > + thermal_zone_bind_cooling_device
> > + (thermal, trip, cdev, -1, -1);
> > + else
> > + result =
> > + thermal_zone_unbind_cooling_device
> > + (thermal, trip, cdev);
> > + if (result)
> > + goto failed;
> > }
> > }
> >
> > @@ -772,11 +777,16 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
> > j++) {
> > handle = tz->trips.active[i].devices.handles[j];
> > status = acpi_bus_get_device(handle, &dev);
> > - if (ACPI_SUCCESS(status) && (dev == device)) {
> > - result = action(thermal, trip, cdev);
> > - if (result)
> > - goto failed;
> > - }
> > + if (ACPI_FAILURE(status) || dev != device)
> > + continue;
> > + if (bind)
> > + result = thermal_zone_bind_cooling_device
> > + (thermal, trip, cdev, -1, -1);
> > + else
> > + result = thermal_zone_unbind_cooling_device
> > + (thermal, trip, cdev);
> > + if (result)
> > + goto failed;
> > }
> > }
> >
> > @@ -784,7 +794,12 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
> > handle = tz->devices.handles[i];
> > status = acpi_bus_get_device(handle, &dev);
> > if (ACPI_SUCCESS(status) && (dev == device)) {
> > - result = action(thermal, -1, cdev);
> > + if (bind)
> > + result = thermal_zone_bind_cooling_device
> > + (thermal, -1, cdev, -1, -1);
> > + else
> > + result = thermal_zone_unbind_cooling_device
> > + (thermal, -1, cdev);
> > if (result)
> > goto failed;
> > }
> > @@ -798,16 +813,14 @@ static int
> > acpi_thermal_bind_cooling_device(struct thermal_zone_device *thermal,
> > struct thermal_cooling_device *cdev)
> > {
> > - return acpi_thermal_cooling_device_cb(thermal, cdev,
> > - thermal_zone_bind_cooling_device);
> > + return acpi_thermal_cooling_device_cb(thermal, cdev, 1);
> > }
> >
> > static int
> > acpi_thermal_unbind_cooling_device(struct thermal_zone_device *thermal,
> > struct thermal_cooling_device *cdev)
> > {
> > - return acpi_thermal_cooling_device_cb(thermal, cdev,
> > - thermal_zone_unbind_cooling_device);
> > + return acpi_thermal_cooling_device_cb(thermal, cdev, 0);
> > }
> >
> > static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> > diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> > index 39abb15..278aa0e 100644
> > --- a/drivers/platform/x86/acerhdf.c
> > +++ b/drivers/platform/x86/acerhdf.c
> > @@ -329,7 +329,7 @@ static int acerhdf_bind(struct thermal_zone_device *thermal,
> > if (cdev != cl_dev)
> > return 0;
> >
> > - if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
> > + if (thermal_zone_bind_cooling_device(thermal, 0, cdev, -1, -1)) {
> > pr_err("error binding cooling dev\n");
> > return -EINVAL;
> > }
> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> > index 4267a49..db35300 100644
> > --- a/drivers/thermal/thermal_sys.c
> > +++ b/drivers/thermal/thermal_sys.c
> > @@ -316,7 +316,7 @@ passive_store(struct device *dev, struct device_attribute *attr,
> > sizeof("Processor")))
> > thermal_zone_bind_cooling_device(tz,
> > THERMAL_TRIPS_NONE,
> > - cdev);
> > + cdev, -1, -1);
> > }
> > mutex_unlock(&thermal_list_lock);
> > if (!tz->passive_delay)
> > @@ -801,7 +801,8 @@ static void thermal_zone_device_check(struct work_struct *work)
> > */
> > int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
> > int trip,
> > - struct thermal_cooling_device *cdev)
> > + struct thermal_cooling_device *cdev,
> > + long upper, long lower)
>
> upper and lower were unsigned long in the previous patch. Can we please
> use the same data type for them everywhere?
>
yep.
thanks,
rui
--
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
