| Message ID | 20200422021046.4375-1-bernard@vivo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/arm: cleanup coding style in arm a bit | expand |
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 >
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! / > --------------- > ¯\_(ツ)_/¯
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); }
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(-)