| Message ID | 20200921131032.91972-1-miaoqinglang@huawei.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [-next] gpu: host1x: simplify the return expression of host1x_cdma_init() | expand |
On 9/21/20 4:10 PM, Qinglang Miao wrote: > Simplify the return expression. > > Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> > --- > 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(&cdma->lock); > init_completion(&cdma->complete); > > @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma) > cdma->running = false; > cdma->torndown = false; > > - err = host1x_pushbuffer_init(&cdma->push_buffer); > - if (err) > - return err; > - > - return 0; > + return host1x_pushbuffer_init(&cdma->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 > } > > /* >
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 <miaoqinglang@huawei.com> > > --- > > 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(&cdma->lock); > > init_completion(&cdma->complete); > > @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma) > > cdma->running = false; > > cdma->torndown = false; > > - err = host1x_pushbuffer_init(&cdma->push_buffer); > > - if (err) > > - return err; > > - > > - return 0; > > + return host1x_pushbuffer_init(&cdma->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
在 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 <miaoqinglang@huawei.com> >> --- >> 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(&cdma->lock); >> init_completion(&cdma->complete); >> @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma) >> cdma->running = false; >> cdma->torndown = false; >> - err = host1x_pushbuffer_init(&cdma->push_buffer); >> - if (err) >> - return err; >> - >> - return 0; >> + return host1x_pushbuffer_init(&cdma->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. >> } >> /* >> > .
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(&cdma->lock); init_completion(&cdma->complete); @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma) cdma->running = false; cdma->torndown = false; - err = host1x_pushbuffer_init(&cdma->push_buffer); - if (err) - return err; - - return 0; + return host1x_pushbuffer_init(&cdma->push_buffer); } /*
Simplify the return expression. Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> --- drivers/gpu/host1x/cdma.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)