diff mbox series

drm/msm/dpu: tidy up some error checking

Message ID ZH7vP2Swu8CYpgUL@moroto
State Superseded
Headers show
Series drm/msm/dpu: tidy up some error checking | expand

Commit Message

Dan Carpenter June 6, 2023, 8:33 a.m. UTC
The "vsync_hz" variable is unsigned int so it can't be less
than zero.  The dpu_kms_get_clk_rate() function used to return a u64
but I previously changed it to return an unsigned long and zero on
error so it matches clk_get_rate().

Change the "vsync_hz" type to unsigned long as well and change the
error checking to check for zero instead of negatives.  This change
does not affect runtime at all.  It's just a clean up.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Abhinav Kumar June 6, 2023, 8:18 p.m. UTC | #1
On 6/6/2023 1:33 AM, Dan Carpenter wrote:
> The "vsync_hz" variable is unsigned int so it can't be less
> than zero.  The dpu_kms_get_clk_rate() function used to return a u64
> but I previously changed it to return an unsigned long and zero on
> error so it matches clk_get_rate().
> 
> Change the "vsync_hz" type to unsigned long as well and change the
> error checking to check for zero instead of negatives.  This change
> does not affect runtime at all.  It's just a clean up.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Dan Carpenter June 7, 2023, 2:42 p.m. UTC | #2
On Tue, Jun 06, 2023 at 10:23:46PM +0200, Marijn Suijten wrote:
> > @@ -359,8 +359,8 @@ static void dpu_encoder_phys_cmd_tearcheck_config(
> >  	 * frequency divided by the no. of rows (lines) in the LCDpanel.
> >  	 */
> >  	vsync_hz = dpu_kms_get_clk_rate(dpu_kms, "vsync");
> > -	if (vsync_hz <= 0) {
> > -		DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %u\n",
> > +	if (!vsync_hz) {
> > +		DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %lu\n",
> >  				 vsync_hz);
> 
> Nit: no need to print the value here, you know it's zero.  Could be
> clarified to just "no vsync clock".
> 

Yeah.  That's obviously not useful.  Sorry, I will resend.

regards,
dan carpenter
Dmitry Baryshkov June 7, 2023, 11:26 p.m. UTC | #3
On 07/06/2023 17:42, Dan Carpenter wrote:
> On Tue, Jun 06, 2023 at 10:23:46PM +0200, Marijn Suijten wrote:
>>> @@ -359,8 +359,8 @@ static void dpu_encoder_phys_cmd_tearcheck_config(
>>>   	 * frequency divided by the no. of rows (lines) in the LCDpanel.
>>>   	 */
>>>   	vsync_hz = dpu_kms_get_clk_rate(dpu_kms, "vsync");
>>> -	if (vsync_hz <= 0) {
>>> -		DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %u\n",
>>> +	if (!vsync_hz) {
>>> +		DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %lu\n",
>>>   				 vsync_hz);
>>
>> Nit: no need to print the value here, you know it's zero.  Could be
>> clarified to just "no vsync clock".
>>
> 
> Yeah.  That's obviously not useful.  Sorry, I will resend.

I'll fix while applying. Seems easier.
Dmitry Baryshkov June 7, 2023, 11:33 p.m. UTC | #4
On Tue, 06 Jun 2023 11:33:03 +0300, Dan Carpenter wrote:
> The "vsync_hz" variable is unsigned int so it can't be less
> than zero.  The dpu_kms_get_clk_rate() function used to return a u64
> but I previously changed it to return an unsigned long and zero on
> error so it matches clk_get_rate().
> 
> Change the "vsync_hz" type to unsigned long as well and change the
> error checking to check for zero instead of negatives.  This change
> does not affect runtime at all.  It's just a clean up.
> 
> [...]

Applied, thanks!

[1/1] drm/msm/dpu: tidy up some error checking
      https://gitlab.freedesktop.org/lumag/msm/-/commit/e7a2cf8e058e

Best regards,
Dan Carpenter June 8, 2023, 11:14 a.m. UTC | #5
On Thu, Jun 08, 2023 at 02:26:14AM +0300, Dmitry Baryshkov wrote:
> On 07/06/2023 17:42, Dan Carpenter wrote:
> > On Tue, Jun 06, 2023 at 10:23:46PM +0200, Marijn Suijten wrote:
> > > > @@ -359,8 +359,8 @@ static void dpu_encoder_phys_cmd_tearcheck_config(
> > > >   	 * frequency divided by the no. of rows (lines) in the LCDpanel.
> > > >   	 */
> > > >   	vsync_hz = dpu_kms_get_clk_rate(dpu_kms, "vsync");
> > > > -	if (vsync_hz <= 0) {
> > > > -		DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %u\n",
> > > > +	if (!vsync_hz) {
> > > > +		DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %lu\n",
> > > >   				 vsync_hz);
> > > 
> > > Nit: no need to print the value here, you know it's zero.  Could be
> > > clarified to just "no vsync clock".
> > > 
> > 
> > Yeah.  That's obviously not useful.  Sorry, I will resend.
> 
> I'll fix while applying. Seems easier.

Thanks!

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index d8ed85a238af..6aecaba14e7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -324,7 +324,7 @@  static void dpu_encoder_phys_cmd_tearcheck_config(
 	struct dpu_hw_tear_check tc_cfg = { 0 };
 	struct drm_display_mode *mode;
 	bool tc_enable = true;
-	u32 vsync_hz;
+	unsigned long vsync_hz;
 	struct dpu_kms *dpu_kms;
 
 	if (phys_enc->has_intf_te) {
@@ -359,8 +359,8 @@  static void dpu_encoder_phys_cmd_tearcheck_config(
 	 * frequency divided by the no. of rows (lines) in the LCDpanel.
 	 */
 	vsync_hz = dpu_kms_get_clk_rate(dpu_kms, "vsync");
-	if (vsync_hz <= 0) {
-		DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %u\n",
+	if (!vsync_hz) {
+		DPU_DEBUG_CMDENC(cmd_enc, "invalid - vsync_hz %lu\n",
 				 vsync_hz);
 		return;
 	}
@@ -381,7 +381,7 @@  static void dpu_encoder_phys_cmd_tearcheck_config(
 	tc_cfg.rd_ptr_irq = mode->vdisplay + 1;
 
 	DPU_DEBUG_CMDENC(cmd_enc,
-		"tc vsync_clk_speed_hz %u vtotal %u vrefresh %u\n",
+		"tc vsync_clk_speed_hz %lu vtotal %u vrefresh %u\n",
 		vsync_hz, mode->vtotal, drm_mode_vrefresh(mode));
 	DPU_DEBUG_CMDENC(cmd_enc,
 		"tc enable %u start_pos %u rd_ptr_irq %u\n",