linux-kernel - Re: [PATCH] PM / OPP: Implement free_opp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 20 May 2014 06:25:42 -0500
From:	Nishanth Menon <nm@...com>
To:	Inderpal Singh <inderpal.s@...sung.com>
Cc:	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Pavel Machek <pavel@....cz>,
	Viresh Kumar <viresh.kumar@...aro.org>
Subject: Re: [PATCH] PM / OPP: Implement free_opp_table function

On Tue, May 20, 2014 at 12:36 AM, Inderpal Singh <inderpal.s@...sung.com> wrote:
> On Tue, May 20, 2014 at 12:04 AM, Nishanth Menon <nm@...com> wrote:
>> On Mon, May 19, 2014 at 1:08 PM, Inderpal Singh <inderpal.s@...sung.com> wrote:
>>> Hi Nishanth,
>>>
>>> Thanks for the review comments.
>>>
>>> On Mon, May 19, 2014 at 6:43 PM, Nishanth Menon <nm@...com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/