[PATCH] drm/amd/amdgpu: Remove redundant else branch in amdgpu_encoders.c

[SHANMUGAM, SRINIVASAN]

[AMD Official Use Only - General]

-----Original Message-----
From: Alex Deucher <alexdeucher at gmail.com>
Sent: Thursday, May 11, 2023 7:55 AM
To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
Cc: Koenig, Christian <Christian.Koenig at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: Remove redundant else branch in amdgpu_encoders.c

On Tue, May 9, 2023 at 1:17 AM SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com> wrote:
>
> [AMD Official Use Only - General]
>
>
>
> -----Original Message-----
> From: Alex Deucher <alexdeucher at gmail.com>
> Sent: Monday, May 8, 2023 9:27 PM
> To: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
> Cc: Koenig, Christian <Christian.Koenig at amd.com>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amd/amdgpu: Remove redundant else branch in
> amdgpu_encoders.c
>
> On Mon, May 8, 2023 at 11:29 AM Srinivasan Shanmugam <srinivasan.shanmugam at amd.com> wrote:
> >
> > Adhere to Linux kernel coding style.
> >
> > Reported by checkpatch:
> >
> > WARNING: else is not generally useful after a break or return
> >
>
> What about the else in the previous case statement?
>
> Alex
>
> Hi Alex,
>
> Thanks a lot for your feedbacks,
>
> the else in the previous case ie., is binded to if statement ie., "if (amdgpu_connector->use_digital) {", am I correct please?, please correct me, if my understanding is wrong? & the best solution with your tips pls, so that I can edit & resend the patch please?
>

Yes that one.  It follows a similar pattern to the case you changed.
Shouldn't checkpatch warn on both?

Alex

So sorry Alex, couldn't reply instantly, was occupied with something else.

Yes Alex, somehow checkpatch was pointing only to below, hence avoided multiple return statements with a break without affecting functionality & proposed a v2 patch: https://patchwork.freedesktop.org/patch/537520/?series=117468&rev=2

WARNING: else is not generally useful after a break or return
#245: FILE: drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c:245:
+                       return false;
+               else {

Srini
> Much appreciate for your help in advance,
>
> > Cc: Christian König <christian.koenig at amd.com>
> > Cc: Alex Deucher <alexander.deucher at amd.com>
> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 26
> > ++++++++++----------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > index c96e458ed088..049e9976ff34 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c
> > @@ -242,19 +242,18 @@ bool amdgpu_dig_monitor_is_duallink(struct drm_encoder *encoder,
> >                 if ((dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
> >                     (dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP))
> >                         return false;
> > -               else {
> > -                       /* HDMI 1.3 supports up to 340 Mhz over single link */
> > -                       if (connector->display_info.is_hdmi) {
> > -                               if (pixel_clock > 340000)
> > -                                       return true;
> > -                               else
> > -                                       return false;
> > -                       } else {
> > -                               if (pixel_clock > 165000)
> > -                                       return true;
> > -                               else
> > -                                       return false;
> > -                       }
> > +
> > +               /* HDMI 1.3 supports up to 340 Mhz over single link */
> > +               if (connector->display_info.is_hdmi) {
> > +                       if (pixel_clock > 340000)
> > +                               return true;
> > +                       else
> > +                               return false;
> > +               } else {
> > +                       if (pixel_clock > 165000)
> > +                               return true;
> > +                       else
> > +                               return false;
> >                 }
> >         default:
> >                 return false;
> > --
> > 2.25.1
> >