Hi Rui, Sorry for sending one incomplete mail before. I like this approach of multiple cooling states for ACTIVE type cooling devices. Some comments added. On 11 June 2012 08:50, Zhang Rui <rui.zhang@xxxxxxxxx> 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> > --- > drivers/acpi/thermal.c | 34 ++++++++++++++++++++++++---------- > drivers/platform/x86/acerhdf.c | 2 +- > drivers/thermal/thermal_sys.c | 20 ++++++++++++++------ > include/linux/thermal.h | 3 ++- > 4 files changed, 41 insertions(+), 18 deletions(-) > > Index: rtd3/drivers/thermal/thermal_sys.c > =================================================================== > --- rtd3.orig/drivers/thermal/thermal_sys.c > +++ rtd3/drivers/thermal/thermal_sys.c > @@ -248,7 +248,7 @@ passive_store(struct device *dev, struct > sizeof("Processor"))) > thermal_zone_bind_cooling_device(tz, > THERMAL_TRIPS_NONE, > - cdev); > + cdev, -1, -1); > } > mutex_unlock(&thermal_list_lock); > if (!tz->passive_delay) > @@ -760,7 +760,8 @@ static void thermal_zone_device_check(st > */ > 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) > { > struct thermal_cooling_device_instance *dev; > struct thermal_cooling_device_instance *pos; > @@ -784,6 +785,15 @@ int thermal_zone_bind_cooling_device(str > if (tz != pos1 || cdev != pos2) > return -EINVAL; > > + cdev->ops->get_max_state(cdev, &max_state); > + > + /* lower default 0, upper default max_state */ > + lower = lower < 0 ? 0 : lower; > + upper = upper < 0 ? max_state : upper; > + > + if (lower > upper || upper > max_state) > + return -EINVAL; > + > dev = > kzalloc(sizeof(struct thermal_cooling_device_instance), GFP_KERNEL); > if (!dev) > @@ -791,10 +801,8 @@ int thermal_zone_bind_cooling_device(str > dev->tz = tz; > dev->cdev = cdev; > dev->trip = trip; > - > - cdev->ops->get_max_state(dev, &max_state); > - dev->upper = max_state; > - dev->lower = 0; > + dev->upper = upper; > + dev->lower = lower; Also some check can be added here like this, cdev->ops->get_max_state(cdev, &cur_state); if (cur_state != lower) { print_err("Set the initial state of the cooling device to lower\n"); return -EINVAL; } This is needed because in your thermal_zone_update implementation the state change is from lower - upper. > > result = get_idr(&tz->idr, &tz->lock, &dev->id); > if (result) > Index: rtd3/include/linux/thermal.h > =================================================================== > --- rtd3.orig/include/linux/thermal.h > +++ rtd3/include/linux/thermal.h > @@ -143,7 +143,8 @@ struct thermal_zone_device *thermal_zone > void thermal_zone_device_unregister(struct thermal_zone_device *); > > int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int, > - struct thermal_cooling_device *); > + struct thermal_cooling_device *, > + long, long); > int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int, > struct thermal_cooling_device *); > void thermal_zone_device_update(struct thermal_zone_device *); > Index: rtd3/drivers/acpi/thermal.c > =================================================================== > --- rtd3.orig/drivers/acpi/thermal.c > +++ rtd3/drivers/acpi/thermal.c > @@ -729,11 +729,9 @@ static int thermal_notify(struct thermal > 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) > { > struct acpi_device *device = cdev->devdata; > struct acpi_thermal *tz = thermal->devdata; > @@ -758,7 +756,14 @@ static int acpi_thermal_cooling_device_c > 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 (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; > } > @@ -775,7 +780,13 @@ static int acpi_thermal_cooling_device_c > 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 (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; > } > @@ -786,7 +797,12 @@ static int acpi_thermal_cooling_device_c > 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; > } > @@ -800,16 +816,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 = { > Index: rtd3/drivers/platform/x86/acerhdf.c > =================================================================== > --- rtd3.orig/drivers/platform/x86/acerhdf.c > +++ rtd3/drivers/platform/x86/acerhdf.c > @@ -329,7 +329,7 @@ static int acerhdf_bind(struct thermal_z > 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; > } > > -- 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