RE: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

2017-04-28 Thread weiyongjun (A) 

> On Tue, Apr 25, 2017 at 07:57:57AM +, weiyongjun (A) wrote:
> > > >
> > > > Since we will remove items off the list using list_del() we need
> > > > to use a safe version of the list_for_each() macro aptly named
> > > > list_for_each_safe().
> > >
> > > ah yes, god catch
> > >
> > > > This is detected by Coccinelle semantic patch.
> > >
> > > It is a good practice to include the script you used, do you mind adding
> > > that in the log?
> > >
> >
> > Sure.
> 
> Sorry I meant this should be in changelog of the patch :)


Oh, a similar patch has been applied. Please ignore this one.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=550b349af0f0e33fedb252aca8dc144299aca308

> 
> >
> > @@
> > iterator name list_for_each;
> > expression E;
> > expression pos, head;
> > identifier l;
> > @@
> > *  list_for_each(pos, head)
> > {
> > ...
> > *  \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > expression E;
> > expression pos, head, ep;
> > identifier list;
> > identifier l;
> > @@
> > *  list_for_each(pos, head)
> > {
> > ...
> > * ep = list_entry(pos, ..., list);
> > ...
> > *  \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_prev;
> > expression E;
> > expression pos, head;
> > identifier l;
> > @@
> > *  list_for_each_prev(pos, head)
> > {
> > ...
> > *  \(list_del\|list_del_init\)(pos);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > *  list_for_each_entry(pos, head, member)
> > {
> > ...
> > *  \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry_from;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > *  list_for_each_entry_from(pos, head, member)
> > {
> > ...
> > *  \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry_reverse;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > *  list_for_each_entry_reverse(pos, head, member)
> > {
> > ...
> > *  \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> 
> --
> ~Vinod

RE: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

2017-04-25 Thread weiyongjun (A) 


> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Wednesday, April 26, 2017 1:38 PM
> To: weiyongjun (A) 
> Cc: Wei Yongjun ; Liam Girdwood
> ; Mark Brown ; Jaroslav
> Kysela ; Takashi Iwai ; Pardha Saradhi K
> ; G Kranthi
> ; Hardik T Shah
> ; Jeeja KP ; alsa-
> de...@alsa-project.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use
> list_for_each_safe() when delete items
> 
> On Tue, Apr 25, 2017 at 07:57:57AM +, weiyongjun (A) wrote:
> > > >
> > > > Since we will remove items off the list using list_del() we need
> > > > to use a safe version of the list_for_each() macro aptly named
> > > > list_for_each_safe().
> > >
> > > ah yes, god catch
> > >
> > > > This is detected by Coccinelle semantic patch.
> > >
> > > It is a good practice to include the script you used, do you mind adding
> > > that in the log?
> > >
> >
> > Sure.
> 
> Sorry I meant this should be in changelog of the patch :)

OK, I will add this and send the v2 version patch later.

Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

2017-04-25 Thread Vinod Koul 
On Tue, Apr 25, 2017 at 07:57:57AM +, weiyongjun (A) wrote:
> > >
> > > Since we will remove items off the list using list_del() we need
> > > to use a safe version of the list_for_each() macro aptly named
> > > list_for_each_safe().
> > 
> > ah yes, god catch
> > 
> > > This is detected by Coccinelle semantic patch.
> > 
> > It is a good practice to include the script you used, do you mind adding
> > that in the log?
> > 
> 
> Sure. 

Sorry I meant this should be in changelog of the patch :)

> 
> @@
> iterator name list_for_each;
> expression E;
> expression pos, head;
> identifier l;
> @@
> *  list_for_each(pos, head)
> {
> ...
> *  \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> expression E;
> expression pos, head, ep;
> identifier list;
> identifier l;
> @@
> *  list_for_each(pos, head)
> {
> ...
> * ep = list_entry(pos, ..., list);
> ...
> *  \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> iterator name list_for_each_prev;
> expression E;
> expression pos, head;
> identifier l;
> @@
> *  list_for_each_prev(pos, head)
> {
> ...
> *  \(list_del\|list_del_init\)(pos);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> iterator name list_for_each_entry;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
> 
> *  list_for_each_entry(pos, head, member)
> {
> ...
> *  \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> iterator name list_for_each_entry_from;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
> 
> *  list_for_each_entry_from(pos, head, member)
> {
> ...
> *  \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> iterator name list_for_each_entry_reverse;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
> 
> *  list_for_each_entry_reverse(pos, head, member)
> {
> ...
> *  \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }

-- 
~Vinod

RE: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

2017-04-25 Thread weiyongjun (A) 


> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Tuesday, April 25, 2017 12:01 PM
> To: Wei Yongjun 
> Cc: Liam Girdwood ; Mark Brown
> ; Jaroslav Kysela ; Takashi Iwai
> ; Pardha Saradhi K
> ; G Kranthi
> ; Hardik T Shah
> ; Jeeja KP ; weiyongjun (A)
> ; alsa-de...@alsa-project.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use
> list_for_each_safe() when delete items
> 
> On Tue, Apr 25, 2017 at 03:28:17AM +, Wei Yongjun wrote:
> > From: Wei Yongjun 
> >
> > Since we will remove items off the list using list_del() we need
> > to use a safe version of the list_for_each() macro aptly named
> > list_for_each_safe().
> 
> ah yes, god catch
> 
> > This is detected by Coccinelle semantic patch.
> 
> It is a good practice to include the script you used, do you mind adding
> that in the log?
> 

Sure. 

@@
iterator name list_for_each;
expression E;
expression pos, head;
identifier l;
@@
*  list_for_each(pos, head)
{
...
*  \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
expression E;
expression pos, head, ep;
identifier list;
identifier l;
@@
*  list_for_each(pos, head)
{
...
* ep = list_entry(pos, ..., list);
...
*  \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
iterator name list_for_each_prev;
expression E;
expression pos, head;
identifier l;
@@
*  list_for_each_prev(pos, head)
{
...
*  \(list_del\|list_del_init\)(pos);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
iterator name list_for_each_entry;
expression E;
expression pos, head;
identifier l, member;
@@

*  list_for_each_entry(pos, head, member)
{
...
*  \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
iterator name list_for_each_entry_from;
expression E;
expression pos, head;
identifier l, member;
@@

*  list_for_each_entry_from(pos, head, member)
{
...
*  \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
iterator name list_for_each_entry_reverse;
expression E;
expression pos, head;
identifier l, member;
@@

*  list_for_each_entry_reverse(pos, head, member)
{
...
*  \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}

Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

2017-04-24 Thread Vinod Koul 
On Tue, Apr 25, 2017 at 03:28:17AM +, Wei Yongjun wrote:
> From: Wei Yongjun 
> 
> Since we will remove items off the list using list_del() we need
> to use a safe version of the list_for_each() macro aptly named
> list_for_each_safe().

ah yes, god catch

> This is detected by Coccinelle semantic patch.

It is a good practice to include the script you used, do you mind adding
that in the log?

> 
> Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
> DSP module bind")
> Signed-off-by: Wei Yongjun 
> ---
>  sound/soc/intel/skylake/skl-pcm.c | 4 ++--
>  sound/soc/intel/skylake/skl-topology.c | 4 ++--
>  2 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c 
> b/sound/soc/intel/skylake/skl-pcm.c
> index 600faad..d43d197 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
>  {
>   struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
>   struct skl *skl = ebus_to_skl(ebus);
> - struct skl_module_deferred_bind *modules;
> + struct skl_module_deferred_bind *modules, *tmp;
>  
>   if (!list_empty(&skl->bind_list)) {
> - list_for_each_entry(modules, &skl->bind_list, node) {
> + list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
>   list_del(&modules->node);
>   kfree(modules);
>   }
> diff --git a/sound/soc/intel/skylake/skl-topology.c 
> b/sound/soc/intel/skylake/skl-topology.c
> index 74b3acf..aea7917 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
> snd_soc_dapm_widget *w,
>   struct skl_module_cfg *src_module = NULL, *dst_module;
>   struct skl_sst *ctx = skl->skl_sst;
>   struct skl_pipe *s_pipe = mconfig->pipe;
> - struct skl_module_deferred_bind *modules;
> + struct skl_module_deferred_bind *modules, *tmp;
>  
>   if (s_pipe->state == SKL_PIPE_INVALID)
>   return -EINVAL;
> @@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
> snd_soc_dapm_widget *w,
>  
>   src_module = w_module->w->priv;
>  
> - list_for_each_entry(modules, &skl->bind_list, node) {
> + list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
>   /*
>* When the destination module is deleted, Unbind the
>* modules from deferred bind list.
> 

-- 
~Vinod

[PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

2017-04-24 Thread Wei Yongjun 
From: Wei Yongjun 

Since we will remove items off the list using list_del() we need
to use a safe version of the list_for_each() macro aptly named
list_for_each_safe().

This is detected by Coccinelle semantic patch.

Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
DSP module bind")
Signed-off-by: Wei Yongjun 
---
 sound/soc/intel/skylake/skl-pcm.c | 4 ++--
 sound/soc/intel/skylake/skl-topology.c | 4 ++--
 2 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c 
b/sound/soc/intel/skylake/skl-pcm.c
index 600faad..d43d197 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
 {
struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
struct skl *skl = ebus_to_skl(ebus);
-   struct skl_module_deferred_bind *modules;
+   struct skl_module_deferred_bind *modules, *tmp;
 
if (!list_empty(&skl->bind_list)) {
-   list_for_each_entry(modules, &skl->bind_list, node) {
+   list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
list_del(&modules->node);
kfree(modules);
}
diff --git a/sound/soc/intel/skylake/skl-topology.c 
b/sound/soc/intel/skylake/skl-topology.c
index 74b3acf..aea7917 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
snd_soc_dapm_widget *w,
struct skl_module_cfg *src_module = NULL, *dst_module;
struct skl_sst *ctx = skl->skl_sst;
struct skl_pipe *s_pipe = mconfig->pipe;
-   struct skl_module_deferred_bind *modules;
+   struct skl_module_deferred_bind *modules, *tmp;
 
if (s_pipe->state == SKL_PIPE_INVALID)
return -EINVAL;
@@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct 
snd_soc_dapm_widget *w,
 
src_module = w_module->w->priv;
 
-   list_for_each_entry(modules, &skl->bind_list, node) {
+   list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
/*
 * When the destination module is deleted, Unbind the
 * modules from deferred bind list.