Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()

[miaoqinglang]

在 2020/9/21 21:12, Mikko Perttunen 写道:

On 9/21/20 4:10 PM, Qinglang Miao wrote:

Simplify the return expression.

Signed-off-by: Qinglang Miao 
---
  drivers/gpu/host1x/cdma.c | 8 +---
  1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
index e8d3fda91..08a0f9e10 100644
--- a/drivers/gpu/host1x/cdma.c
+++ b/drivers/gpu/host1x/cdma.c
@@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct 
host1x_cdma *cdma,

   */
  int host1x_cdma_init(struct host1x_cdma *cdma)
  {
-    int err;
-
  mutex_init(>lock);
  init_completion(>complete);
@@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma)
  cdma->running = false;
  cdma->torndown = false;
-    err = host1x_pushbuffer_init(>push_buffer);
-    if (err)
-    return err;
-
-    return 0;
+    return host1x_pushbuffer_init(>push_buffer);


IMHO, this makes it less readable since now it kind of looks like 
host1x_pushbuffer_init is returning some meaningful value, instead of 
the code just handling error values in a sequence.


Mikko


Hi Mikko,

It sounds resonable, thanks for your reply.


  }
  /*


.

Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()

[Thierry Reding]

On Mon, Sep 21, 2020 at 04:12:20PM +0300, Mikko Perttunen wrote:
> On 9/21/20 4:10 PM, Qinglang Miao wrote:
> > Simplify the return expression.
> > 
> > Signed-off-by: Qinglang Miao 
> > ---
> >   drivers/gpu/host1x/cdma.c | 8 +---
> >   1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> > index e8d3fda91..08a0f9e10 100644
> > --- a/drivers/gpu/host1x/cdma.c
> > +++ b/drivers/gpu/host1x/cdma.c
> > @@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma 
> > *cdma,
> >*/
> >   int host1x_cdma_init(struct host1x_cdma *cdma)
> >   {
> > -   int err;
> > -
> > mutex_init(>lock);
> > init_completion(>complete);
> > @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma)
> > cdma->running = false;
> > cdma->torndown = false;
> > -   err = host1x_pushbuffer_init(>push_buffer);
> > -   if (err)
> > -   return err;
> > -
> > -   return 0;
> > +   return host1x_pushbuffer_init(>push_buffer);
> 
> IMHO, this makes it less readable since now it kind of looks like
> host1x_pushbuffer_init is returning some meaningful value, instead of the
> code just handling error values in a sequence.

Agreed. Collapsing the error handling like this also makes adding code
to the bottom of this function more complicated than necessary.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()

[Mikko Perttunen]

On 9/21/20 4:10 PM, Qinglang Miao wrote:

Simplify the return expression.

Signed-off-by: Qinglang Miao 
---
  drivers/gpu/host1x/cdma.c | 8 +---
  1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
index e8d3fda91..08a0f9e10 100644
--- a/drivers/gpu/host1x/cdma.c
+++ b/drivers/gpu/host1x/cdma.c
@@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma *cdma,
   */
  int host1x_cdma_init(struct host1x_cdma *cdma)
  {
-   int err;
-
mutex_init(>lock);
init_completion(>complete);
  
@@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma)

cdma->running = false;
cdma->torndown = false;
  
-	err = host1x_pushbuffer_init(>push_buffer);

-   if (err)
-   return err;
-
-   return 0;
+   return host1x_pushbuffer_init(>push_buffer);


IMHO, this makes it less readable since now it kind of looks like 
host1x_pushbuffer_init is returning some meaningful value, instead of 
the code just handling error values in a sequence.


Mikko


  }
  
  /*

[PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()

[Qinglang Miao]

Simplify the return expression.

Signed-off-by: Qinglang Miao 
---
 drivers/gpu/host1x/cdma.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
index e8d3fda91..08a0f9e10 100644
--- a/drivers/gpu/host1x/cdma.c
+++ b/drivers/gpu/host1x/cdma.c
@@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma *cdma,
  */
 int host1x_cdma_init(struct host1x_cdma *cdma)
 {
-   int err;
-
mutex_init(>lock);
init_completion(>complete);
 
@@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma)
cdma->running = false;
cdma->torndown = false;
 
-   err = host1x_pushbuffer_init(>push_buffer);
-   if (err)
-   return err;
-
-   return 0;
+   return host1x_pushbuffer_init(>push_buffer);
 }
 
 /*
-- 
2.23.0