Re: [PATCH] tty: synclink: Adjust indentation and style in several functions

[Nathan Chancellor]

On Fri, Dec 20, 2019 at 10:04:02AM -0800, Nick Desaulniers wrote:
> On Tue, Dec 17, 2019 at 6:28 PM Nathan Chancellor
> <natechancellor@xxxxxxxxx> wrote:
> >
> > Clang warns:
> >
> > ../drivers/tty/synclink.c:1167:3: warning: misleading indentation;
> > statement is not part of the previous 'if' [-Wmisleading-indentation]
> >         if ( (status & RXSTATUS_ABORT_RECEIVED) &&
> >         ^
> > ../drivers/tty/synclink.c:1163:2: note: previous statement is here
> >         if ( debug_level >= DEBUG_LEVEL_ISR )
> >         ^
> > ../drivers/tty/synclink.c:1973:3: warning: misleading indentation;
> > statement is not part of the previous 'if' [-Wmisleading-indentation]
> >         if (I_BRKINT(info->port.tty) || I_PARMRK(info->port.tty))
> >         ^
> > ../drivers/tty/synclink.c:1971:2: note: previous statement is here
> >         if (I_INPCK(info->port.tty))
> >         ^
> > ../drivers/tty/synclink.c:3229:3: warning: misleading indentation;
> > statement is not part of the previous 'else' [-Wmisleading-indentation]
> >         usc_set_serial_signals(info);
> >         ^
> > ../drivers/tty/synclink.c:3227:2: note: previous statement is here
> >         else
> >         ^
> > ../drivers/tty/synclink.c:4918:4: warning: misleading indentation;
> > statement is not part of the previous 'else' [-Wmisleading-indentation]
> >                 if ( info->params.clock_speed )
> >                 ^
> > ../drivers/tty/synclink.c:4901:3: note: previous statement is here
> >                 else
> >                 ^
> > 4 warnings generated.
> >
> > The indentation on these lines is not at all consistent, tabs and spaces
> > are mixed together. Convert to just using tabs to be consistent with the
> > Linux kernel coding style and eliminate these warnings from clang.
> >
> > Additionally, clean up some of lines touched by the indentation shift to
> > eliminate checkpatch warnings and leave this code in a better condition
> > than when it was left.
> 
> Indeed, this file is kind of a mess.
> 
> >
> > -:10: ERROR: trailing whitespace
> > -:10: ERROR: that open brace { should be on the previous line
> > -:10: ERROR: space prohibited after that open parenthesis '('
> > -:14: ERROR: space prohibited before that close parenthesis ')'
> > -:82: ERROR: trailing whitespace
> > -:87: WARNING: Block comments use a trailing */ on a separate line
> > -:88: ERROR: that open brace { should be on the previous line
> > -:88: ERROR: space prohibited after that open parenthesis '('
> > -:88: ERROR: space prohibited before that close parenthesis ')'
> > -:99: ERROR: else should follow close brace '}'
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/821
> > Signed-off-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
> > ---
> >  drivers/tty/synclink.c | 55 ++++++++++++++++++++----------------------
> >  1 file changed, 26 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/tty/synclink.c b/drivers/tty/synclink.c
> > index 61dc6b4a43d0..586810defb21 100644
> > --- a/drivers/tty/synclink.c
> > +++ b/drivers/tty/synclink.c
> > @@ -1164,21 +1164,20 @@ static void mgsl_isr_receive_status( struct mgsl_struct *info )
> >                 printk("%s(%d):mgsl_isr_receive_status status=%04X\n",
> >                         __FILE__,__LINE__,status);
> >
> > -       if ( (status & RXSTATUS_ABORT_RECEIVED) &&
> > +       if ((status & RXSTATUS_ABORT_RECEIVED) &&
> >                 info->loopmode_insert_requested &&
> > -               usc_loopmode_active(info) )
> > -       {
> > +               usc_loopmode_active(info)) {
> >                 ++info->icount.rxabort;
> > -               info->loopmode_insert_requested = false;
> > -
> > -               /* clear CMR:13 to start echoing RxD to TxD */
> > +               info->loopmode_insert_requested = false;
> > +
> > +               /* clear CMR:13 to start echoing RxD to TxD */
> >                 info->cmr_value &= ~BIT13;
> > -               usc_OutReg(info, CMR, info->cmr_value);
> > -
> > +               usc_OutReg(info, CMR, info->cmr_value);
> > +
> >                 /* disable received abort irq (no longer required) */
> > -               usc_OutReg(info, RICR,
> > -                       (usc_InReg(info, RICR) & ~RXSTATUS_ABORT_RECEIVED));
> > -       }
> > +               usc_OutReg(info, RICR,
> > +                       (usc_InReg(info, RICR) & ~RXSTATUS_ABORT_RECEIVED));
> > +       }
> >
> >         if (status & (RXSTATUS_EXITED_HUNT | RXSTATUS_IDLE_RECEIVED)) {
> >                 if (status & RXSTATUS_EXITED_HUNT)
> > @@ -1970,8 +1969,8 @@ static void mgsl_change_params(struct mgsl_struct *info)
> 
> I'm surprised the next hunk isn't mgsl_isr_transmit_status() in
> L1211-L1268?  I don't mind reformatting this file, but would you mind:
> 1. splitting the changes that fix the warning and reformatting the
> rest of the file in two?  That way the warning fix is more likely to
> merge back cleanly to LTS branches with less risk of merge conflict?
> Warning fix first, then reformat.
> 2. reformat the whole thing, not just most of it.

Yes, I will go ahead and break down these three TTY commits into six
(first three fixing the Clang warnings then the next three fixing all of
the indentation and spacing warnings).

I should be able to do this tonight or tomorrow at some point.

Cheers,
Nathan