[PATCH v2 4/7] OMAP: PM CONSTRAINTS: implement the constraints management code
Kevin Hilman
khilman at ti.com
Fri Mar 18 11:06:47 EDT 2011
Jean Pihet <jean.pihet at newoldbits.com> 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
More information about the linux-arm-kernel
mailing list