Re:Re: [PATCH] drm/arm: cleanup coding style in arm a bit

[èåå]

From: Liviu Dudau <liviu.dudau@xxxxxxx>
Date: 2020-04-23 20:50:07
To:  Bernard Zhao <bernard@xxxxxxxx>
Cc:  Brian Starkey <brian.starkey@xxxxxxx>,David Airlie <airlied@xxxxxxxx>,Daniel Vetter <daniel@xxxxxxxx>,dri-devel@xxxxxxxxxxxxxxxxxxxxx,linux-kernel@xxxxxxxxxxxxxxx,opensource.kernel@xxxxxxxx
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@xxxxxxxx>
>> ---
>>  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!  /
>  ---------------
>    Â\_(ã)_/Â