Re: [PATCH] PM / OPP: Implement free_opp_table function

[Nishanth Menon]

On Tue, May 20, 2014 at 12:36 AM, Inderpal Singh <inderpal.s@xxxxxxxxxxx> wrote:
> On Tue, May 20, 2014 at 12:04 AM, Nishanth Menon <nm@xxxxxx> wrote:
>> On Mon, May 19, 2014 at 1:08 PM, Inderpal Singh <inderpal.s@xxxxxxxxxxx> wrote:
>>> Hi Nishanth,
>>>
>>> Thanks for the review comments.
>>>
>>> On Mon, May 19, 2014 at 6:43 PM, Nishanth Menon <nm@xxxxxx> wrote:
>>>> On 05/16/2014 04:09 AM, Inderpal Singh wrote:

[..]
>>>>> +     /* Hold our list modification lock here */
>>>>> +     mutex_lock(&dev_opp_list_lock);
>>>>> +
>>>>> +     /* Check for existing list for 'dev' */
>>>>> +     dev_opp = find_device_opp(dev);
>>>>> +     if (IS_ERR(dev_opp)) {
>>>>> +             mutex_unlock(&dev_opp_list_lock);
>>>>> +             return;
>>>>> +     }
>>>>> +
>>>>> +     while (!list_empty(&dev_opp->opp_list)) {
>>>>> +             opp = list_entry_rcu(dev_opp->opp_list.next,
>>>>> +                                     struct dev_pm_opp, node);
>>>>> +             list_del_rcu(&opp->node);
>>>>> +             kfree_rcu(opp, head);
>>>>> +     }
>>>>
>>>> How about the OPP notifiers? should'nt we add a new event
>>>> OPP_EVENT_REMOVE?
>>>>
>>>
>>> As this function is to free the whole opp table. Hence, I think,
>>> notifier may not be needed. It may be required for per opp removal as
>>> is the case with opp addition and enable/disable. But at present there
>>> are no users of these notifiers at all. Let me know your view.
>>
>> umm.. we do have devfreq which depends on OPPs :).
>
> Yes, devfreq does depend on OPPs, but no devfreq driver is registering
> its notifier_block to handle OPP notifications.
>

Lets not forget the power of downstream tree drivers that use the API set :)

>>
>>>> To maintain non-dt behavior coherency, should'nt we rather add a
>>>> opp_remove or an opp_del function?
>>>
>>> Yes we should have opp_remove as well, but what's the use case ?
>>> Should we go ahead and implement it Or, wait for the use-case?
>>
>> IMHO, if we are doing it properly, we should add the requisite
>> function as well. we dont want to have differing behavior device tree
>> Vs non-DT.
>
> So we will have 2 functions then. One to remove the whole opp table
> and the the other to remove the individual OPPs.
> I will cover this in v2. Will also take care of the OPP_EVENT_REMOVE
> notification part.
>

Thanks.

--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/