From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Hilman Subject: Re: [PATCH v2 4/7] OMAP: PM CONSTRAINTS: implement the constraints management code Date: Fri, 18 Mar 2011 08:06:47 -0700 Message-ID: <87ipvgo4c8.fsf@ti.com> References: <1299779247-20511-1-git-send-email-j-pihet@ti.com> <1299779247-20511-5-git-send-email-j-pihet@ti.com> <874o71zdpu.fsf@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: base64 Return-path: Received: from na3sys009aog108.obsmtp.com ([74.125.149.199]:51003 "EHLO na3sys009aog108.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752724Ab1CRPGv (ORCPT ); Fri, 18 Mar 2011 11:06:51 -0400 Received: by mail-iw0-f180.google.com with SMTP id 6so6408714iwn.25 for ; Fri, 18 Mar 2011 08:06:50 -0700 (PDT) In-Reply-To: (Jean Pihet's message of "Fri, 18 Mar 2011 12:33:22 +0100") Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Jean Pihet Cc: linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, paul@pwsan.com, Jean Pihet SmVhbiBQaWhldCA8amVhbi5waWhldEBuZXdvbGRiaXRzLmNvbT4gd3JpdGVzOg0KDQpbLi4uXQ0K DQo+Pj4gK2V4aXRfb2s6DQo+Pj4gKyDCoCDCoCAvKiBGaW5kIHRoZSBzdHJvbmdlc3QgY29uc3Ry YWludCBmb3IgdGhlIGdpdmVuIHRhcmdldCAqLw0KPj4+ICsgwqAgwqAgcmV0ID0gMDsNCj4+PiAr IMKgIMKgIGlmIChhc2NlbmRpbmcpIHsNCj4+PiArIMKgIMKgIMKgIMKgIMKgIMKgIGxpc3RfZm9y X2VhY2hfZW50cnkodXNlciwgJihjb25zdHJhaW50c19saXN0KS0+bm9kZV9saXN0LA0KPj4+ICsg wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgbm9kZS5wbGlz dC5ub2RlX2xpc3QpIHsNCj4+PiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIC8qIEZp bmQgdGhlIGxvd2VzdCAoaS5lLiBmaXJzdCkgdmFsdWUgZm9yIHRoZSB0YXJnZXQgKi8NCj4+PiAr IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGlmICh1c2VyLT50YXJnZXQgPT0gdGFyZ2V0 KSB7DQo+Pj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCByZXQg PSB1c2VyLT5jb25zdHJhaW50X3ZhbHVlOw0KPj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg wqAgwqAgwqAgwqAgwqAgwqAgYnJlYWs7DQo+Pj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC oCDCoCB9DQo+Pj4gKyDCoCDCoCDCoCDCoCDCoCDCoCB9DQo+Pj4gKyDCoCDCoCB9IGVsc2Ugew0K Pj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgbGlzdF9mb3JfZWFjaF9lbnRyeV9yZXZlcnNlKHVzZXIs DQo+Pj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCAmKGNvbnN0cmFpbnRzX2xpc3QpLT5ub2RlX2xpc3QsDQo+Pj4gKyDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBub2Rl LnBsaXN0Lm5vZGVfbGlzdCkgew0KPj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg LyogRmluZCB0aGUgaGlnaGVzdCAoaS5lLiBsYXN0KSB2YWx1ZSBmb3IgdGhlIHRhcmdldCAqLw0K Pj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgaWYgKHVzZXItPnRhcmdldCA9PSB0 YXJnZXQpIHsNCj4+PiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg IHJldCA9IHVzZXItPmNvbnN0cmFpbnRfdmFsdWU7DQo+Pj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBicmVhazsNCj4+PiArIMKgIMKgIMKgIMKgIMKgIMKgIMKg IMKgIMKgIMKgIH0NCj4+PiArIMKgIMKgIMKgIMKgIMKgIMKgIH0NCj4+PiArIMKgIMKgIH0NCj4+ DQo+PiBIbW0sIHdoeSBjYW4ndCB5b3UgdXNlIHBsaXN0X2ZpcnN0KCkgYW5kIHBsaXN0X2xhc3Qo KSBoZXJlPw0KPiBCZWNhdXNlIHRoZSBwbGlzdCBpcyBzb3J0ZWQgYnkgdGhlICdwcmlvJyBmaWVs ZCAod2hpY2ggaXMgJ3ZhbHVlJyBpbg0KPiB0aGlzIGNvZGUpIGFuZCB0aGlzIGZ1bmN0aW9uIHNl YXJjaGVzIGZvciB0aGUgc3Ryb25nZXN0IGNvbnN0cmFpbnQgZm9yDQo+IGEgZ2l2ZW4gdGFyZ2V0 LiBTbyBpdCBpcyBuZWVkZWQgdG8gaXRlcmF0ZSB0aHJvdWdoIHRoZSBsaXN0IGluIG9yZGVyDQo+ IHRvIGZpbmQgdGhlIGZpcnN0IChvciBsYXN0KSBjb25zdHJhaW50IHdpdGggdGhlIHJpZ2h0IHRh cmdldC4NCg0KSG1tLCBzdGlsbCBjb25mdXNpbmcuDQoNClRoZSBtYWluIHRoaW5nIEkgZG9uJ3Qg Z2V0IGlzIHdoeSB5b3UgbmVlZCB0aGUgJ3RhcmdldCcgZmllbGQgaW4gdGhlDQpmaXJzdCBwbGFj ZS4gIFlvdSBzaG91bGQganVzdCBrZWVwIGEgbGlzdCBvZiBjb25zdHJhaW50cyBwZXIgdGFyZ2V0 DQpvbWFwX2RldmljZS4gIElPVywgY3VycmVudGx5IHlvdXIgX3drdXBfbGF0X2NvbnN0cmFpbnRz X2xpc3QgaXMgYSBnbG9iYWwNCmxpc3QsIGJ1dCBpbnN0ZWFkIGl0IHNob3VsZCBiZSBjb25uZWN0 ZWQgdG8gdGhlIG9tYXBfZGV2aWNlLCBhbmQgZWFjaA0Kb21hcF9kZXZpY2Ugd291bGQgaGF2ZSBh IHBsaXN0IGZvciBlYWNoIGNsYXNzLg0KDQoNCj4+DQo+Pj4gK2V4aXRfZXJyb3I6DQo+Pj4gKyDC oCDCoCBtdXRleF91bmxvY2soJl9jb25zdHJhaW50c19tdXRleCk7DQo+Pj4gKw0KPj4+ICsgwqAg wqAgcmV0dXJuIHJldDsNCj4+PiArfQ0KPj4+DQo+Pj4gwqAvKiBQdWJsaWMgZnVuY3Rpb25zIGZv ciB1c2UgYnkgY29yZSBjb2RlICovDQo+Pj4NCj4+PiDCoC8qKg0KPj4+ICsgKiBvbWFwX2Rldmlj ZV9zZXRfZGV2X2NvbnN0cmFpbnQgLSBzZXQvcmVsZWFzZSBhIGRldmljZSBjb25zdHJhaW50DQo+ Pj4gKyAqIEBjbGFzczogY29uc3RyYWludCBjbGFzcw0KPj4+ICsgKiBAcmVxX2RldjogY29uc3Ry YWludCByZXF1ZXN0ZXIsIHVzZWQgZm9yIHRyYWNraW5nIHRoZSBjb25zdHJhaW50cw0KPj4+ICsg KiBAZGV2OiBkZXZpY2UgdG8gYXBwbHkgdGhlIGNvbnN0cmFpbnQgdG8uIE11c3QgaGF2ZSBhbiBh c3NvY2lhdGVkIG9tYXBfZGV2aWNlDQo+Pj4gKyAqIEB0OiBjb25zdHJhaW50IHZhbHVlLiBBIHZh bHVlIG9mIC0xIHJlbW92ZXMgdGhlIGNvbnN0cmFpbnQuDQo+Pj4gKyAqDQo+Pj4gKyAqIFVzaW5n IHRoZSBwcmltYXJ5IGh3bW9kLCBzZXQvcmVsZWFzZSBhIGRldmljZSBjb25zdHJhaW50IGZvciB0 aGUgZGV2DQo+Pj4gKyAqIGRldmljZSwgcmVxdWVzdGVkIGJ5IHRoZSByZXFfZGV2IGRldmljZS4g RGVwZW5kaW5nIG9mIHRoZSBjb25zdHJhaW50IGNsYXNzDQo+Pj4gKyAqIHRoaXMgY29kZSBjYWxs cyB0aGUgYXBwcm9wcmlhdGUgbG93IGxldmVsIGNvZGUsIGUuZy4gcG93ZXIgZG9tYWluIGZvcg0K Pj4+ICsgKiB0aGUgd2FrZS11cCBsYXRlbmN5IGNvbnN0cmFpbnRzLg0KPj4+ICsgKg0KPj4+ICsg KiBJZiBhbnkgaHdtb2RzIGV4aXN0IGZvciB0aGUgb21hcF9kZXZpY2UgYXNzb2lhdGVkIHdpdGgg QGRldiwNCj4+PiArICogc2V0L3JlbGVhc2UgdGhlIGNvbnN0cmFpbnQgZm9yIHRoZSBjb3JyZXNw b25kaW5nIGh3bW9kcywgb3RoZXJ3aXNlIHJldHVybg0KPj4+ICsgKiAtRUlOVkFMLg0KPj4+ICsg Ki8NCj4+PiAraW50IG9tYXBfZGV2aWNlX3NldF9kZXZfY29uc3RyYWludChlbnVtIG9tYXBfcG1f Y29uc3RyYWludF9jbGFzcyBjbGFzcywNCj4+PiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg IMKgIMKgIMKgIMKgIMKgIMKgIMKgc3RydWN0IGRldmljZSAqcmVxX2RldiwNCj4+PiArIMKgIMKg IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgc3RydWN0IGRldmljZSAq ZGV2LCBsb25nIHQpDQo+Pj4gK3sNCj4+PiArIMKgIMKgIHN0cnVjdCBvbWFwX2RldmljZSAqb2Q7 DQo+Pj4gKyDCoCDCoCBzdHJ1Y3Qgb21hcF9od21vZCAqb2g7DQo+Pj4gKyDCoCDCoCBzdHJ1Y3Qg cGxhdGZvcm1fZGV2aWNlICpwZGV2Ow0KPj4+ICsgwqAgwqAgc3RydWN0IHBvd2VyZG9tYWluICpw d3JkbSA9IE5VTEw7DQo+Pj4gKyDCoCDCoCB1MzIgcmV0ID0gLUVJTlZBTDsNCj4+PiArDQo+Pj4g KyDCoCDCoCAvKiBMb29rIGZvciB0aGUgcGxhdGZvcm0gZGV2aWNlIGZvciBkZXYgKi8NCj4+PiAr IMKgIMKgIHBkZXYgPSB0b19wbGF0Zm9ybV9kZXZpY2UoZGV2KTsNCj4+PiArDQo+Pj4gKyDCoCDC oCAvKiBUcnkgdG8gY2F0Y2ggbm9uIHBsYXRmb3JtIGRldmljZXMuICovDQo+Pj4gKyDCoCDCoCBp ZiAocGRldi0+bmFtZSA9PSBOVUxMKSB7DQo+Pg0KPj4gVGhpcyBzaG91bGQgY2hlY2sgZm9yIGEg dmFsaWQgb21hcF9kZXZpY2UsIG5vdCBwbGF0Zm9ybV9kZXZpY2UuDQo+IFRoaXMgY2hlY2sgcmVt YWlucyBidXQgdGhlIGNoZWNrIGJlbG93IGlzIGNoYW5nZWQsIHNlZSBiZWxvdy4NCg0KV2hhdCBJ IG1lYW4gaXMgdGhpcyBzaG91bGQgY2hlY2sgZm9yIG9tYXBfZGV2aWNlX3BhcmVudCB0byBzZWUg aWYgdGhlDQpzdHJ1Y3QgZGV2aWNlIGlzIGFuIG9tYXBfZGV2aWNlLiAgIElPVywgeW91IGNhcmUg d2hldGhlciBvciBub3QgaXQncyBhbg0Kb21hcF9kZXZpY2UsIG5vdCB3aGV0aGVyIG9yIG5vdCBp dCdzIGEgdmFsaWQgcGxhdGZvcm1fZGV2aWNlLg0KDQpLZXZpbg0K From mboxrd@z Thu Jan 1 00:00:00 1970 From: khilman@ti.com (Kevin Hilman) Date: Fri, 18 Mar 2011 08:06:47 -0700 Subject: [PATCH v2 4/7] OMAP: PM CONSTRAINTS: implement the constraints management code In-Reply-To: (Jean Pihet's message of "Fri, 18 Mar 2011 12:33:22 +0100") References: <1299779247-20511-1-git-send-email-j-pihet@ti.com> <1299779247-20511-5-git-send-email-j-pihet@ti.com> <874o71zdpu.fsf@ti.com> Message-ID: <87ipvgo4c8.fsf@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Jean Pihet writes: [...] >>> +exit_ok: >>> + ? ? /* Find the strongest constraint for the given target */ >>> + ? ? ret = 0; >>> + ? ? if (ascending) { >>> + ? ? ? ? ? ? list_for_each_entry(user, &(constraints_list)->node_list, >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? node.plist.node_list) { >>> + ? ? ? ? ? ? ? ? ? ? /* Find the lowest (i.e. first) value for the target */ >>> + ? ? ? ? ? ? ? ? ? ? if (user->target == target) { >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret = user->constraint_value; >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? break; >>> + ? ? ? ? ? ? ? ? ? ? } >>> + ? ? ? ? ? ? } >>> + ? ? } else { >>> + ? ? ? ? ? ? list_for_each_entry_reverse(user, >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &(constraints_list)->node_list, >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? node.plist.node_list) { >>> + ? ? ? ? ? ? ? ? ? ? /* Find the highest (i.e. last) value for the target */ >>> + ? ? ? ? ? ? ? ? ? ? if (user->target == target) { >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret = user->constraint_value; >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? break; >>> + ? ? ? ? ? ? ? ? ? ? } >>> + ? ? ? ? ? ? } >>> + ? ? } >> >> Hmm, why can't you use plist_first() and plist_last() here? > Because the plist is sorted by the 'prio' field (which is 'value' in > this code) and this function searches for the strongest constraint for > a given target. So it is needed to iterate through the list in order > to find the first (or last) constraint with the right target. Hmm, still confusing. The main thing I don't get is why you need the 'target' field in the first place. You should just keep a list of constraints per target omap_device. IOW, currently your _wkup_lat_constraints_list is a global list, but instead it should be connected to the omap_device, and each omap_device would have a plist for each class. >> >>> +exit_error: >>> + ? ? mutex_unlock(&_constraints_mutex); >>> + >>> + ? ? return ret; >>> +} >>> >>> ?/* Public functions for use by core code */ >>> >>> ?/** >>> + * omap_device_set_dev_constraint - set/release a device constraint >>> + * @class: constraint class >>> + * @req_dev: constraint requester, used for tracking the constraints >>> + * @dev: device to apply the constraint to. Must have an associated omap_device >>> + * @t: constraint value. A value of -1 removes the constraint. >>> + * >>> + * Using the primary hwmod, set/release a device constraint for the dev >>> + * device, requested by the req_dev device. Depending of the constraint class >>> + * this code calls the appropriate low level code, e.g. power domain for >>> + * the wake-up latency constraints. >>> + * >>> + * If any hwmods exist for the omap_device assoiated with @dev, >>> + * set/release the constraint for the corresponding hwmods, otherwise return >>> + * -EINVAL. >>> + */ >>> +int omap_device_set_dev_constraint(enum omap_pm_constraint_class class, >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device *req_dev, >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device *dev, long t) >>> +{ >>> + ? ? struct omap_device *od; >>> + ? ? struct omap_hwmod *oh; >>> + ? ? struct platform_device *pdev; >>> + ? ? struct powerdomain *pwrdm = NULL; >>> + ? ? u32 ret = -EINVAL; >>> + >>> + ? ? /* Look for the platform device for dev */ >>> + ? ? pdev = to_platform_device(dev); >>> + >>> + ? ? /* Try to catch non platform devices. */ >>> + ? ? if (pdev->name == NULL) { >> >> This should check for a valid omap_device, not platform_device. > This check remains but the check below is changed, see below. What I mean is this should check for omap_device_parent to see if the struct device is an omap_device. IOW, you care whether or not it's an omap_device, not whether or not it's a valid platform_device. Kevin