| Message ID | 1344516365-7230-8-git-send-email-durgadoss.r@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On ?, 2012-08-09 at 18:15 +0530, Durgadoss R wrote: > This patch updates the binding logic in thermal_sys.c > It uses the platform layer data to bind a thermal zone > to a cdev for a particular trip point. > > * If we do not have platform data and do not have > .bind defined, do not bind. > * If we do not have platform data but .bind is > defined, then use tz->ops->bind. > * If we have platform data, use it to create binding. > > The same logic sequence is followed for unbind also. > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com> > --- > drivers/thermal/thermal_sys.c | 170 ++++++++++++++++++++++++++++++++++------- > 1 file changed, 144 insertions(+), 26 deletions(-) > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > index 195e529..748b12f 100644 > --- a/drivers/thermal/thermal_sys.c > +++ b/drivers/thermal/thermal_sys.c > @@ -158,6 +158,107 @@ static void retrieve_zone_params(struct thermal_zone_device *tz) > } > } > > +static void print_bind_err_msg(struct thermal_zone_device *tz, > + struct thermal_cooling_device *cdev, int ret) > +{ > + dev_err(&tz->device, "binding zone %s with cdev %s failed:%d\n", > + tz->type, cdev->type, ret); > +} > + > +static void __bind(struct thermal_zone_device *tz, int mask, > + struct thermal_cooling_device *cdev) > +{ > + int i, ret; > + > + for (i = 0; i < tz->trips; i++) { > + if (mask & (1 << i)) { > + ret = thermal_zone_bind_cooling_device(tz, i, cdev, > + THERMAL_NO_LIMIT, THERMAL_NO_LIMIT); > + if (ret) > + print_bind_err_msg(tz, cdev, ret); > + } > + } > +} > + > +static void __unbind(struct thermal_zone_device *tz, int mask, > + struct thermal_cooling_device *cdev) > +{ > + int i; > + > + for (i = 0; i < tz->trips; i++) > + if (mask & (1 << i)) > + thermal_zone_unbind_cooling_device(tz, i, cdev); > +} > + > +static void update_bind_info(struct thermal_cooling_device *cdev) > +{ > + int i, ret; > + struct thermal_zone_params *tzp; > + struct thermal_zone_device *pos = NULL; > + > + mutex_lock(&thermal_list_lock); > + > + list_for_each_entry(pos, &thermal_tz_list, node) { > + if (!pos->tzp && !pos->ops->bind) > + continue; > + > + if (!pos->tzp && pos->ops->bind) { > + ret = pos->ops->bind(pos, cdev); > + if (ret) > + print_bind_err_msg(pos, cdev, ret); > + } > + > + tzp = pos->tzp; > + for (i = 0; i < tzp->num_cdevs; i++) { > + if (!strcmp(tzp->cdevs_name[i], cdev->type)) { > + __bind(pos, tzp->trip_mask[i], cdev); > + break; > + } > + } > + } > + mutex_unlock(&thermal_list_lock); > +} I still do not understand why we need this kind of bind. Say, the platform thermal driver knows the platform data, i.e. it knows which cooling devices should be bound to which trip points. why we can not move this kind of logic to the .bind() callback, offered by the platform thermal driver? say, in .bind() callback, the platform thermal driver has the pointer of the platform data, right? the .cdev parameter can be used to find the cooling device name, and we can make the comparison there. instead of introducing new binding functions in the generic thermal layer. > + > +static void do_binding(struct thermal_zone_device *tz) > +{ > + int i, ret; > + char *name; > + struct thermal_cooling_device *pos = NULL; > + struct thermal_zone_params *tzp = tz->tzp; > + > + if (!tzp && !tz->ops->bind) > + return; > + > + /* If there is no platform data, try to use ops->bind */ > + if (!tzp && tz->ops->bind) { > + mutex_lock(&thermal_list_lock); > + > + list_for_each_entry(pos, &thermal_cdev_list, node) { > + ret = tz->ops->bind(tz, pos); > + if (ret) > + print_bind_err_msg(tz, pos, ret); > + } > + > + mutex_unlock(&thermal_list_lock); > + return; > + } > + > + /* If platform data is available, use it to create binding */ > + for (i = 0; i < tzp->num_cdevs; i++) { > + name = tzp->cdevs_name[i]; > + pos = get_cdev_by_name(name); > + > + if (!pos) { > + dev_err(&tz->device, "cannot find cdev %s\n", name); > + continue; > + } > + > + mutex_lock(&thermal_list_lock); > + __bind(tz, tzp->trip_mask[i], pos); > + mutex_unlock(&thermal_list_lock); > + } > +} > + > /* sys I/F for thermal zone */ > > #define to_thermal_zone(_dev) \ > @@ -975,7 +1076,6 @@ thermal_cooling_device_register(char *type, void *devdata, > const struct thermal_cooling_device_ops *ops) > { > struct thermal_cooling_device *cdev; > - struct thermal_zone_device *pos; > int result; > > if (strlen(type) >= THERMAL_NAME_LENGTH) > @@ -1025,20 +1125,15 @@ thermal_cooling_device_register(char *type, void *devdata, > if (result) > goto unregister; > > + /* Add 'this' new cdev to the global cdev list */ > mutex_lock(&thermal_list_lock); > list_add(&cdev->node, &thermal_cdev_list); > - list_for_each_entry(pos, &thermal_tz_list, node) { > - if (!pos->ops->bind) > - continue; > - result = pos->ops->bind(pos, cdev); > - if (result) > - break; > - > - } > mutex_unlock(&thermal_list_lock); > > - if (!result) > - return cdev; > + /* Update binding information for 'this' new cdev */ > + update_bind_info(cdev); > + > + return cdev; > > unregister: > release_idr(&thermal_cdev_idr, &thermal_idr_lock, cdev->id); > @@ -1054,10 +1149,10 @@ EXPORT_SYMBOL(thermal_cooling_device_register); > * thermal_cooling_device_unregister() must be called when the device is no > * longer needed. > */ > -void thermal_cooling_device_unregister(struct > - thermal_cooling_device > - *cdev) > +void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev) > { > + int i; > + struct thermal_zone_params *tzp; > struct thermal_zone_device *tz; > struct thermal_cooling_device *pos = NULL; > > @@ -1074,12 +1169,23 @@ void thermal_cooling_device_unregister(struct > return; > } > list_del(&cdev->node); > + > + /* Unbind all thermal zones associated with 'this' cdev */ > list_for_each_entry(tz, &thermal_tz_list, node) { > - if (!tz->ops->unbind) > + tzp = tz->tzp; > + if (!tzp && !tz->ops->unbind) > continue; > - tz->ops->unbind(tz, cdev); > + > + if (!tzp && tz->ops->unbind) > + tz->ops->unbind(tz, cdev); > + > + for (i = 0; i < tzp->num_cdevs; i++) > + if (!strcmp(cdev->type, tzp->cdevs_name[i])) > + __unbind(tz, tzp->trip_mask[i], cdev); > } > + > mutex_unlock(&thermal_list_lock); > + > if (cdev->type[0]) > device_remove_file(&cdev->device, &dev_attr_cdev_type); > device_remove_file(&cdev->device, &dev_attr_max_state); > @@ -1424,7 +1530,6 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, > int passive_delay, int polling_delay) > { > struct thermal_zone_device *tz; > - struct thermal_cooling_device *pos; > enum thermal_trip_type trip_type; > int result; > int count; > @@ -1519,14 +1624,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, > > mutex_lock(&thermal_list_lock); > list_add_tail(&tz->node, &thermal_tz_list); > - if (ops->bind) > - list_for_each_entry(pos, &thermal_cdev_list, node) { > - result = ops->bind(tz, pos); > - if (result) > - break; > - } > mutex_unlock(&thermal_list_lock); > > + /* Bind cooling devices for this zone */ > + do_binding(tz); > + > + > INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check); > > thermal_zone_device_update(tz); > @@ -1547,12 +1650,16 @@ EXPORT_SYMBOL(thermal_zone_device_register); > */ > void thermal_zone_device_unregister(struct thermal_zone_device *tz) > { > + int i; > + struct thermal_zone_params *tzp; > struct thermal_cooling_device *cdev; > struct thermal_zone_device *pos = NULL; > > if (!tz) > return; > > + tzp = tz->tzp; > + > mutex_lock(&thermal_list_lock); > list_for_each_entry(pos, &thermal_tz_list, node) > if (pos == tz) > @@ -1563,9 +1670,20 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz) > return; > } > list_del(&tz->node); > - if (tz->ops->unbind) > - list_for_each_entry(cdev, &thermal_cdev_list, node) > - tz->ops->unbind(tz, cdev); > + > + /* Unbind all cdevs associated with 'this' thermal zone */ > + list_for_each_entry(cdev, &thermal_cdev_list, node) { > + if (!tzp && !tz->ops->unbind) > + break; > + > + if (!tzp && tz->ops->unbind) > + tz->ops->unbind(tz, cdev); > + > + for (i = 0; i < tzp->num_cdevs; i++) > + if (!strcmp(cdev->type, tzp->cdevs_name[i])) > + __unbind(tz, tzp->trip_mask[i], cdev); > + } > + > mutex_unlock(&thermal_list_lock); > > thermal_zone_device_set_polling(tz, 0); -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rui, > -----Original Message----- > From: Zhang, Rui > Sent: Monday, August 13, 2012 12:11 PM > To: R, Durgadoss > Cc: lenb@kernel.org; rjw@sisk.pl; linux-acpi@vger.kernel.org; linux- > pm@vger.kernel.org; eduardo.valentin@ti.com; amit.kachhap@linaro.org; > wni@nvidia.com > Subject: Re: [PATCH 07/13] Thermal: Update binding logic based on platform data > > On ?, 2012-08-09 at 18:15 +0530, Durgadoss R wrote: > > This patch updates the binding logic in thermal_sys.c > > It uses the platform layer data to bind a thermal zone > > to a cdev for a particular trip point. > > > > * If we do not have platform data and do not have > > .bind defined, do not bind. > > * If we do not have platform data but .bind is > > defined, then use tz->ops->bind. > > * If we have platform data, use it to create binding. > > > > The same logic sequence is followed for unbind also. > > > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com> > > --- > > drivers/thermal/thermal_sys.c | 170 > ++++++++++++++++++++++++++++++++++------- > > 1 file changed, 144 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > > index 195e529..748b12f 100644 > > --- a/drivers/thermal/thermal_sys.c > > +++ b/drivers/thermal/thermal_sys.c > > @@ -158,6 +158,107 @@ static void retrieve_zone_params(struct > thermal_zone_device *tz) > > } > > } > > > > +static void print_bind_err_msg(struct thermal_zone_device *tz, > > + struct thermal_cooling_device *cdev, int ret) > > +{ > > + dev_err(&tz->device, "binding zone %s with cdev %s failed:%d\n", > > + tz->type, cdev->type, ret); > > +} > > + > > +static void __bind(struct thermal_zone_device *tz, int mask, > > + struct thermal_cooling_device *cdev) > > +{ > > + int i, ret; > > + > > + for (i = 0; i < tz->trips; i++) { > > + if (mask & (1 << i)) { > > + ret = thermal_zone_bind_cooling_device(tz, i, cdev, > > + THERMAL_NO_LIMIT, > THERMAL_NO_LIMIT); > > + if (ret) > > + print_bind_err_msg(tz, cdev, ret); > > + } > > + } > > +} > > + > > +static void __unbind(struct thermal_zone_device *tz, int mask, > > + struct thermal_cooling_device *cdev) > > +{ > > + int i; > > + > > + for (i = 0; i < tz->trips; i++) > > + if (mask & (1 << i)) > > + thermal_zone_unbind_cooling_device(tz, i, cdev); > > +} > > + > > +static void update_bind_info(struct thermal_cooling_device *cdev) > > +{ > > + int i, ret; > > + struct thermal_zone_params *tzp; > > + struct thermal_zone_device *pos = NULL; > > + > > + mutex_lock(&thermal_list_lock); > > + > > + list_for_each_entry(pos, &thermal_tz_list, node) { > > + if (!pos->tzp && !pos->ops->bind) > > + continue; > > + > > + if (!pos->tzp && pos->ops->bind) { > > + ret = pos->ops->bind(pos, cdev); > > + if (ret) > > + print_bind_err_msg(pos, cdev, ret); > > + } > > + > > + tzp = pos->tzp; > > + for (i = 0; i < tzp->num_cdevs; i++) { > > + if (!strcmp(tzp->cdevs_name[i], cdev->type)) { > > + __bind(pos, tzp->trip_mask[i], cdev); > > + break; > > + } > > + } > > + } > > + mutex_unlock(&thermal_list_lock); > > +} > > I still do not understand why we need this kind of bind. > Say, the platform thermal driver knows the platform data, i.e. it knows > which cooling devices should be bound to which trip points. > why we can not move this kind of logic to the .bind() callback, offered > by the platform thermal driver? > say, in .bind() callback, > the platform thermal driver has the pointer of the platform data, right? > the .cdev parameter can be used to find the cooling device name, > and we can make the comparison there. instead of introducing new binding > functions in the generic thermal layer. For once, I got little confused between the generic platform thermal sensor drivers (the chip drivers) and the platform level driver (not specific for chip, but for a platform). So, yes we can put this in the platform level driver. On the other hand, I believe we will have more and more platform thermal drivers, as new devices arrive. And in each of the drivers, we are going to do the 'looping, finding cdev and then binding' part. I was wondering wouldn't it be better to move the common code to the framework, so that the same code does not get duplicated, over several places. So, please give a second thought on this and let me know your opinion. Thanks, Durga
On ?, 2012-08-13 at 09:41 -0600, R, Durgadoss wrote: > Hi Rui, > > > -----Original Message----- > > From: Zhang, Rui > > Sent: Monday, August 13, 2012 12:11 PM > > To: R, Durgadoss > > Cc: lenb@kernel.org; rjw@sisk.pl; linux-acpi@vger.kernel.org; linux- > > pm@vger.kernel.org; eduardo.valentin@ti.com; amit.kachhap@linaro.org; > > wni@nvidia.com > > Subject: Re: [PATCH 07/13] Thermal: Update binding logic based on platform data > > > > On ?, 2012-08-09 at 18:15 +0530, Durgadoss R wrote: > > > This patch updates the binding logic in thermal_sys.c > > > It uses the platform layer data to bind a thermal zone > > > to a cdev for a particular trip point. > > > > > > * If we do not have platform data and do not have > > > .bind defined, do not bind. > > > * If we do not have platform data but .bind is > > > defined, then use tz->ops->bind. > > > * If we have platform data, use it to create binding. > > > > > > The same logic sequence is followed for unbind also. > > > > > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com> > > > --- > > > drivers/thermal/thermal_sys.c | 170 > > ++++++++++++++++++++++++++++++++++------- > > > 1 file changed, 144 insertions(+), 26 deletions(-) > > > > > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > > > index 195e529..748b12f 100644 > > > --- a/drivers/thermal/thermal_sys.c > > > +++ b/drivers/thermal/thermal_sys.c > > > @@ -158,6 +158,107 @@ static void retrieve_zone_params(struct > > thermal_zone_device *tz) > > > } > > > } > > > > > > +static void print_bind_err_msg(struct thermal_zone_device *tz, > > > + struct thermal_cooling_device *cdev, int ret) > > > +{ > > > + dev_err(&tz->device, "binding zone %s with cdev %s failed:%d\n", > > > + tz->type, cdev->type, ret); > > > +} > > > + > > > +static void __bind(struct thermal_zone_device *tz, int mask, > > > + struct thermal_cooling_device *cdev) > > > +{ > > > + int i, ret; > > > + > > > + for (i = 0; i < tz->trips; i++) { > > > + if (mask & (1 << i)) { > > > + ret = thermal_zone_bind_cooling_device(tz, i, cdev, > > > + THERMAL_NO_LIMIT, > > THERMAL_NO_LIMIT); > > > + if (ret) > > > + print_bind_err_msg(tz, cdev, ret); > > > + } > > > + } > > > +} > > > + > > > +static void __unbind(struct thermal_zone_device *tz, int mask, > > > + struct thermal_cooling_device *cdev) > > > +{ > > > + int i; > > > + > > > + for (i = 0; i < tz->trips; i++) > > > + if (mask & (1 << i)) > > > + thermal_zone_unbind_cooling_device(tz, i, cdev); > > > +} > > > + > > > +static void update_bind_info(struct thermal_cooling_device *cdev) > > > +{ > > > + int i, ret; > > > + struct thermal_zone_params *tzp; > > > + struct thermal_zone_device *pos = NULL; > > > + > > > + mutex_lock(&thermal_list_lock); > > > + > > > + list_for_each_entry(pos, &thermal_tz_list, node) { > > > + if (!pos->tzp && !pos->ops->bind) > > > + continue; > > > + > > > + if (!pos->tzp && pos->ops->bind) { > > > + ret = pos->ops->bind(pos, cdev); > > > + if (ret) > > > + print_bind_err_msg(pos, cdev, ret); > > > + } > > > + > > > + tzp = pos->tzp; > > > + for (i = 0; i < tzp->num_cdevs; i++) { > > > + if (!strcmp(tzp->cdevs_name[i], cdev->type)) { > > > + __bind(pos, tzp->trip_mask[i], cdev); > > > + break; > > > + } > > > + } > > > + } > > > + mutex_unlock(&thermal_list_lock); > > > +} > > > > I still do not understand why we need this kind of bind. > > Say, the platform thermal driver knows the platform data, i.e. it knows > > which cooling devices should be bound to which trip points. > > why we can not move this kind of logic to the .bind() callback, offered > > by the platform thermal driver? > > say, in .bind() callback, > > the platform thermal driver has the pointer of the platform data, right? > > the .cdev parameter can be used to find the cooling device name, > > and we can make the comparison there. instead of introducing new binding > > functions in the generic thermal layer. > > For once, I got little confused between the generic platform thermal sensor > drivers (the chip drivers) and the platform level driver (not specific for chip, > but for a platform). So, yes we can put this in the platform level driver. > Hmm, I'm not clear about the difference between these two drivers. what is supposed to be done in the platform thermal sensor drivers and what is supposed to be done in the platform level driver? At least for now, all the thermal drivers are both thermal sensor driver and platform level driver, right? thanks, rui > On the other hand, I believe we will have more and more platform thermal > drivers, as new devices arrive. And in each of the drivers, we are going to > do the 'looping, finding cdev and then binding' part. I was wondering wouldn't > it be better to move the common code to the framework, so that the same > code does not get duplicated, over several places. > > > > So, please give a second thought on this and let me know your opinion. > Thanks, > Durga -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SGkgUnVpLA0KDQo+ID4gPiA+ICtzdGF0aWMgdm9pZCB1cGRhdGVfYmluZF9pbmZvKHN0cnVjdCB0 aGVybWFsX2Nvb2xpbmdfZGV2aWNlICpjZGV2KQ0KPiA+ID4gPiArew0KPiA+ID4gPiArCWludCBp LCByZXQ7DQo+ID4gPiA+ICsJc3RydWN0IHRoZXJtYWxfem9uZV9wYXJhbXMgKnR6cDsNCj4gPiA+ ID4gKwlzdHJ1Y3QgdGhlcm1hbF96b25lX2RldmljZSAqcG9zID0gTlVMTDsNCj4gPiA+ID4gKw0K PiA+ID4gPiArCW11dGV4X2xvY2soJnRoZXJtYWxfbGlzdF9sb2NrKTsNCj4gPiA+ID4gKw0KPiA+ ID4gPiArCWxpc3RfZm9yX2VhY2hfZW50cnkocG9zLCAmdGhlcm1hbF90el9saXN0LCBub2RlKSB7 DQo+ID4gPiA+ICsJCWlmICghcG9zLT50enAgJiYgIXBvcy0+b3BzLT5iaW5kKQ0KPiA+ID4gPiAr CQkJY29udGludWU7DQo+ID4gPiA+ICsNCj4gPiA+ID4gKwkJaWYgKCFwb3MtPnR6cCAmJiBwb3Mt Pm9wcy0+YmluZCkgew0KPiA+ID4gPiArCQkJcmV0ID0gcG9zLT5vcHMtPmJpbmQocG9zLCBjZGV2 KTsNCj4gPiA+ID4gKwkJCWlmIChyZXQpDQo+ID4gPiA+ICsJCQkJcHJpbnRfYmluZF9lcnJfbXNn KHBvcywgY2RldiwgcmV0KTsNCj4gPiA+ID4gKwkJfQ0KPiA+ID4gPiArDQo+ID4gPiA+ICsJCXR6 cCA9IHBvcy0+dHpwOw0KPiA+ID4gPiArCQlmb3IgKGkgPSAwOyBpIDwgdHpwLT5udW1fY2RldnM7 IGkrKykgew0KPiA+ID4gPiArCQkJaWYgKCFzdHJjbXAodHpwLT5jZGV2c19uYW1lW2ldLCBjZGV2 LT50eXBlKSkgew0KPiA+ID4gPiArCQkJCV9fYmluZChwb3MsIHR6cC0+dHJpcF9tYXNrW2ldLCBj ZGV2KTsNCj4gPiA+ID4gKwkJCQlicmVhazsNCj4gPiA+ID4gKwkJCX0NCj4gPiA+ID4gKwkJfQ0K PiA+ID4gPiArCX0NCj4gPiA+ID4gKwltdXRleF91bmxvY2soJnRoZXJtYWxfbGlzdF9sb2NrKTsN Cj4gPiA+ID4gK30NCj4gPiA+DQo+ID4gPiBJIHN0aWxsIGRvIG5vdCB1bmRlcnN0YW5kIHdoeSB3 ZSBuZWVkIHRoaXMga2luZCBvZiBiaW5kLg0KPiA+ID4gU2F5LCB0aGUgcGxhdGZvcm0gdGhlcm1h bCBkcml2ZXIga25vd3MgdGhlIHBsYXRmb3JtIGRhdGEsIGkuZS4gaXQga25vd3MNCj4gPiA+IHdo aWNoIGNvb2xpbmcgZGV2aWNlcyBzaG91bGQgYmUgYm91bmQgdG8gd2hpY2ggdHJpcCBwb2ludHMu DQo+ID4gPiB3aHkgd2UgY2FuIG5vdCBtb3ZlIHRoaXMga2luZCBvZiBsb2dpYyB0byB0aGUgLmJp bmQoKSBjYWxsYmFjaywgb2ZmZXJlZA0KPiA+ID4gYnkgdGhlIHBsYXRmb3JtIHRoZXJtYWwgZHJp dmVyPw0KPiA+ID4gc2F5LCBpbiAuYmluZCgpIGNhbGxiYWNrLA0KPiA+ID4gdGhlIHBsYXRmb3Jt IHRoZXJtYWwgZHJpdmVyIGhhcyB0aGUgcG9pbnRlciBvZiB0aGUgcGxhdGZvcm0gZGF0YSwgcmln aHQ/DQo+ID4gPiB0aGUgLmNkZXYgcGFyYW1ldGVyIGNhbiBiZSB1c2VkIHRvIGZpbmQgdGhlIGNv b2xpbmcgZGV2aWNlIG5hbWUsDQo+ID4gPiBhbmQgd2UgY2FuIG1ha2UgdGhlIGNvbXBhcmlzb24g dGhlcmUuIGluc3RlYWQgb2YgaW50cm9kdWNpbmcgbmV3IGJpbmRpbmcNCj4gPiA+IGZ1bmN0aW9u cyBpbiB0aGUgZ2VuZXJpYyB0aGVybWFsIGxheWVyLg0KPiA+DQo+ID4gRm9yIG9uY2UsIEkgZ290 IGxpdHRsZSBjb25mdXNlZCBiZXR3ZWVuIHRoZSBnZW5lcmljIHBsYXRmb3JtIHRoZXJtYWwgc2Vu c29yDQo+ID4gZHJpdmVycyAodGhlIGNoaXAgZHJpdmVycykgYW5kIHRoZSBwbGF0Zm9ybSBsZXZl bCBkcml2ZXIgKG5vdCBzcGVjaWZpYyBmb3IgY2hpcCwNCj4gPiBidXQgZm9yIGEgcGxhdGZvcm0p LiBTbywgeWVzIHdlIGNhbiBwdXQgdGhpcyBpbiB0aGUgcGxhdGZvcm0gbGV2ZWwgZHJpdmVyLg0K PiA+DQo+IEhtbSwNCj4gSSdtIG5vdCBjbGVhciBhYm91dCB0aGUgZGlmZmVyZW5jZSBiZXR3ZWVu IHRoZXNlIHR3byBkcml2ZXJzLg0KPiB3aGF0IGlzIHN1cHBvc2VkIHRvIGJlIGRvbmUgaW4gdGhl IHBsYXRmb3JtIHRoZXJtYWwgc2Vuc29yIGRyaXZlcnMgYW5kDQo+IHdoYXQgaXMgc3VwcG9zZWQg dG8gYmUgZG9uZSBpbiB0aGUgcGxhdGZvcm0gbGV2ZWwgZHJpdmVyPw0KDQpBIHNlbnNvciBkcml2 ZXIgY2FuIGJlIGEgZ2VuZXJpYyBjaGlwIGRyaXZlciBsaWtlIGVtYzE0MDMgKHRoaXMgaXMgdGhl IG9uZQ0KdGhhdCBJIGhhdmUgd29ya2VkIG9uLi4pIG9yIGNvcmV0ZW1wICh0aGUgQ1BVIERUUyBk cml2ZXIgZm9yIHg4NikuIFRoZXkgc2l0DQppbiBkaWZmZXJlbnQgc3ViIHN5c3RlbXMgKHRoZXNl IHR3byBpbiBod21vbikuIFdlIG1pZ2h0IG5vdCBiZSBhbGxvd2VkIHRvDQphZGQgYW55IHRoZXJt YWwgZnJhbWV3b3JrIHNwZWNpZmljIGNvZGUgaW4gdGhlc2UgZHJpdmVycy4gVGhlIHNhbWUgZHJp dmVyDQp3b3JrcyBvbiBhbGwgcGxhdGZvcm1zLg0KDQpBIHBsYXRmb3JtIGxldmVsIHRoZXJtYWwg ZHJpdmVyIGtub3dzIGluZm9ybWF0aW9uIGFib3V0IHRoZSB0aGVybWFsIHNlbnNvcnMsDQphbmQg dGhlaXIgem9uZXMgb24gdGhlIHBsYXRmb3JtOyBhbmQgaXMgc3BlY2lmaWMgdG8gdGhlIHBsYXRm b3JtLg0KRm9yIHg4NiwgdGhpcyB3aWxsIGJlIGluIGRyaXZlcnMveDg2L3BsYXRmb3JtLyB3aGVy ZWFzIG1pZ2h0IGJlIGluIHNvbWUgb3RoZXINCnBsYWNlIGZvciBvdGhlciBhcmNoaXRlY3R1cmVz LiBBbiBleGFtcGxlIGlzIGludGVsX21pZF90aGVybWFsLmMgd2hpY2ggc2l0cw0KaW4gZHJpdmVy cy94ODYvcGxhdGZvcm0uIFdlIGNhbiBhZGQgb3VyIHRoZXJtYWwgZnJhbWV3b3JrIHNwZWNpZmlj IGNvZGUNCnRvIHRoaXMgZHJpdmVyLg0KDQo+IA0KPiBBdCBsZWFzdCBmb3Igbm93LCBhbGwgdGhl IHRoZXJtYWwgZHJpdmVycyBhcmUgYm90aCB0aGVybWFsIHNlbnNvciBkcml2ZXINCj4gYW5kIHBs YXRmb3JtIGxldmVsIGRyaXZlciwgcmlnaHQ/DQogDQpOb3QgYWxsIHRoZSB0aW1lcywgYWx0aG91 Z2ggdGhlcmUgYXJlIHNvbWUgaW5zdGFuY2VzIHdoZXJlIGJvdGggYXJlIHNhbWUuDQpXZSB1c2Ug Y29yZXRlbXAuYyBhbmQgaW50ZWxfbWlkX3RoZXJtYWwuYyAod2hpY2ggYXJlIGRpZmZlcmVudCks IGZvciB0aGUNCng4NiBtaWQgcGxhdGZvcm1zLg0KDQpJIGhvcGUgdGhpcyBleHBsYW5hdGlvbiBo ZWxwcyB5b3UuLg0KDQpUaGFua3MsDQpEdXJnYQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On ?, 2012-08-15 at 03:17 -0600, R, Durgadoss wrote: > Hi Rui, > > > > > > +static void update_bind_info(struct thermal_cooling_device *cdev) > > > > > +{ > > > > > + int i, ret; > > > > > + struct thermal_zone_params *tzp; > > > > > + struct thermal_zone_device *pos = NULL; > > > > > + > > > > > + mutex_lock(&thermal_list_lock); > > > > > + > > > > > + list_for_each_entry(pos, &thermal_tz_list, node) { > > > > > + if (!pos->tzp && !pos->ops->bind) > > > > > + continue; > > > > > + > > > > > + if (!pos->tzp && pos->ops->bind) { > > > > > + ret = pos->ops->bind(pos, cdev); > > > > > + if (ret) > > > > > + print_bind_err_msg(pos, cdev, ret); > > > > > + } > > > > > + > > > > > + tzp = pos->tzp; > > > > > + for (i = 0; i < tzp->num_cdevs; i++) { > > > > > + if (!strcmp(tzp->cdevs_name[i], cdev->type)) { > > > > > + __bind(pos, tzp->trip_mask[i], cdev); > > > > > + break; > > > > > + } > > > > > + } > > > > > + } > > > > > + mutex_unlock(&thermal_list_lock); > > > > > +} > > > > > > > > I still do not understand why we need this kind of bind. > > > > Say, the platform thermal driver knows the platform data, i.e. it knows > > > > which cooling devices should be bound to which trip points. > > > > why we can not move this kind of logic to the .bind() callback, offered > > > > by the platform thermal driver? > > > > say, in .bind() callback, > > > > the platform thermal driver has the pointer of the platform data, right? > > > > the .cdev parameter can be used to find the cooling device name, > > > > and we can make the comparison there. instead of introducing new binding > > > > functions in the generic thermal layer. > > > > > > For once, I got little confused between the generic platform thermal sensor > > > drivers (the chip drivers) and the platform level driver (not specific for chip, > > > but for a platform). So, yes we can put this in the platform level driver. > > > > > Hmm, > > I'm not clear about the difference between these two drivers. > > what is supposed to be done in the platform thermal sensor drivers and > > what is supposed to be done in the platform level driver? > > A sensor driver can be a generic chip driver like emc1403 (this is the one > that I have worked on..) or coretemp (the CPU DTS driver for x86). They sit > in different sub systems (these two in hwmon). We might not be allowed to > add any thermal framework specific code in these drivers. The same driver > works on all platforms. does the sensor know anything about the "policy"? Say, does it have any trip points? does it know which device can be throttled to cool itself? I think the answer is "no", right? > > A platform level thermal driver knows information about the thermal sensors, > and their zones on the platform; and is specific to the platform. > For x86, this will be in drivers/x86/platform/ whereas might be in some other > place for other architectures. An example is intel_mid_thermal.c which sits > in drivers/x86/platform. We can add our thermal framework specific code > to this driver. > but I think intel_mide_thermal driver is also a platform thermal sensor driver at the same time. > > > > At least for now, all the thermal drivers are both thermal sensor driver > > and platform level driver, right? > > Not all the times, although there are some instances where both are same. > We use coretemp.c and intel_mid_thermal.c (which are different), for the > x86 mid platforms. > so you want to use coretemp.c as a temperature sensor, and then bind your own cooling devices to it in your platform level thermal driver? thanks, rui -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SGkgUnVpLA0KDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogWmhhbmcs IFJ1aQ0KPiBTZW50OiBUaHVyc2RheSwgQXVndXN0IDE2LCAyMDEyIDk6MDAgQU0NCj4gVG86IFIs IER1cmdhZG9zcw0KPiBDYzogbGVuYkBrZXJuZWwub3JnOyByandAc2lzay5wbDsgbGludXgtYWNw aUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LQ0KPiBwbUB2Z2VyLmtlcm5lbC5vcmc7IGVkdWFyZG8u dmFsZW50aW5AdGkuY29tOyBhbWl0LmthY2hoYXBAbGluYXJvLm9yZzsNCj4gd25pQG52aWRpYS5j b20NCj4gU3ViamVjdDogUkU6IFtQQVRDSCAwNy8xM10gVGhlcm1hbDogVXBkYXRlIGJpbmRpbmcg bG9naWMgYmFzZWQgb24gcGxhdGZvcm0NCj4gZGF0YQ0KPiANCj4gT24g5LiJLCAyMDEyLTA4LTE1 IGF0IDAzOjE3IC0wNjAwLCBSLCBEdXJnYWRvc3Mgd3JvdGU6DQo+ID4gSGkgUnVpLA0KPiA+DQo+ ID4gPiA+ID4gPiArc3RhdGljIHZvaWQgdXBkYXRlX2JpbmRfaW5mbyhzdHJ1Y3QgdGhlcm1hbF9j b29saW5nX2RldmljZQ0KPiAqY2RldikNCj4gPiA+ID4gPiA+ICt7DQo+ID4gPiA+ID4gPiArCWlu dCBpLCByZXQ7DQo+ID4gPiA+ID4gPiArCXN0cnVjdCB0aGVybWFsX3pvbmVfcGFyYW1zICp0enA7 DQo+ID4gPiA+ID4gPiArCXN0cnVjdCB0aGVybWFsX3pvbmVfZGV2aWNlICpwb3MgPSBOVUxMOw0K PiA+ID4gPiA+ID4gKw0KPiA+ID4gPiA+ID4gKwltdXRleF9sb2NrKCZ0aGVybWFsX2xpc3RfbG9j ayk7DQo+ID4gPiA+ID4gPiArDQo+ID4gPiA+ID4gPiArCWxpc3RfZm9yX2VhY2hfZW50cnkocG9z LCAmdGhlcm1hbF90el9saXN0LCBub2RlKSB7DQo+ID4gPiA+ID4gPiArCQlpZiAoIXBvcy0+dHpw ICYmICFwb3MtPm9wcy0+YmluZCkNCj4gPiA+ID4gPiA+ICsJCQljb250aW51ZTsNCj4gPiA+ID4g PiA+ICsNCj4gPiA+ID4gPiA+ICsJCWlmICghcG9zLT50enAgJiYgcG9zLT5vcHMtPmJpbmQpIHsN Cj4gPiA+ID4gPiA+ICsJCQlyZXQgPSBwb3MtPm9wcy0+YmluZChwb3MsIGNkZXYpOw0KPiA+ID4g PiA+ID4gKwkJCWlmIChyZXQpDQo+ID4gPiA+ID4gPiArCQkJCXByaW50X2JpbmRfZXJyX21zZyhw b3MsIGNkZXYsIHJldCk7DQo+ID4gPiA+ID4gPiArCQl9DQo+ID4gPiA+ID4gPiArDQo+ID4gPiA+ ID4gPiArCQl0enAgPSBwb3MtPnR6cDsNCj4gPiA+ID4gPiA+ICsJCWZvciAoaSA9IDA7IGkgPCB0 enAtPm51bV9jZGV2czsgaSsrKSB7DQo+ID4gPiA+ID4gPiArCQkJaWYgKCFzdHJjbXAodHpwLT5j ZGV2c19uYW1lW2ldLCBjZGV2LT50eXBlKSkNCj4gew0KPiA+ID4gPiA+ID4gKwkJCQlfX2JpbmQo cG9zLCB0enAtPnRyaXBfbWFza1tpXSwgY2Rldik7DQo+ID4gPiA+ID4gPiArCQkJCWJyZWFrOw0K PiA+ID4gPiA+ID4gKwkJCX0NCj4gPiA+ID4gPiA+ICsJCX0NCj4gPiA+ID4gPiA+ICsJfQ0KPiA+ ID4gPiA+ID4gKwltdXRleF91bmxvY2soJnRoZXJtYWxfbGlzdF9sb2NrKTsNCj4gPiA+ID4gPiA+ ICt9DQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBJIHN0aWxsIGRvIG5vdCB1bmRlcnN0YW5kIHdoeSB3 ZSBuZWVkIHRoaXMga2luZCBvZiBiaW5kLg0KPiA+ID4gPiA+IFNheSwgdGhlIHBsYXRmb3JtIHRo ZXJtYWwgZHJpdmVyIGtub3dzIHRoZSBwbGF0Zm9ybSBkYXRhLCBpLmUuIGl0DQo+IGtub3dzDQo+ ID4gPiA+ID4gd2hpY2ggY29vbGluZyBkZXZpY2VzIHNob3VsZCBiZSBib3VuZCB0byB3aGljaCB0 cmlwIHBvaW50cy4NCj4gPiA+ID4gPiB3aHkgd2UgY2FuIG5vdCBtb3ZlIHRoaXMga2luZCBvZiBs b2dpYyB0byB0aGUgLmJpbmQoKSBjYWxsYmFjaywgb2ZmZXJlZA0KPiA+ID4gPiA+IGJ5IHRoZSBw bGF0Zm9ybSB0aGVybWFsIGRyaXZlcj8NCj4gPiA+ID4gPiBzYXksIGluIC5iaW5kKCkgY2FsbGJh Y2ssDQo+ID4gPiA+ID4gdGhlIHBsYXRmb3JtIHRoZXJtYWwgZHJpdmVyIGhhcyB0aGUgcG9pbnRl ciBvZiB0aGUgcGxhdGZvcm0gZGF0YSwNCj4gcmlnaHQ/DQo+ID4gPiA+ID4gdGhlIC5jZGV2IHBh cmFtZXRlciBjYW4gYmUgdXNlZCB0byBmaW5kIHRoZSBjb29saW5nIGRldmljZSBuYW1lLA0KPiA+ ID4gPiA+IGFuZCB3ZSBjYW4gbWFrZSB0aGUgY29tcGFyaXNvbiB0aGVyZS4gaW5zdGVhZCBvZiBp bnRyb2R1Y2luZyBuZXcNCj4gYmluZGluZw0KPiA+ID4gPiA+IGZ1bmN0aW9ucyBpbiB0aGUgZ2Vu ZXJpYyB0aGVybWFsIGxheWVyLg0KPiA+ID4gPg0KPiA+ID4gPiBGb3Igb25jZSwgSSBnb3QgbGl0 dGxlIGNvbmZ1c2VkIGJldHdlZW4gdGhlIGdlbmVyaWMgcGxhdGZvcm0gdGhlcm1hbA0KPiBzZW5z b3INCj4gPiA+ID4gZHJpdmVycyAodGhlIGNoaXAgZHJpdmVycykgYW5kIHRoZSBwbGF0Zm9ybSBs ZXZlbCBkcml2ZXIgKG5vdCBzcGVjaWZpYyBmb3INCj4gY2hpcCwNCj4gPiA+ID4gYnV0IGZvciBh IHBsYXRmb3JtKS4gU28sIHllcyB3ZSBjYW4gcHV0IHRoaXMgaW4gdGhlIHBsYXRmb3JtIGxldmVs IGRyaXZlci4NCj4gPiA+ID4NCj4gPiA+IEhtbSwNCj4gPiA+IEknbSBub3QgY2xlYXIgYWJvdXQg dGhlIGRpZmZlcmVuY2UgYmV0d2VlbiB0aGVzZSB0d28gZHJpdmVycy4NCj4gPiA+IHdoYXQgaXMg c3VwcG9zZWQgdG8gYmUgZG9uZSBpbiB0aGUgcGxhdGZvcm0gdGhlcm1hbCBzZW5zb3IgZHJpdmVy cyBhbmQNCj4gPiA+IHdoYXQgaXMgc3VwcG9zZWQgdG8gYmUgZG9uZSBpbiB0aGUgcGxhdGZvcm0g bGV2ZWwgZHJpdmVyPw0KPiA+DQo+ID4gQSBzZW5zb3IgZHJpdmVyIGNhbiBiZSBhIGdlbmVyaWMg Y2hpcCBkcml2ZXIgbGlrZSBlbWMxNDAzICh0aGlzIGlzIHRoZSBvbmUNCj4gPiB0aGF0IEkgaGF2 ZSB3b3JrZWQgb24uLikgb3IgY29yZXRlbXAgKHRoZSBDUFUgRFRTIGRyaXZlciBmb3IgeDg2KS4g VGhleSBzaXQNCj4gPiBpbiBkaWZmZXJlbnQgc3ViIHN5c3RlbXMgKHRoZXNlIHR3byBpbiBod21v bikuIFdlIG1pZ2h0IG5vdCBiZSBhbGxvd2VkDQo+IHRvDQo+ID4gYWRkIGFueSB0aGVybWFsIGZy YW1ld29yayBzcGVjaWZpYyBjb2RlIGluIHRoZXNlIGRyaXZlcnMuIFRoZSBzYW1lIGRyaXZlcg0K PiA+IHdvcmtzIG9uIGFsbCBwbGF0Zm9ybXMuDQo+IA0KPiBkb2VzIHRoZSBzZW5zb3Iga25vdyBh bnl0aGluZyBhYm91dCB0aGUgInBvbGljeSI/DQo+IFNheSwgZG9lcyBpdCBoYXZlIGFueSB0cmlw IHBvaW50cz8gZG9lcyBpdCBrbm93IHdoaWNoIGRldmljZSBjYW4gYmUNCj4gdGhyb3R0bGVkIHRv IGNvb2wgaXRzZWxmPw0KPiBJIHRoaW5rIHRoZSBhbnN3ZXIgaXMgIm5vIiwgcmlnaHQ/DQoNClll cy4gWW91IGFyZSByaWdodCA6LSkNClRoZSBhbnN3ZXIgaXMgJ05vJy4NCg0KPiA+DQo+ID4gQSBw bGF0Zm9ybSBsZXZlbCB0aGVybWFsIGRyaXZlciBrbm93cyBpbmZvcm1hdGlvbiBhYm91dCB0aGUg dGhlcm1hbA0KPiBzZW5zb3JzLA0KPiA+IGFuZCB0aGVpciB6b25lcyBvbiB0aGUgcGxhdGZvcm07 IGFuZCBpcyBzcGVjaWZpYyB0byB0aGUgcGxhdGZvcm0uDQo+ID4gRm9yIHg4NiwgdGhpcyB3aWxs IGJlIGluIGRyaXZlcnMveDg2L3BsYXRmb3JtLyB3aGVyZWFzIG1pZ2h0IGJlIGluIHNvbWUNCj4g b3RoZXINCj4gPiBwbGFjZSBmb3Igb3RoZXIgYXJjaGl0ZWN0dXJlcy4gQW4gZXhhbXBsZSBpcyBp bnRlbF9taWRfdGhlcm1hbC5jIHdoaWNoIHNpdHMNCj4gPiBpbiBkcml2ZXJzL3g4Ni9wbGF0Zm9y bS4gV2UgY2FuIGFkZCBvdXIgdGhlcm1hbCBmcmFtZXdvcmsgc3BlY2lmaWMgY29kZQ0KPiA+IHRv IHRoaXMgZHJpdmVyLg0KPiA+DQo+IGJ1dCBJIHRoaW5rIGludGVsX21pZGVfdGhlcm1hbCBkcml2 ZXIgaXMgYWxzbyBhIHBsYXRmb3JtIHRoZXJtYWwgc2Vuc29yDQo+IGRyaXZlciBhdCB0aGUgc2Ft ZSB0aW1lLg0KDQpZZXMgdG9kYXkgaXQgaXMgYm90aC4uDQoNCj4gPiA+DQo+ID4gPiBBdCBsZWFz dCBmb3Igbm93LCBhbGwgdGhlIHRoZXJtYWwgZHJpdmVycyBhcmUgYm90aCB0aGVybWFsIHNlbnNv ciBkcml2ZXINCj4gPiA+IGFuZCBwbGF0Zm9ybSBsZXZlbCBkcml2ZXIsIHJpZ2h0Pw0KPiA+DQo+ ID4gTm90IGFsbCB0aGUgdGltZXMsIGFsdGhvdWdoIHRoZXJlIGFyZSBzb21lIGluc3RhbmNlcyB3 aGVyZSBib3RoIGFyZSBzYW1lLg0KPiA+IFdlIHVzZSBjb3JldGVtcC5jIGFuZCBpbnRlbF9taWRf dGhlcm1hbC5jICh3aGljaCBhcmUgZGlmZmVyZW50KSwgZm9yIHRoZQ0KPiA+IHg4NiBtaWQgcGxh dGZvcm1zLg0KPiA+DQo+IHNvIHlvdSB3YW50IHRvIHVzZSBjb3JldGVtcC5jIGFzIGEgdGVtcGVy YXR1cmUgc2Vuc29yLCBhbmQgdGhlbiBiaW5kDQo+IHlvdXIgb3duIGNvb2xpbmcgZGV2aWNlcyB0 byBpdCBpbiB5b3VyIHBsYXRmb3JtIGxldmVsIHRoZXJtYWwgZHJpdmVyPw0KDQpZZXMsIGNvcmV0 ZW1wIGlzIG9uZSBmaW5lIGV4YW1wbGUuDQpBdCBsZWFzdCBJIHdvdWxkIGxpa2UgdG8gZ2V0IHRo ZSBzYW1lIHRoaW5nIGRvbmUgZm9yIGVtYzE0MDMuYw0KKGFuZCBmZXcgaHdtb24gZHJpdmVycyku Lg0KDQpUaGFua3MsDQpEdXJnYQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Thu, Aug 16, 2012 at 03:31:55AM +0000, R, Durgadoss wrote: > Hi Rui, > > > > -----Original Message----- > > From: Zhang, Rui > > Sent: Thursday, August 16, 2012 9:00 AM > > To: R, Durgadoss > > Cc: lenb@kernel.org; rjw@sisk.pl; linux-acpi@vger.kernel.org; linux- > > pm@vger.kernel.org; eduardo.valentin@ti.com; amit.kachhap@linaro.org; > > wni@nvidia.com > > Subject: RE: [PATCH 07/13] Thermal: Update binding logic based on platform > > data > > > > On ?, 2012-08-15 at 03:17 -0600, R, Durgadoss wrote: > > > Hi Rui, > > > > > > > > > > +static void update_bind_info(struct thermal_cooling_device > > *cdev) > > > > > > > +{ > > > > > > > + int i, ret; > > > > > > > + struct thermal_zone_params *tzp; > > > > > > > + struct thermal_zone_device *pos = NULL; > > > > > > > + > > > > > > > + mutex_lock(&thermal_list_lock); > > > > > > > + > > > > > > > + list_for_each_entry(pos, &thermal_tz_list, node) { > > > > > > > + if (!pos->tzp && !pos->ops->bind) > > > > > > > + continue; > > > > > > > + > > > > > > > + if (!pos->tzp && pos->ops->bind) { > > > > > > > + ret = pos->ops->bind(pos, cdev); > > > > > > > + if (ret) > > > > > > > + print_bind_err_msg(pos, cdev, ret); > > > > > > > + } > > > > > > > + > > > > > > > + tzp = pos->tzp; > > > > > > > + for (i = 0; i < tzp->num_cdevs; i++) { > > > > > > > + if (!strcmp(tzp->cdevs_name[i], cdev->type)) > > { > > > > > > > + __bind(pos, tzp->trip_mask[i], cdev); > > > > > > > + break; > > > > > > > + } > > > > > > > + } > > > > > > > + } > > > > > > > + mutex_unlock(&thermal_list_lock); > > > > > > > +} > > > > > > > > > > > > I still do not understand why we need this kind of bind. > > > > > > Say, the platform thermal driver knows the platform data, i.e. it > > knows > > > > > > which cooling devices should be bound to which trip points. > > > > > > why we can not move this kind of logic to the .bind() callback, offered > > > > > > by the platform thermal driver? > > > > > > say, in .bind() callback, > > > > > > the platform thermal driver has the pointer of the platform data, > > right? > > > > > > the .cdev parameter can be used to find the cooling device name, > > > > > > and we can make the comparison there. instead of introducing new > > binding > > > > > > functions in the generic thermal layer. > > > > > > > > > > For once, I got little confused between the generic platform thermal > > sensor > > > > > drivers (the chip drivers) and the platform level driver (not specific for > > chip, > > > > > but for a platform). So, yes we can put this in the platform level driver. > > > > > > > > > Hmm, > > > > I'm not clear about the difference between these two drivers. > > > > what is supposed to be done in the platform thermal sensor drivers and > > > > what is supposed to be done in the platform level driver? > > > > > > A sensor driver can be a generic chip driver like emc1403 (this is the one > > > that I have worked on..) or coretemp (the CPU DTS driver for x86). They sit > > > in different sub systems (these two in hwmon). We might not be allowed > > to > > > add any thermal framework specific code in these drivers. The same driver > > > works on all platforms. > > > > does the sensor know anything about the "policy"? > > Say, does it have any trip points? does it know which device can be > > throttled to cool itself? > > I think the answer is "no", right? > > Yes. You are right :-) > The answer is 'No'. > > > > > > > A platform level thermal driver knows information about the thermal > > sensors, > > > and their zones on the platform; and is specific to the platform. > > > For x86, this will be in drivers/x86/platform/ whereas might be in some > > other > > > place for other architectures. An example is intel_mid_thermal.c which sits > > > in drivers/x86/platform. We can add our thermal framework specific code > > > to this driver. > > > > > but I think intel_mide_thermal driver is also a platform thermal sensor > > driver at the same time. > > Yes today it is both.. So, what is the conclusion from above? I think we need to have a call here as it will drive how driver code is going to look like. So far, the way I am designing the OMAP thermal support is that the temp sensor drivers would know about how to cool the zones, at SoC level. But the cooling of a platform/end-product level would require another driver, which would require knowledge of availability of sensors and cooling devices, in order to define the board policy. > > > > > > > > > At least for now, all the thermal drivers are both thermal sensor driver > > > > and platform level driver, right? > > > > > > Not all the times, although there are some instances where both are same. > > > We use coretemp.c and intel_mid_thermal.c (which are different), for the > > > x86 mid platforms. > > > > > so you want to use coretemp.c as a temperature sensor, and then bind > > your own cooling devices to it in your platform level thermal driver? > > Yes, coretemp is one fine example. > At least I would like to get the same thing done for emc1403.c > (and few hwmon drivers).. > > Thanks, > Durga -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c index 195e529..748b12f 100644 --- a/drivers/thermal/thermal_sys.c +++ b/drivers/thermal/thermal_sys.c @@ -158,6 +158,107 @@ static void retrieve_zone_params(struct thermal_zone_device *tz) } } +static void print_bind_err_msg(struct thermal_zone_device *tz, + struct thermal_cooling_device *cdev, int ret) +{ + dev_err(&tz->device, "binding zone %s with cdev %s failed:%d\n", + tz->type, cdev->type, ret); +} + +static void __bind(struct thermal_zone_device *tz, int mask, + struct thermal_cooling_device *cdev) +{ + int i, ret; + + for (i = 0; i < tz->trips; i++) { + if (mask & (1 << i)) { + ret = thermal_zone_bind_cooling_device(tz, i, cdev, + THERMAL_NO_LIMIT, THERMAL_NO_LIMIT); + if (ret) + print_bind_err_msg(tz, cdev, ret); + } + } +} + +static void __unbind(struct thermal_zone_device *tz, int mask, + struct thermal_cooling_device *cdev) +{ + int i; + + for (i = 0; i < tz->trips; i++) + if (mask & (1 << i)) + thermal_zone_unbind_cooling_device(tz, i, cdev); +} + +static void update_bind_info(struct thermal_cooling_device *cdev) +{ + int i, ret; + struct thermal_zone_params *tzp; + struct thermal_zone_device *pos = NULL; + + mutex_lock(&thermal_list_lock); + + list_for_each_entry(pos, &thermal_tz_list, node) { + if (!pos->tzp && !pos->ops->bind) + continue; + + if (!pos->tzp && pos->ops->bind) { + ret = pos->ops->bind(pos, cdev); + if (ret) + print_bind_err_msg(pos, cdev, ret); + } + + tzp = pos->tzp; + for (i = 0; i < tzp->num_cdevs; i++) { + if (!strcmp(tzp->cdevs_name[i], cdev->type)) { + __bind(pos, tzp->trip_mask[i], cdev); + break; + } + } + } + mutex_unlock(&thermal_list_lock); +} + +static void do_binding(struct thermal_zone_device *tz) +{ + int i, ret; + char *name; + struct thermal_cooling_device *pos = NULL; + struct thermal_zone_params *tzp = tz->tzp; + + if (!tzp && !tz->ops->bind) + return; + + /* If there is no platform data, try to use ops->bind */ + if (!tzp && tz->ops->bind) { + mutex_lock(&thermal_list_lock); + + list_for_each_entry(pos, &thermal_cdev_list, node) { + ret = tz->ops->bind(tz, pos); + if (ret) + print_bind_err_msg(tz, pos, ret); + } + + mutex_unlock(&thermal_list_lock); + return; + } + + /* If platform data is available, use it to create binding */ + for (i = 0; i < tzp->num_cdevs; i++) { + name = tzp->cdevs_name[i]; + pos = get_cdev_by_name(name); + + if (!pos) { + dev_err(&tz->device, "cannot find cdev %s\n", name); + continue; + } + + mutex_lock(&thermal_list_lock); + __bind(tz, tzp->trip_mask[i], pos); + mutex_unlock(&thermal_list_lock); + } +} + /* sys I/F for thermal zone */ #define to_thermal_zone(_dev) \ @@ -975,7 +1076,6 @@ thermal_cooling_device_register(char *type, void *devdata, const struct thermal_cooling_device_ops *ops) { struct thermal_cooling_device *cdev; - struct thermal_zone_device *pos; int result; if (strlen(type) >= THERMAL_NAME_LENGTH) @@ -1025,20 +1125,15 @@ thermal_cooling_device_register(char *type, void *devdata, if (result) goto unregister; + /* Add 'this' new cdev to the global cdev list */ mutex_lock(&thermal_list_lock); list_add(&cdev->node, &thermal_cdev_list); - list_for_each_entry(pos, &thermal_tz_list, node) { - if (!pos->ops->bind) - continue; - result = pos->ops->bind(pos, cdev); - if (result) - break; - - } mutex_unlock(&thermal_list_lock); - if (!result) - return cdev; + /* Update binding information for 'this' new cdev */ + update_bind_info(cdev); + + return cdev; unregister: release_idr(&thermal_cdev_idr, &thermal_idr_lock, cdev->id); @@ -1054,10 +1149,10 @@ EXPORT_SYMBOL(thermal_cooling_device_register); * thermal_cooling_device_unregister() must be called when the device is no * longer needed. */ -void thermal_cooling_device_unregister(struct - thermal_cooling_device - *cdev) +void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev) { + int i; + struct thermal_zone_params *tzp; struct thermal_zone_device *tz; struct thermal_cooling_device *pos = NULL; @@ -1074,12 +1169,23 @@ void thermal_cooling_device_unregister(struct return; } list_del(&cdev->node); + + /* Unbind all thermal zones associated with 'this' cdev */ list_for_each_entry(tz, &thermal_tz_list, node) { - if (!tz->ops->unbind) + tzp = tz->tzp; + if (!tzp && !tz->ops->unbind) continue; - tz->ops->unbind(tz, cdev); + + if (!tzp && tz->ops->unbind) + tz->ops->unbind(tz, cdev); + + for (i = 0; i < tzp->num_cdevs; i++) + if (!strcmp(cdev->type, tzp->cdevs_name[i])) + __unbind(tz, tzp->trip_mask[i], cdev); } + mutex_unlock(&thermal_list_lock); + if (cdev->type[0]) device_remove_file(&cdev->device, &dev_attr_cdev_type); device_remove_file(&cdev->device, &dev_attr_max_state); @@ -1424,7 +1530,6 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, int passive_delay, int polling_delay) { struct thermal_zone_device *tz; - struct thermal_cooling_device *pos; enum thermal_trip_type trip_type; int result; int count; @@ -1519,14 +1624,12 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, mutex_lock(&thermal_list_lock); list_add_tail(&tz->node, &thermal_tz_list); - if (ops->bind) - list_for_each_entry(pos, &thermal_cdev_list, node) { - result = ops->bind(tz, pos); - if (result) - break; - } mutex_unlock(&thermal_list_lock); + /* Bind cooling devices for this zone */ + do_binding(tz); + + INIT_DELAYED_WORK(&(tz->poll_queue), thermal_zone_device_check); thermal_zone_device_update(tz); @@ -1547,12 +1650,16 @@ EXPORT_SYMBOL(thermal_zone_device_register); */ void thermal_zone_device_unregister(struct thermal_zone_device *tz) { + int i; + struct thermal_zone_params *tzp; struct thermal_cooling_device *cdev; struct thermal_zone_device *pos = NULL; if (!tz) return; + tzp = tz->tzp; + mutex_lock(&thermal_list_lock); list_for_each_entry(pos, &thermal_tz_list, node) if (pos == tz) @@ -1563,9 +1670,20 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz) return; } list_del(&tz->node); - if (tz->ops->unbind) - list_for_each_entry(cdev, &thermal_cdev_list, node) - tz->ops->unbind(tz, cdev); + + /* Unbind all cdevs associated with 'this' thermal zone */ + list_for_each_entry(cdev, &thermal_cdev_list, node) { + if (!tzp && !tz->ops->unbind) + break; + + if (!tzp && tz->ops->unbind) + tz->ops->unbind(tz, cdev); + + for (i = 0; i < tzp->num_cdevs; i++) + if (!strcmp(cdev->type, tzp->cdevs_name[i])) + __unbind(tz, tzp->trip_mask[i], cdev); + } + mutex_unlock(&thermal_list_lock); thermal_zone_device_set_polling(tz, 0);
This patch updates the binding logic in thermal_sys.c It uses the platform layer data to bind a thermal zone to a cdev for a particular trip point. * If we do not have platform data and do not have .bind defined, do not bind. * If we do not have platform data but .bind is defined, then use tz->ops->bind. * If we have platform data, use it to create binding. The same logic sequence is followed for unbind also. Signed-off-by: Durgadoss R <durgadoss.r@intel.com> --- drivers/thermal/thermal_sys.c | 170 ++++++++++++++++++++++++++++++++++------- 1 file changed, 144 insertions(+), 26 deletions(-)