diff mbox series

[DRM] drm: msm: adreno: Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) +PTR_ERR

Message ID 20181018204815.GA23390@armorer (mailing list archive)
State New, archived
Headers show
Series [DRM] drm: msm: adreno: Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) +PTR_ERR | expand

Commit Message

Mamta Shukla Oct. 18, 2018, 8:48 p.m. UTC
Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR because its
better to have inlined function rather than code-opened implementation.

Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
---
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Julia Lawall Oct. 19, 2018, 6:18 a.m. UTC | #1
On Fri, 19 Oct 2018, Mamta Shukla wrote:

> Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR because its
> better to have inlined function rather than code-opened implementation.


Just for your information, people have mixed feelings about this.  In your
case, it's probably fine, because this is the only if in the function.
But when there is a series of if (...) return; followed by a return 0; at
the end, people ofen prefer to keep it as it is, so the last case doesn't
look different than the others.

julia

>
> Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 6a68493..b751b76 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1222,10 +1222,7 @@ static int a5xx_crashdumper_init(struct msm_gpu *gpu,
>  		SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
>  		&dumper->bo, &dumper->iova);
>
> -	if (IS_ERR(dumper->ptr))
> -		return PTR_ERR(dumper->ptr);
> -
> -	return 0;
> +	return PTR_ERR_OR_ZERO(dumper->ptr);
>  }
>
>  static void a5xx_crashdumper_free(struct msm_gpu *gpu,
> --
> 1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181018204815.GA23390%40armorer.
> For more options, visit https://groups.google.com/d/optout.
>
Daniel Vetter Oct. 19, 2018, 7:19 a.m. UTC | #2
On Fri, Oct 19, 2018 at 08:18:52AM +0200, Julia Lawall wrote:
> On Fri, 19 Oct 2018, Mamta Shukla wrote:
> > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR because its
> > better to have inlined function rather than code-opened implementation.
> 
> 
> Just for your information, people have mixed feelings about this.  In your
> case, it's probably fine, because this is the only if in the function.
> But when there is a series of if (...) return; followed by a return 0; at
> the end, people ofen prefer to keep it as it is, so the last case doesn't
> look different than the others.

Yeah I asked the maintainer for this driver for an ack, the other patches
look good, will merged them as soon as I have the answer for this one
here.

Mamta, since you'll have a few things merged already with these I think
it's a good time to start looking into bigger tasks and figuring out what
you might want to do in your internship:

https://dri.freedesktop.org/docs/drm/gpu/todo.html

Since this todo list is very unstructured it's best to discuss a bit over
irc what you would want to, and what makes sense as an internship.

Thanks, Daniel
> 
> julia
> 
> >
> > Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> > ---
> >  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > index 6a68493..b751b76 100644
> > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > @@ -1222,10 +1222,7 @@ static int a5xx_crashdumper_init(struct msm_gpu *gpu,
> >  		SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
> >  		&dumper->bo, &dumper->iova);
> >
> > -	if (IS_ERR(dumper->ptr))
> > -		return PTR_ERR(dumper->ptr);
> > -
> > -	return 0;
> > +	return PTR_ERR_OR_ZERO(dumper->ptr);
> >  }
> >
> >  static void a5xx_crashdumper_free(struct msm_gpu *gpu,
> > --
> > 1.9.1
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181018204815.GA23390%40armorer.
> > For more options, visit https://groups.google.com/d/optout.
> >
Jordan Crouse Oct. 19, 2018, 4:57 p.m. UTC | #3
On Fri, Oct 19, 2018 at 09:19:32AM +0200, Daniel Vetter wrote:
> On Fri, Oct 19, 2018 at 08:18:52AM +0200, Julia Lawall wrote:
> > On Fri, 19 Oct 2018, Mamta Shukla wrote:
> > > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR because its
> > > better to have inlined function rather than code-opened implementation.
> > 
> > 
> > Just for your information, people have mixed feelings about this.  In your
> > case, it's probably fine, because this is the only if in the function.
> > But when there is a series of if (...) return; followed by a return 0; at
> > the end, people ofen prefer to keep it as it is, so the last case doesn't
> > look different than the others.
> 
> Yeah I asked the maintainer for this driver for an ack, the other patches
> look good, will merged them as soon as I have the answer for this one
> here.
> 
> Mamta, since you'll have a few things merged already with these I think
> it's a good time to start looking into bigger tasks and figuring out what
> you might want to do in your internship:
> 
> https://dri.freedesktop.org/docs/drm/gpu/todo.html
> 
> Since this todo list is very unstructured it's best to discuss a bit over
> irc what you would want to, and what makes sense as an internship.

> Thanks, Daniel
> > 
> > julia
> > 

I'm not the maintainer but this seems reasonable to me. Yay negative lines.

Jordan

> > >
> > > Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> > > ---
> > >  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > > index 6a68493..b751b76 100644
> > > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > > @@ -1222,10 +1222,7 @@ static int a5xx_crashdumper_init(struct msm_gpu *gpu,
> > >  		SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
> > >  		&dumper->bo, &dumper->iova);
> > >
> > > -	if (IS_ERR(dumper->ptr))
> > > -		return PTR_ERR(dumper->ptr);
> > > -
> > > -	return 0;
> > > +	return PTR_ERR_OR_ZERO(dumper->ptr);
> > >  }
> > >
> > >  static void a5xx_crashdumper_free(struct msm_gpu *gpu,
> > > --
> > > 1.9.1
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181018204815.GA23390%40armorer.
> > > For more options, visit https://groups.google.com/d/optout.
> > >
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Sean Paul Oct. 23, 2018, 12:54 p.m. UTC | #4
On Thu, Oct 18, 2018 at 7:11 PM Mamta Shukla <mamtashukla555@gmail.com> wrote:
>
> Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR because its
> better to have inlined function rather than code-opened implementation.
>
> Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>

Reviewed-by: Sean Paul <sean@poorly.run>

> ---
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index 6a68493..b751b76 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1222,10 +1222,7 @@ static int a5xx_crashdumper_init(struct msm_gpu *gpu,
>                 SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
>                 &dumper->bo, &dumper->iova);
>
> -       if (IS_ERR(dumper->ptr))
> -               return PTR_ERR(dumper->ptr);
> -
> -       return 0;
> +       return PTR_ERR_OR_ZERO(dumper->ptr);
>  }
>
>  static void a5xx_crashdumper_free(struct msm_gpu *gpu,
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter Oct. 23, 2018, 1:59 p.m. UTC | #5
On Tue, Oct 23, 2018 at 08:54:36AM -0400, Sean Paul wrote:
> On Thu, Oct 18, 2018 at 7:11 PM Mamta Shukla <mamtashukla555@gmail.com> wrote:
> >
> > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR because its
> > better to have inlined function rather than code-opened implementation.
> >
> > Signed-off-by: Mamta Shukla <mamtashukla555@gmail.com>
> 
> Reviewed-by: Sean Paul <sean@poorly.run>

Applied to drm-misc-next so Julia can easily spot them all. Thanks for
patch&review.
-Daniel
> 
> > ---
> >  drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > index 6a68493..b751b76 100644
> > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> > @@ -1222,10 +1222,7 @@ static int a5xx_crashdumper_init(struct msm_gpu *gpu,
> >                 SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
> >                 &dumper->bo, &dumper->iova);
> >
> > -       if (IS_ERR(dumper->ptr))
> > -               return PTR_ERR(dumper->ptr);
> > -
> > -       return 0;
> > +       return PTR_ERR_OR_ZERO(dumper->ptr);
> >  }
> >
> >  static void a5xx_crashdumper_free(struct msm_gpu *gpu,
> > --
> > 1.9.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 6a68493..b751b76 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1222,10 +1222,7 @@  static int a5xx_crashdumper_init(struct msm_gpu *gpu,
 		SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
 		&dumper->bo, &dumper->iova);
 
-	if (IS_ERR(dumper->ptr))
-		return PTR_ERR(dumper->ptr);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(dumper->ptr);
 }
 
 static void a5xx_crashdumper_free(struct msm_gpu *gpu,