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 <[email protected]>
---
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);
clk_set_rate(hdlcd->clk, m->crtc_clock * 1000);
}
--
2.26.2
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 <[email protected]>
> ---
> 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
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
From: Liviu Dudau <[email protected]>
Date: 2020-04-23 20:50:07
To: Bernard Zhao <[email protected]>
Cc: Brian Starkey <[email protected]>,David Airlie <[email protected]>,Daniel Vetter <[email protected]>,[email protected],[email protected],[email protected]
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 <[email protected]>
>> ---
>> 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! /
> ---------------
> ¯\_(ツ)_/¯