lkml.org 
[lkml]   [2015]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] ASoC: qcom: change PTR_ERR argument


On Sun, 30 Aug 2015, walter harms wrote:

>
>
> Am 30.08.2015 20:05, schrieb Julia Lawall:
> > Apply PTR_ERR to the value that was recently assigned.
> >
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression x,y;
> > @@
> >
> > if (IS_ERR(x) || ...) {
> > ... when any
> > when != IS_ERR(...)
> > (
> > PTR_ERR(x)
> > |
> > * PTR_ERR(y)
> > )
> > ... when any
> > }
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> > sound/soc/qcom/lpass-cpu.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> > index 23f3d59..94beb99 100644
> > --- a/sound/soc/qcom/lpass-cpu.c
> > +++ b/sound/soc/qcom/lpass-cpu.c
> > @@ -438,7 +438,8 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
> > if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
> > dev_err(&pdev->dev,
> > "%s() error getting mi2s-bit-clk: %ld\n",
> > - __func__, PTR_ERR(drvdata->mi2s_bit_clk[i]));
> > + __func__,
> > + PTR_ERR(drvdata->mi2s_bit_clk[dai_id]));
> > return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
> > }
> > }
> >
>
> just a note:
> using a shorter name instead of drvdata->mi2s_bit_clk[dai_id] whould help to make the code
> more readable (yes, the other code is alike). something like:
>
> struct clk *tmp = devm_clk_get(&pdev->dev,clk_name);

Where do you suggest to put this?

Maybe it would be reasonable to declare a variable struct clk *clk; at the
top of the function, and then use that as a temporary variable for all
three calls.

However, now I see that the first call, unlike the other two doesn't cause
a return from the function.

if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
dev_warn(&pdev->dev,
"%s() error getting mi2s-osr-clk: %ld\n",
__func__,
PTR_ERR(drvdata->mi2s_osr_clk[dai_id]));
}

Is that intentional?

thanks,
julia

> if (IS_ERR(tmp)) {
> dev_err(&pdev->dev,"%s() error getting mi2s-bit-clk: %ld\n",__func__, PTR_ERR(tmp));
> return PTR_ERR(tmp);
> }
> drvdata->mi2s_bit_clk[dai_id]=tmp;
>
>
> just one minor:
> the dev_warn() just before says: " error getting mi2s-osr-clk" may be it should be "warnig ..."
> That will make it more easy to rep for real error in a log.
>
> re,
> wh
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


\
 
 \ /
  Last update: 2015-08-30 22:21    [W:0.095 / U:0.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site