drm/arm: cleanup coding style in arm a bit
diff mbox series

Message ID 20200422021046.4375-1-bernard@vivo.com
State New, archived
Headers show
Series
  • drm/arm: cleanup coding style in arm a bit
Related show

Commit Message

Bernard Zhao April 22, 2020, 2:10 a.m. UTC
For the code logic, an alarm is thrown after failure, but the
code continues to run and returns successfully, so to the caller
the if check and return branch will never run.
The change is to make the code a bit more readable.

Signed-off-by: Bernard Zhao <bernard@vivo.com>
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Liviu Dudau April 23, 2020, 12:50 p.m. UTC | #1
Hi Bernard,

On Tue, Apr 21, 2020 at 07:10:46PM -0700, Bernard Zhao wrote:
> For the code logic, an alarm is thrown after failure, but the
> code continues to run and returns successfully, so to the caller
> the if check and return branch will never run.
> The change is to make the code a bit more readable.
> 
> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> ---
>  drivers/gpu/drm/arm/hdlcd_crtc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index af67fefed38d..32bda13250f5 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -160,9 +160,7 @@ static void hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
>  	hdlcd_write(hdlcd, HDLCD_REG_H_SYNC, vm.hsync_len - 1);
>  	hdlcd_write(hdlcd, HDLCD_REG_POLARITIES, polarities);
>  
> -	err = hdlcd_set_pxl_fmt(crtc);
> -	if (err)
> -		return;
> +	hdlcd_set_pxl_fmt(crtc);

I think you found a real bug. hdlcd_set_pxl_fmt() is not supposed to return zero if
the format is not supported and here we would stop enabling the pixel clock.

Do you care to send a patch for fixing the bug, rather than this one?

Best regards,
Liviu

>  
>  	clk_set_rate(hdlcd->clk, m->crtc_clock * 1000);
>  }
> -- 
> 2.26.2
>
Bernard Zhao April 24, 2020, 12:20 a.m. UTC | #2
From: Liviu Dudau <liviu.dudau@arm.com>
Date: 2020-04-23 20:50:07
To:  Bernard Zhao <bernard@vivo.com>
Cc:  Brian Starkey <brian.starkey@arm.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org,opensource.kernel@vivo.com
Subject: Re: [PATCH] drm/arm: cleanup coding style in arm a bit>Hi Bernard,
>
>On Tue, Apr 21, 2020 at 07:10:46PM -0700, Bernard Zhao wrote:
>> For the code logic, an alarm is thrown after failure, but the
>> code continues to run and returns successfully, so to the caller
>> the if check and return branch will never run.
>> The change is to make the code a bit more readable.
>> 
>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> ---
>>  drivers/gpu/drm/arm/hdlcd_crtc.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> index af67fefed38d..32bda13250f5 100644
>> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
>> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
>> @@ -160,9 +160,7 @@ static void hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>  	hdlcd_write(hdlcd, HDLCD_REG_H_SYNC, vm.hsync_len - 1);
>>  	hdlcd_write(hdlcd, HDLCD_REG_POLARITIES, polarities);
>>  
>> -	err = hdlcd_set_pxl_fmt(crtc);
>> -	if (err)
>> -		return;
>> +	hdlcd_set_pxl_fmt(crtc);
>
>I think you found a real bug. hdlcd_set_pxl_fmt() is not supposed to return zero if
>the format is not supported and here we would stop enabling the pixel clock.
>
>Do you care to send a patch for fixing the bug, rather than this one?
>
>Best regards,
>Liviu
>

Sure, I do have a bit confusing about this code, I will resubmit a patch and try to fix it.

Regards,
Bernard

>>  
>>  	clk_set_rate(hdlcd->clk, m->crtc_clock * 1000);
>>  }
>> -- 
>> 2.26.2
>> 
>
>-- 
>====================
>| I would like to |
>| fix the world,  |
>| but they're not |
>| giving me the   |
> \ source code!  /
>  ---------------
>    ¯\_(ツ)_/¯

Patch
diff mbox series

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index af67fefed38d..32bda13250f5 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -160,9 +160,7 @@  static void hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	hdlcd_write(hdlcd, HDLCD_REG_H_SYNC, vm.hsync_len - 1);
 	hdlcd_write(hdlcd, HDLCD_REG_POLARITIES, polarities);
 
-	err = hdlcd_set_pxl_fmt(crtc);
-	if (err)
-		return;
+	hdlcd_set_pxl_fmt(crtc);
 
 	clk_set_rate(hdlcd->clk, m->crtc_clock * 1000);
 }