Re: [PATCH] PM / OPP: Implement free_opp_table function
On Tue, May 20, 2014 at 12:36 AM, Inderpal Singh wrote:
> On Tue, May 20, 2014 at 12:04 AM, Nishanth Menon wrote:
>> On Mon, May 19, 2014 at 1:08 PM, Inderpal Singh
>> wrote:
>>> Hi Nishanth,
>>>
>>> Thanks for the review comments.
>>>
>>> On Mon, May 19, 2014 at 6:43 PM, Nishanth Menon 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
On Tue, May 20, 2014 at 12:04 AM, Nishanth Menon wrote:
> On Mon, May 19, 2014 at 1:08 PM, Inderpal Singh
> wrote:
>> Hi Nishanth,
>>
>> Thanks for the review comments.
>>
>> On Mon, May 19, 2014 at 6:43 PM, Nishanth Menon wrote:
>>> On 05/16/2014 04:09 AM, Inderpal Singh wrote:
At the driver unloading time the associated opp table may need
to be deleted. Otherwise it amounts to memory leak. The existing
OPP library does not have provison to do so.
Hence this patch implements the function to free the opp table.
Signed-off-by: Inderpal Singh
---
drivers/base/power/opp.c | 41 +
include/linux/pm_opp.h | 6 ++
2 files changed, 47 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index d9e376a..d45ffd5 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -654,4 +654,45 @@ int of_init_opp_table(struct device *dev)
return 0;
}
EXPORT_SYMBOL_GPL(of_init_opp_table);
+
+/**
+ * dev_pm_opp_free_opp_table() - free the opp table
+ * @dev: device for which we do this operation
+ *
+ * Free up the allocated opp table
+ *
+ * Locking: The internal device_opp and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex
locks to
+ * keep the integrity of the internal data structures. Callers should
ensure
+ * that this function is *NOT* called under RCU protection or in contexts
where
+ * mutex locking or synchronize_rcu() blocking calls cannot be used.
+ */
+void dev_pm_opp_free_opp_table(struct device *dev)
+{
+ struct device_opp *dev_opp = NULL;
+ struct dev_pm_opp *opp;
+
>>> if (!dev)
>>> return;
>>>
>>
>> missed it. Will take care in the next version.
>>
+ /* 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.
>
>>> 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.
Regards,
Inder
>
> Regards,
> Nishanth Menon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
On Mon, May 19, 2014 at 1:08 PM, Inderpal Singh wrote:
> Hi Nishanth,
>
> Thanks for the review comments.
>
> On Mon, May 19, 2014 at 6:43 PM, Nishanth Menon wrote:
>> On 05/16/2014 04:09 AM, Inderpal Singh wrote:
>>> At the driver unloading time the associated opp table may need
>>> to be deleted. Otherwise it amounts to memory leak. The existing
>>> OPP library does not have provison to do so.
>>>
>>> Hence this patch implements the function to free the opp table.
>>>
>>> Signed-off-by: Inderpal Singh
>>> ---
>>> drivers/base/power/opp.c | 41 +
>>> include/linux/pm_opp.h | 6 ++
>>> 2 files changed, 47 insertions(+)
>>>
>>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>>> index d9e376a..d45ffd5 100644
>>> --- a/drivers/base/power/opp.c
>>> +++ b/drivers/base/power/opp.c
>>> @@ -654,4 +654,45 @@ int of_init_opp_table(struct device *dev)
>>> return 0;
>>> }
>>> EXPORT_SYMBOL_GPL(of_init_opp_table);
>>> +
>>> +/**
>>> + * dev_pm_opp_free_opp_table() - free the opp table
>>> + * @dev: device for which we do this operation
>>> + *
>>> + * Free up the allocated opp table
>>> + *
>>> + * Locking: The internal device_opp and opp structures are RCU protected.
>>> + * Hence this function internally uses RCU updater strategy with mutex
>>> locks to
>>> + * keep the integrity of the internal data structures. Callers should
>>> ensure
>>> + * that this function is *NOT* called under RCU protection or in contexts
>>> where
>>> + * mutex locking or synchronize_rcu() blocking calls cannot be used.
>>> + */
>>> +void dev_pm_opp_free_opp_table(struct device *dev)
>>> +{
>>> + struct device_opp *dev_opp = NULL;
>>> + struct dev_pm_opp *opp;
>>> +
>> if (!dev)
>> return;
>>
>
> missed it. Will take care in the next version.
>
>>> + /* 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 :).
>> 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.
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
Hi Nishanth,
Thanks for the review comments.
On Mon, May 19, 2014 at 6:43 PM, Nishanth Menon wrote:
> On 05/16/2014 04:09 AM, Inderpal Singh wrote:
>> At the driver unloading time the associated opp table may need
>> to be deleted. Otherwise it amounts to memory leak. The existing
>> OPP library does not have provison to do so.
>>
>> Hence this patch implements the function to free the opp table.
>>
>> Signed-off-by: Inderpal Singh
>> ---
>> drivers/base/power/opp.c | 41 +
>> include/linux/pm_opp.h | 6 ++
>> 2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>> index d9e376a..d45ffd5 100644
>> --- a/drivers/base/power/opp.c
>> +++ b/drivers/base/power/opp.c
>> @@ -654,4 +654,45 @@ int of_init_opp_table(struct device *dev)
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(of_init_opp_table);
>> +
>> +/**
>> + * dev_pm_opp_free_opp_table() - free the opp table
>> + * @dev: device for which we do this operation
>> + *
>> + * Free up the allocated opp table
>> + *
>> + * Locking: The internal device_opp and opp structures are RCU protected.
>> + * Hence this function internally uses RCU updater strategy with mutex
>> locks to
>> + * keep the integrity of the internal data structures. Callers should ensure
>> + * that this function is *NOT* called under RCU protection or in contexts
>> where
>> + * mutex locking or synchronize_rcu() blocking calls cannot be used.
>> + */
>> +void dev_pm_opp_free_opp_table(struct device *dev)
>> +{
>> + struct device_opp *dev_opp = NULL;
>> + struct dev_pm_opp *opp;
>> +
> if (!dev)
> return;
>
missed it. Will take care in the next version.
>> + /* 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.
> 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?
Thanks,
Inder
>
>> +
>> + list_del_rcu(&dev_opp->node);
>> + mutex_unlock(&dev_opp_list_lock);
>> + synchronize_rcu();
>> + kfree(dev_opp);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_opp_free_opp_table);
>> #endif
>> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
>> index 0330217..3c29620 100644
>> --- a/include/linux/pm_opp.h
>> +++ b/include/linux/pm_opp.h
>> @@ -50,6 +50,8 @@ int dev_pm_opp_enable(struct device *dev, unsigned long
>> freq);
>> int dev_pm_opp_disable(struct device *dev, unsigned long freq);
>>
>> struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
>> +
>> +void dev_pm_opp_free_opp_table(struct device *dev);
>> #else
>> static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
>> {
>> @@ -105,6 +107,10 @@ static inline struct srcu_notifier_head
>> *dev_pm_opp_get_notifier(
>> {
>> return ERR_PTR(-EINVAL);
>> }
>> +
>> +void dev_pm_opp_free_opp_table(struct device *dev)
>> +{
>> +}
>> #endif /* CONFIG_PM_OPP */
>>
>> #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
>>
>
>
> --
> Regards,
> Nishanth Menon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
On 05/16/2014 04:09 AM, Inderpal Singh wrote:
> At the driver unloading time the associated opp table may need
> to be deleted. Otherwise it amounts to memory leak. The existing
> OPP library does not have provison to do so.
>
> Hence this patch implements the function to free the opp table.
>
> Signed-off-by: Inderpal Singh
> ---
> drivers/base/power/opp.c | 41 +
> include/linux/pm_opp.h | 6 ++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index d9e376a..d45ffd5 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -654,4 +654,45 @@ int of_init_opp_table(struct device *dev)
> return 0;
> }
> EXPORT_SYMBOL_GPL(of_init_opp_table);
> +
> +/**
> + * dev_pm_opp_free_opp_table() - free the opp table
> + * @dev: device for which we do this operation
> + *
> + * Free up the allocated opp table
> + *
> + * Locking: The internal device_opp and opp structures are RCU protected.
> + * Hence this function internally uses RCU updater strategy with mutex locks
> to
> + * keep the integrity of the internal data structures. Callers should ensure
> + * that this function is *NOT* called under RCU protection or in contexts
> where
> + * mutex locking or synchronize_rcu() blocking calls cannot be used.
> + */
> +void dev_pm_opp_free_opp_table(struct device *dev)
> +{
> + struct device_opp *dev_opp = NULL;
> + struct dev_pm_opp *opp;
> +
if (!dev)
return;
> + /* 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?
To maintain non-dt behavior coherency, should'nt we rather add a
opp_remove or an opp_del function?
> +
> + list_del_rcu(&dev_opp->node);
> + mutex_unlock(&dev_opp_list_lock);
> + synchronize_rcu();
> + kfree(dev_opp);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_opp_free_opp_table);
> #endif
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index 0330217..3c29620 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -50,6 +50,8 @@ int dev_pm_opp_enable(struct device *dev, unsigned long
> freq);
> int dev_pm_opp_disable(struct device *dev, unsigned long freq);
>
> struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
> +
> +void dev_pm_opp_free_opp_table(struct device *dev);
> #else
> static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
> {
> @@ -105,6 +107,10 @@ static inline struct srcu_notifier_head
> *dev_pm_opp_get_notifier(
> {
> return ERR_PTR(-EINVAL);
> }
> +
> +void dev_pm_opp_free_opp_table(struct device *dev)
> +{
> +}
> #endif /* CONFIG_PM_OPP */
>
> #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
>
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
On Fri, May 16, 2014 at 10:41 PM, Viresh Kumar wrote:
> On 16 May 2014 22:38, Inderpal Singh wrote:
+ while (!list_empty(&dev_opp->opp_list)) {
+ opp = list_entry_rcu(dev_opp->opp_list.next,
+ struct dev_pm_opp, node);
>>>
>>> list_for_each_entry_rcu ?
>>>
>>
>> list_for_each_entry_rcu can not be used as opp is being deleted in the loop.
>
> So what? The above code can be replaced easily I think.
> This is how it is implemented:
>
> #define list_for_each_entry_rcu(pos, head, member) \
> for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> &pos->member != (head); \
> pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
by the time "pos = list_entry_rcu(pos->member.
next, typeof(*pos), member)" is executed, the pos would have been
freed in the loop.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
On 16 May 2014 22:54, Inderpal Singh wrote: > by the time "pos = list_entry_rcu(pos->member. > next, typeof(*pos), member)" is executed, the pos would have been > freed in the loop. Oops!! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
On 16 May 2014 22:38, Inderpal Singh wrote:
>>> + while (!list_empty(&dev_opp->opp_list)) {
>>> + opp = list_entry_rcu(dev_opp->opp_list.next,
>>> + struct dev_pm_opp, node);
>>
>> list_for_each_entry_rcu ?
>>
>
> list_for_each_entry_rcu can not be used as opp is being deleted in the loop.
So what? The above code can be replaced easily I think.
This is how it is implemented:
#define list_for_each_entry_rcu(pos, head, member) \
for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
&pos->member != (head); \
pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
Hi Viresh,
Thanks for the review.
On Fri, May 16, 2014 at 3:36 PM, Viresh Kumar wrote:
> On 16 May 2014 14:39, Inderpal Singh wrote:
>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>> +void dev_pm_opp_free_opp_table(struct device *dev)
>> +{
>> + struct device_opp *dev_opp = NULL;
>> + struct dev_pm_opp *opp;
>> +
>> + /* 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_for_each_entry_rcu ?
>
list_for_each_entry_rcu can not be used as opp is being deleted in the loop.
Thanks,
Inder
>> + list_del_rcu(&opp->node);
>> + kfree_rcu(opp, head);
>> + }
>> +
>> + list_del_rcu(&dev_opp->node);
>> + mutex_unlock(&dev_opp_list_lock);
>> + synchronize_rcu();
>> + kfree(dev_opp);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_opp_free_opp_table);
>> #endif
>> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
>> index 0330217..3c29620 100644
>> --- a/include/linux/pm_opp.h
>> +++ b/include/linux/pm_opp.h
>> @@ -50,6 +50,8 @@ int dev_pm_opp_enable(struct device *dev, unsigned long
>> freq);
>> int dev_pm_opp_disable(struct device *dev, unsigned long freq);
>>
>> struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
>> +
>> +void dev_pm_opp_free_opp_table(struct device *dev);
>> #else
>> static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
>> {
>> @@ -105,6 +107,10 @@ static inline struct srcu_notifier_head
>> *dev_pm_opp_get_notifier(
>> {
>> return ERR_PTR(-EINVAL);
>> }
>> +
>> +void dev_pm_opp_free_opp_table(struct device *dev)
>> +{
>> +}
>> #endif /* CONFIG_PM_OPP */
>>
>> #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
>
> Otherwise looks fine.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] PM / OPP: Implement free_opp_table function
On 16 May 2014 14:39, Inderpal Singh wrote:
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> +void dev_pm_opp_free_opp_table(struct device *dev)
> +{
> + struct device_opp *dev_opp = NULL;
> + struct dev_pm_opp *opp;
> +
> + /* 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_for_each_entry_rcu ?
> + list_del_rcu(&opp->node);
> + kfree_rcu(opp, head);
> + }
> +
> + list_del_rcu(&dev_opp->node);
> + mutex_unlock(&dev_opp_list_lock);
> + synchronize_rcu();
> + kfree(dev_opp);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_opp_free_opp_table);
> #endif
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index 0330217..3c29620 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -50,6 +50,8 @@ int dev_pm_opp_enable(struct device *dev, unsigned long
> freq);
> int dev_pm_opp_disable(struct device *dev, unsigned long freq);
>
> struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
> +
> +void dev_pm_opp_free_opp_table(struct device *dev);
> #else
> static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
> {
> @@ -105,6 +107,10 @@ static inline struct srcu_notifier_head
> *dev_pm_opp_get_notifier(
> {
> return ERR_PTR(-EINVAL);
> }
> +
> +void dev_pm_opp_free_opp_table(struct device *dev)
> +{
> +}
> #endif /* CONFIG_PM_OPP */
>
> #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
Otherwise looks fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] PM / OPP: Implement free_opp_table function
At the driver unloading time the associated opp table may need
to be deleted. Otherwise it amounts to memory leak. The existing
OPP library does not have provison to do so.
Hence this patch implements the function to free the opp table.
Signed-off-by: Inderpal Singh
---
drivers/base/power/opp.c | 41 +
include/linux/pm_opp.h | 6 ++
2 files changed, 47 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index d9e376a..d45ffd5 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -654,4 +654,45 @@ int of_init_opp_table(struct device *dev)
return 0;
}
EXPORT_SYMBOL_GPL(of_init_opp_table);
+
+/**
+ * dev_pm_opp_free_opp_table() - free the opp table
+ * @dev: device for which we do this operation
+ *
+ * Free up the allocated opp table
+ *
+ * Locking: The internal device_opp and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks to
+ * keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex locking or synchronize_rcu() blocking calls cannot be used.
+ */
+void dev_pm_opp_free_opp_table(struct device *dev)
+{
+ struct device_opp *dev_opp = NULL;
+ struct dev_pm_opp *opp;
+
+ /* 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);
+ }
+
+ list_del_rcu(&dev_opp->node);
+ mutex_unlock(&dev_opp_list_lock);
+ synchronize_rcu();
+ kfree(dev_opp);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_free_opp_table);
#endif
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 0330217..3c29620 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -50,6 +50,8 @@ int dev_pm_opp_enable(struct device *dev, unsigned long freq);
int dev_pm_opp_disable(struct device *dev, unsigned long freq);
struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
+
+void dev_pm_opp_free_opp_table(struct device *dev);
#else
static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
{
@@ -105,6 +107,10 @@ static inline struct srcu_notifier_head
*dev_pm_opp_get_notifier(
{
return ERR_PTR(-EINVAL);
}
+
+void dev_pm_opp_free_opp_table(struct device *dev)
+{
+}
#endif /* CONFIG_PM_OPP */
#if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
