Jean Pihet <jean.pihet@xxxxxxxxxxxxxx> 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
ÿô.nlj·Ÿ®‰†+%ŠË±é¥Šwÿº{.nlj·¥Š{±þ‰š§ø¡Ü}©ž²ÆzÚj:+v‰¨þø®w¥þŠàÞ¨è&¢)ß«a¶Úÿûz¹ÞúŽŠÝjÿŠwèf
