* [PATCH] tty: synclink: Adjust indentation and style in several functions
@ 2019-12-18 2:27 Nathan Chancellor
2019-12-20 18:04 ` Nick Desaulniers
0 siblings, 1 reply; 3+ messages in thread
From: Nathan Chancellor @ 2019-12-18 2:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, clang-built-linux, Nathan Chancellor
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.
-: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@gmail.com>
---
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)
info->read_status_mask = RXSTATUS_OVERRUN;
if (I_INPCK(info->port.tty))
info->read_status_mask |= RXSTATUS_PARITY_ERROR | RXSTATUS_FRAMING_ERROR;
- if (I_BRKINT(info->port.tty) || I_PARMRK(info->port.tty))
- info->read_status_mask |= RXSTATUS_BREAK_RECEIVED;
+ if (I_BRKINT(info->port.tty) || I_PARMRK(info->port.tty))
+ info->read_status_mask |= RXSTATUS_BREAK_RECEIVED;
if (I_IGNPAR(info->port.tty))
info->ignore_status_mask |= RXSTATUS_PARITY_ERROR | RXSTATUS_FRAMING_ERROR;
@@ -3211,7 +3210,7 @@ static int carrier_raised(struct tty_port *port)
struct mgsl_struct *info = container_of(port, struct mgsl_struct, port);
spin_lock_irqsave(&info->irq_spinlock, flags);
- usc_get_serial_signals(info);
+ usc_get_serial_signals(info);
spin_unlock_irqrestore(&info->irq_spinlock, flags);
return (info->serial_signals & SerialSignal_DCD) ? 1 : 0;
}
@@ -3226,7 +3225,7 @@ static void dtr_rts(struct tty_port *port, int on)
info->serial_signals |= SerialSignal_RTS | SerialSignal_DTR;
else
info->serial_signals &= ~(SerialSignal_RTS | SerialSignal_DTR);
- usc_set_serial_signals(info);
+ usc_set_serial_signals(info);
spin_unlock_irqrestore(&info->irq_spinlock,flags);
}
@@ -4907,24 +4906,22 @@ static void usc_set_sdlc_mode( struct mgsl_struct *info )
/* of rounding up and then subtracting 1 we just don't subtract */
/* the one in this case. */
- /*--------------------------------------------------
- * ejz: for DPLL mode, application should use the
- * same clock speed as the partner system, even
- * though clocking is derived from the input RxData.
- * In case the user uses a 0 for the clock speed,
- * default to 0xffffffff and don't try to divide by
- * zero
- *--------------------------------------------------*/
- if ( info->params.clock_speed )
- {
+ /*
+ * ejz: for DPLL mode, application should use the
+ * same clock speed as the partner system, even
+ * though clocking is derived from the input RxData.
+ * In case the user uses a 0 for the clock speed,
+ * default to 0xffffffff and don't try to divide by
+ * zero
+ */
+ if (info->params.clock_speed) {
Tc = (u16)((XtalSpeed/DpllDivisor)/info->params.clock_speed);
if ( !((((XtalSpeed/DpllDivisor) % info->params.clock_speed) * 2)
/ info->params.clock_speed) )
Tc--;
- }
- else
- Tc = -1;
-
+ } else {
+ Tc = -1;
+ }
/* Write 16-bit Time Constant for BRG1 */
usc_OutReg( info, TC1R, Tc );
--
2.24.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] tty: synclink: Adjust indentation and style in several functions
2019-12-18 2:27 [PATCH] tty: synclink: Adjust indentation and style in several functions Nathan Chancellor
@ 2019-12-20 18:04 ` Nick Desaulniers
2019-12-20 19:12 ` Nathan Chancellor
0 siblings, 1 reply; 3+ messages in thread
From: Nick Desaulniers @ 2019-12-20 18:04 UTC (permalink / raw)
To: Nathan Chancellor; +Cc: Greg Kroah-Hartman, Jiri Slaby, LKML, clang-built-linux
On Tue, Dec 17, 2019 at 6:28 PM Nathan Chancellor
<natechancellor@gmail.com> 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@gmail.com>
> ---
> 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.
It's easier to see if you set up your editor to highlight all tabs and spaces.
mgsl_isr_io_pin() for instance has trailing tabs, tabs between
`struct` and `mgsl_icount`...
mgsl_change_params() has tabs followed by spaces followed by nothing...
> info->read_status_mask = RXSTATUS_OVERRUN;
> if (I_INPCK(info->port.tty))
> info->read_status_mask |= RXSTATUS_PARITY_ERROR | RXSTATUS_FRAMING_ERROR;
> - if (I_BRKINT(info->port.tty) || I_PARMRK(info->port.tty))
> - info->read_status_mask |= RXSTATUS_BREAK_RECEIVED;
> + if (I_BRKINT(info->port.tty) || I_PARMRK(info->port.tty))
> + info->read_status_mask |= RXSTATUS_BREAK_RECEIVED;
>
> if (I_IGNPAR(info->port.tty))
> info->ignore_status_mask |= RXSTATUS_PARITY_ERROR | RXSTATUS_FRAMING_ERROR;
> @@ -3211,7 +3210,7 @@ static int carrier_raised(struct tty_port *port)
> struct mgsl_struct *info = container_of(port, struct mgsl_struct, port);
>
> spin_lock_irqsave(&info->irq_spinlock, flags);
> - usc_get_serial_signals(info);
> + usc_get_serial_signals(info);
> spin_unlock_irqrestore(&info->irq_spinlock, flags);
> return (info->serial_signals & SerialSignal_DCD) ? 1 : 0;
> }
> @@ -3226,7 +3225,7 @@ static void dtr_rts(struct tty_port *port, int on)
> info->serial_signals |= SerialSignal_RTS | SerialSignal_DTR;
> else
> info->serial_signals &= ~(SerialSignal_RTS | SerialSignal_DTR);
> - usc_set_serial_signals(info);
> + usc_set_serial_signals(info);
> spin_unlock_irqrestore(&info->irq_spinlock,flags);
> }
>
> @@ -4907,24 +4906,22 @@ static void usc_set_sdlc_mode( struct mgsl_struct *info )
> /* of rounding up and then subtracting 1 we just don't subtract */
> /* the one in this case. */
>
> - /*--------------------------------------------------
> - * ejz: for DPLL mode, application should use the
> - * same clock speed as the partner system, even
> - * though clocking is derived from the input RxData.
> - * In case the user uses a 0 for the clock speed,
> - * default to 0xffffffff and don't try to divide by
> - * zero
> - *--------------------------------------------------*/
> - if ( info->params.clock_speed )
> - {
> + /*
> + * ejz: for DPLL mode, application should use the
> + * same clock speed as the partner system, even
> + * though clocking is derived from the input RxData.
> + * In case the user uses a 0 for the clock speed,
> + * default to 0xffffffff and don't try to divide by
> + * zero
> + */
> + if (info->params.clock_speed) {
> Tc = (u16)((XtalSpeed/DpllDivisor)/info->params.clock_speed);
> if ( !((((XtalSpeed/DpllDivisor) % info->params.clock_speed) * 2)
> / info->params.clock_speed) )
> Tc--;
> - }
> - else
> - Tc = -1;
> -
> + } else {
> + Tc = -1;
> + }
>
> /* Write 16-bit Time Constant for BRG1 */
> usc_OutReg( info, TC1R, Tc );
> --
> 2.24.1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20191218022758.53697-1-natechancellor%40gmail.com.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tty: synclink: Adjust indentation and style in several functions
2019-12-20 18:04 ` Nick Desaulniers
@ 2019-12-20 19:12 ` Nathan Chancellor
0 siblings, 0 replies; 3+ messages in thread
From: Nathan Chancellor @ 2019-12-20 19:12 UTC (permalink / raw)
To: Nick Desaulniers; +Cc: Greg Kroah-Hartman, Jiri Slaby, LKML, clang-built-linux
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@gmail.com> 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@gmail.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-20 19:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 2:27 [PATCH] tty: synclink: Adjust indentation and style in several functions Nathan Chancellor
2019-12-20 18:04 ` Nick Desaulniers
2019-12-20 19:12 ` Nathan Chancellor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).