Re: [alsa-devel] [PATCH] wm8940: remove unecessary if statement
On 06/06/11 01:47, Greg Dietsche wrote:
the code always returns ret regardless, so if(ret) check is unecessary.
Good point, though please spell check your commit messages. unecessary -> unnecessary
Also if you want to do this sort of cleanup, please also fix the equivalent in wm8940_resume and wm8940_add_widgets. Ack is for what is here, plus those if you do them.
Just as an aside, there is no earthly point in cc'ing lkml for a simple cleanup like this. Just adds to already huge amount of noise!
Signed-off-by: Greg Dietsche Gregory.Dietsche@cuw.edu
Acked-by: Jonathan Cameron jic23@cam.ac.uk
sound/soc/codecs/wm8940.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/sound/soc/codecs/wm8940.c b/sound/soc/codecs/wm8940.c index 25580e3..1ada6d3 100644 --- a/sound/soc/codecs/wm8940.c +++ b/sound/soc/codecs/wm8940.c @@ -730,9 +730,6 @@ static int wm8940_probe(struct snd_soc_codec *codec) if (ret) return ret; ret = wm8940_add_widgets(codec);
- if (ret)
return ret;- return ret;
}
On 06/06/11 10:31, Jonathan Cameron wrote:
On 06/06/11 01:47, Greg Dietsche wrote:
the code always returns ret regardless, so if(ret) check is unecessary.
Good point, though please spell check your commit messages. unecessary -> unnecessary
Also if you want to do this sort of cleanup, please also fix the equivalent in wm8940_resume and wm8940_add_widgets. Ack is for what is here, plus those if you do them.
Just as an aside, there is no earthly point in cc'ing lkml for a simple cleanup like this. Just adds to already huge amount of noise!
Signed-off-by: Greg Dietsche Gregory.Dietsche@cuw.edu
Acked-by: Jonathan Cameron jic23@cam.ac.uk
Acked-by: Liam Girdwood lrg@ti.com
removing unnecessary if(ret) checks
This updated patch corrects a minor spelling problem in the commit message and resolves two other (similar) issues found in wm8940.c by Jonathan Cameron.
Signed-off-by: Greg Dietsche Gregory.Dietsche@cuw.edu --- sound/soc/codecs/wm8940.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/wm8940.c b/sound/soc/codecs/wm8940.c index 25580e3..056daa0 100644 --- a/sound/soc/codecs/wm8940.c +++ b/sound/soc/codecs/wm8940.c @@ -297,8 +297,6 @@ static int wm8940_add_widgets(struct snd_soc_codec *codec) if (ret) goto error_ret; ret = snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map)); - if (ret) - goto error_ret;
error_ret: return ret; @@ -683,8 +681,6 @@ static int wm8940_resume(struct snd_soc_codec *codec) } } ret = wm8940_set_bias_level(codec, SND_SOC_BIAS_STANDBY); - if (ret) - goto error_ret;
error_ret: return ret; @@ -730,9 +726,6 @@ static int wm8940_probe(struct snd_soc_codec *codec) if (ret) return ret; ret = wm8940_add_widgets(codec); - if (ret) - return ret; - return ret; }
On 06/06/11 21:53, Greg Dietsche wrote:
removing unnecessary if(ret) checks
This updated patch corrects a minor spelling problem in the commit message and resolves two other (similar) issues found in wm8940.c by Jonathan Cameron.
Thanks
Signed-off-by: Greg Dietsche Gregory.Dietsche@cuw.edu
Acked-by: Jonathan Cameron jic23@cam.ac.uk
sound/soc/codecs/wm8940.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/sound/soc/codecs/wm8940.c b/sound/soc/codecs/wm8940.c index 25580e3..056daa0 100644 --- a/sound/soc/codecs/wm8940.c +++ b/sound/soc/codecs/wm8940.c @@ -297,8 +297,6 @@ static int wm8940_add_widgets(struct snd_soc_codec *codec) if (ret) goto error_ret; ret = snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map));
- if (ret)
goto error_ret;error_ret: return ret; @@ -683,8 +681,6 @@ static int wm8940_resume(struct snd_soc_codec *codec) } } ret = wm8940_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
- if (ret)
goto error_ret;error_ret: return ret; @@ -730,9 +726,6 @@ static int wm8940_probe(struct snd_soc_codec *codec) if (ret) return ret; ret = wm8940_add_widgets(codec);
- if (ret)
return ret;- return ret;
}
On 06/06/11 21:53, Greg Dietsche wrote:
removing unnecessary if(ret) checks
This updated patch corrects a minor spelling problem in the commit message and resolves two other (similar) issues found in wm8940.c by Jonathan Cameron.
Signed-off-by: Greg Dietsche Gregory.Dietsche@cuw.edu
Acked-by: Liam Girdwood lrg@ti.com
On Mon, Jun 06, 2011 at 03:53:01PM -0500, Greg Dietsche wrote:
removing unnecessary if(ret) checks
This updated patch corrects a minor spelling problem in the commit message and resolves two other (similar) issues found in wm8940.c by Jonathan Cameron.
Applied, thanks.
Hi Jonathan,
On 06/06/2011 04:31 AM, Jonathan Cameron wrote:
On 06/06/11 01:47, Greg Dietsche wrote:
the code always returns ret regardless, so if(ret) check is unecessary.
Good point, though please spell check your commit messages. unecessary -> unnecessary
oops! usually I'm the guy critiquing spelling :)
Also if you want to do this sort of cleanup, please also fix the equivalent in wm8940_resume and wm8940_add_widgets. Ack is for what is here, plus those if you do them.
I will take a look at these, but it might be a few days. I used coccinelle to create this patch and my semantic patch wasn't 'smart' enough to find them.
Just as an aside, there is no earthly point in cc'ing lkml for a simple cleanup like this. Just adds to already huge amount of noise!
Thanks for all of your feedback. In your opinion, what is the best way for someone such as myself to send patches like these? I read in Documentation/SubmittingPatches "Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org"
Also, for this embarrassing spelling problem... do I submit a new patch? :)
Thanks, Greg
On 06/06/11 13:45, Greg Dietsche wrote:
Hi Jonathan,
On 06/06/2011 04:31 AM, Jonathan Cameron wrote:
On 06/06/11 01:47, Greg Dietsche wrote:
the code always returns ret regardless, so if(ret) check is unecessary.
Good point, though please spell check your commit messages. unecessary -> unnecessary
oops! usually I'm the guy critiquing spelling :)
The advantage of reviewing patches in an email client that sticks wiggly red lines under words it doesn't recognise (I'd never have noticed otherwise!)
Also if you want to do this sort of cleanup, please also fix the equivalent in wm8940_resume and wm8940_add_widgets. Ack is for what is here, plus those if you do them.
I will take a look at these, but it might be a few days. I used coccinelle to create this patch and my semantic patch wasn't 'smart' enough to find them.
Just as an aside, there is no earthly point in cc'ing lkml for a simple cleanup like this. Just adds to already huge amount of noise!
Thanks for all of your feedback. In your opinion, what is the best way for someone such as myself to send patches like these? I read in Documentation/SubmittingPatches "Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org"
Fair enough. The posting to lkml makes more sense now I know it came out of coccinelle (I guess with a load of others? - if so convention would be to put them all in a series cc'ing the relevant lists / maintainers for individual patches in the series - that way everyone knows what is going on).
If it is an individual patch like this, then use apply common sense. It makes no functional changes + is well within a subsystem with it's own active mailing list. It needs to be sent somewhere publicly, but in this case I'd say alsa-devel is the right destination. The only people who are even going to read this are the subsystem maintainer, the driver author or the chronically bored.
Also I think convention is to have the script somewhere (cover letter to that series perhaps?). See the other series people have done with coccinelle and how they handled this.
Also, for this embarrassing spelling problem... do I submit a new patch? :)
Probably easiest option, though maintainer might just fix it up for you (best not to assume they will though).
Git history is full of typos, so I wouldn't worry too much (a good few of them are mine for starters).
Thanks, Greg
Hi Jonathan,
On 06/06/2011 08:05 AM, Jonathan Cameron wrote:
On 06/06/11 13:45, Greg Dietsche wrote:
Also if you want to do this sort of cleanup, please also fix the equivalent in wm8940_resume and wm8940_add_widgets. Ack is for what is here, plus those if you do them.
I updated the patch to include these too.
Just as an aside, there is no earthly point in cc'ing lkml for a simple cleanup like this. Just adds to already huge amount of noise!
...and remove LKML from the CC list... :)
Fair enough. The posting to lkml makes more sense now I know it came out of coccinelle (I guess with a load of others? - if so convention would be
a handful... not too many, but it sounds like if my semantic patch were to be improved, there might be a few more.
to put them all in a series cc'ing the relevant lists / maintainers for individual patches in the series - that way everyone knows what is going on).
If it is an individual patch like this, then use apply common sense. It makes no functional changes + is well within a subsystem with it's own active mailing list. It needs to be sent somewhere publicly, but in this case I'd say alsa-devel is the right destination. The only people who are even going to read this are the subsystem maintainer, the driver author or the chronically bored.
Also I think convention is to have the script somewhere (cover letter to that series perhaps?). See the other series people have done with coccinelle and how they handled this.
Thanks so much for the great explanation being patient with a kernel newbie :)
Greg
participants (4)
-
Greg Dietsche -
Jonathan Cameron -
Liam Girdwood -
Mark Brown