Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1474513 > unrolled thread

[PATCH] staging: dgnc: Let line have less 80 char

Back to article view | Back to linux.kernel

Contents

  [PATCH] staging: dgnc: Let line have less 80 char Sean <sean@sean.taipei> - 2016-09-01 18:00 +0200
    Re: [PATCH] staging: dgnc: Let line have less 80 char Greg KH <gregkh@linuxfoundation.org> - 2016-09-01 18:10 +0200
    Re: [PATCH] staging: dgnc: Let line have less 80 char Richard Weinberger <richard.weinberger@gmail.com> - 2016-09-01 18:20 +0200

#1474513 — [PATCH] staging: dgnc: Let line have less 80 char

This patch fix a minor checkpath warming:

"WARNING: line over 80 characters"

Signed-off-by: Sean Wei <sean@sean.taipei>
---
 drivers/staging/dgnc/dgnc_neo.c | 116 ++++++++++++++++++++++++++++------------
 1 file changed, 82 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index ba57e95..37fb556 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -107,7 +107,7 @@ static inline void neo_set_cts_flow_control(struct channel_t *ch)
 	/* Turn off auto Xon flow control */
 	efr &= ~UART_17158_EFR_IXON;
 
-	/* Why? Becuz Exar's spec says we have to zero it out before setting it */
+	/* Becuz Exar's spec says we have to zero it out before setting it */
 	writeb(0, &ch->ch_neo_uart->efr);
 
 	/* Turn on UART enhanced bits */
@@ -143,7 +143,7 @@ static inline void neo_set_rts_flow_control(struct channel_t *ch)
 	ier &= ~UART_17158_IER_XOFF;
 	efr &= ~UART_17158_EFR_IXOFF;
 
-	/* Why? Becuz Exar's spec says we have to zero it out before setting it */
+	/* Becuz Exar's spec says we have to zero it out before setting it */
 	writeb(0, &ch->ch_neo_uart->efr);
 
 	/* Turn on UART enhanced bits */
@@ -181,7 +181,7 @@ static inline void neo_set_ixon_flow_control(struct channel_t *ch)
 	/* Turn on auto Xon flow control */
 	efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXON);
 
-	/* Why? Becuz Exar's spec says we have to zero it out before setting it */
+	/* Becuz Exar's spec says we have to zero it out before setting it */
 	writeb(0, &ch->ch_neo_uart->efr);
 
 	/* Turn on UART enhanced bits */
@@ -219,7 +219,7 @@ static inline void neo_set_ixoff_flow_control(struct channel_t *ch)
 	ier |= UART_17158_IER_XOFF;
 	efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXOFF);
 
-	/* Why? Becuz Exar's spec says we have to zero it out before setting it */
+	/* Becuz Exar's spec says we have to zero it out before setting it */
 	writeb(0, &ch->ch_neo_uart->efr);
 
 	/* Turn on UART enhanced bits */
@@ -260,7 +260,7 @@ static inline void neo_set_no_input_flow_control(struct channel_t *ch)
 	else
 		efr &= ~(UART_17158_EFR_ECB | UART_17158_EFR_IXOFF);
 
-	/* Why? Becuz Exar's spec says we have to zero it out before setting it */
+	/* Becuz Exar's spec says we have to zero it out before setting it */
 	writeb(0, &ch->ch_neo_uart->efr);
 
 	/* Turn on UART enhanced bits */
@@ -298,7 +298,7 @@ static inline void neo_set_no_output_flow_control(struct channel_t *ch)
 	else
 		efr &= ~(UART_17158_EFR_ECB | UART_17158_EFR_IXON);
 
-	/* Why? Becuz Exar's spec says we have to zero it out before setting it */
+	/* Becuz Exar's spec says we have to zero it out before setting it */
 	writeb(0, &ch->ch_neo_uart->efr);
 
 	/* Turn on UART enhanced bits */
@@ -403,7 +403,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
 			ch->ch_intr_rx++;
 			neo_copy_data_from_uart_to_queue(ch);
 
-			/* Call our tty layer to enforce queue flow control if needed. */
+			/* 
+			 * Call our tty layer to enforcer
+			 * queue flow control if needed.
+			 */
 			spin_lock_irqsave(&ch->ch_lock, flags);
 			dgnc_check_queue_flow_control(ch);
 			spin_unlock_irqrestore(&ch->ch_lock, flags);
@@ -428,7 +431,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
 			 * one it was, so we can suspend or resume data flow.
 			 */
 			if (cause == UART_17158_XON_DETECT) {
-				/* Is output stopped right now, if so, resume it */
+				/*
+				 * Is output stopped right now,
+				 * if so, resume it
+				 */
 				if (brd->channels[port]->ch_flags & CH_STOP) {
 					spin_lock_irqsave(&ch->ch_lock,
 							  flags);
@@ -449,8 +455,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
 
 		if (isr & UART_17158_IIR_HWFLOW_STATE_CHANGE) {
 			/*
-			 * If we get here, this means the hardware is doing auto flow control.
-			 * Check to see whether RTS/DTR or CTS/DSR caused this interrupt.
+			 * If we get here,
+			 * this means the hardware is doing auto flow control.
+			 * Check to see whether RTS/DTR or CTS/DSR
+			 * caused this interrupt.
 			 */
 			brd->intr_modem++;
 			ch->ch_intr_modem++;
@@ -545,7 +553,8 @@ static inline void neo_parse_lsr(struct dgnc_board *brd, uint port)
 		 * Rx Oruns. Exar says that an orun will NOT corrupt
 		 * the FIFO. It will just replace the holding register
 		 * with this new data byte. So basically just ignore this.
-		 * Probably we should eventually have an orun stat in our driver...
+		 * Probably we should eventually have
+		 * an orun stat in our driver...
 		 */
 		ch->ch_err_overrun++;
 	}
@@ -665,7 +674,10 @@ static void neo_param(struct tty_struct *tty)
 				4800,   9600,   19200,  38400 }
 		};
 
-		/* Only use the TXPrint baud rate if the terminal unit is NOT open */
+		/*
+		 * Only use the TXPrint baud rate
+		 * if the terminal unit is NOT open
+		 */
 		if (!(ch->ch_tun.un_flags & UN_ISOPEN) &&
 		    (un->un_type == DGNC_PRINT))
 			baud = C_BAUD(ch->ch_pun.un_tty) & 0xff;
@@ -788,7 +800,10 @@ static void neo_param(struct tty_struct *tty)
 	if (ch->ch_digi.digi_flags & CTSPACE || ch->ch_c_cflag & CRTSCTS) {
 		neo_set_cts_flow_control(ch);
 	} else if (ch->ch_c_iflag & IXON) {
-		/* If start/stop is set to disable, then we should disable flow control */
+		/*
+		 * If start/stop is set to disable,
+		 * then we should disable flow control.
+		*/
 		if ((ch->ch_startc == _POSIX_VDISABLE) ||
 		    (ch->ch_stopc == _POSIX_VDISABLE))
 			neo_set_no_output_flow_control(ch);
@@ -801,7 +816,10 @@ static void neo_param(struct tty_struct *tty)
 	if (ch->ch_digi.digi_flags & RTSPACE || ch->ch_c_cflag & CRTSCTS) {
 		neo_set_rts_flow_control(ch);
 	} else if (ch->ch_c_iflag & IXOFF) {
-		/* If start/stop is set to disable, then we should disable flow control */
+		/*
+		 * If start/stop is set to disable,
+		 * then we should disable flow control.
+		 */
 		if ((ch->ch_startc == _POSIX_VDISABLE) ||
 		    (ch->ch_stopc == _POSIX_VDISABLE))
 			neo_set_no_input_flow_control(ch);
@@ -940,14 +958,19 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
 
 	/*
 	 * If 0, no interrupts pending.
-	 * This can happen if the IRQ is shared among a couple Neo/Classic boards.
+	 * This can happen if the IRQ is shared
+	 * among a couple Neo/Classic boards.
 	 */
 	if (!uart_poll) {
 		spin_unlock_irqrestore(&brd->bd_intr_lock, flags);
 		return IRQ_NONE;
 	}
 
-	/* At this point, we have at least SOMETHING to service, dig further... */
+	/*
+	 * At this point,
+	 * we have at least SOMETHING to service,
+	 * dig further...
+	 */
 
 	/* Loop on each port */
 	while ((uart_poll & 0xff) != 0) {
@@ -971,7 +994,10 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
 			ch = brd->channels[port];
 			neo_copy_data_from_uart_to_queue(ch);
 
-			/* Call our tty layer to enforce queue flow control if needed. */
+			/*
+			 * Call our tty layer to
+			 * enforce queue flow control if needed.
+			 */
 			spin_lock_irqsave(&ch->ch_lock, flags2);
 			dgnc_check_queue_flow_control(ch);
 			spin_unlock_irqrestore(&ch->ch_lock, flags2);
@@ -987,16 +1013,19 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
 
 		case UART_17158_TXRDY:
 			/*
-			 * TXRDY interrupt clears after reading ISR register for the UART channel.
+			 * TXRDY interrupt clears after reading ISR register
+			 * for the UART channel.
 			 */
 
 			/*
 			 * Yes, this is odd...
 			 * Why would I check EVERY possibility of type of
 			 * interrupt, when we know its TXRDY???
-			 * Becuz for some reason, even tho we got triggered for TXRDY,
-			 * it seems to be occasionally wrong. Instead of TX, which
-			 * it should be, I was getting things like RXDY too. Weird.
+			 * Becuz for some reason,
+			 * even tho we got triggered for TXRDY,
+			 * it seems to be occasionally wrong.
+			 * Instead of TX, which it should be,
+			 * I was getting things like RXDY too. Weird.
 			 */
 			neo_parse_isr(brd, port);
 			break;
@@ -1011,7 +1040,8 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
 		default:
 			/*
 			 * The UART triggered us with a bogus interrupt type.
-			 * It appears the Exar chip, when REALLY bogged down, will throw
+			 * It appears the Exar chip,
+			 * when REALLY bogged down, will throw
 			 * these once and awhile.
 			 * Its harmless, just ignore it and move on.
 			 */
@@ -1230,7 +1260,8 @@ static void neo_copy_data_from_uart_to_queue(struct channel_t *ch)
 		}
 
 		/*
-		 * If our queue is full, we have no choice but to drop some data.
+		 * If our queue is full,
+		 * we have no choice but to drop some data.
 		 * The assumption is that HWFLOW or SWFLOW should have stopped
 		 * things way way before we got to this point.
 		 *
@@ -1323,7 +1354,10 @@ static void neo_flush_uart_write(struct channel_t *ch)
 	neo_pci_posting_flush(ch->ch_bd);
 
 	for (i = 0; i < 10; i++) {
-		/* Check to see if the UART feels it completely flushed the FIFO. */
+		/*
+		 * Check to see if the UART feels it completely
+		 * flushed the FIFO.
+		 */
 		tmp = readb(&ch->ch_neo_uart->isr_fcr);
 		if (tmp & 4)
 			udelay(10);
@@ -1352,7 +1386,10 @@ static void neo_flush_uart_read(struct channel_t *ch)
 	neo_pci_posting_flush(ch->ch_bd);
 
 	for (i = 0; i < 10; i++) {
-		/* Check to see if the UART feels it completely flushed the FIFO. */
+		/*
+		 * Check to see if the UART feels it completely
+		 * flushed the FIFO.
+		 */
 		tmp = readb(&ch->ch_neo_uart->isr_fcr);
 		if (tmp & 2)
 			udelay(10);
@@ -1397,8 +1434,10 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 			ch->ch_cached_lsr &= ~(UART_LSR_THRE);
 
 			/*
-			 * If RTS Toggle mode is on, turn on RTS now if not already set,
-			 * and make sure we get an event when the data transfer has completed.
+			 * If RTS Toggle mode is on,
+			 * turn on RTS now if not already set,
+			 * and make sure we get an event
+			 * when the data transfer has completed.
 			 */
 			if (ch->ch_digi.digi_flags & DIGI_RTS_TOGGLE) {
 				if (!(ch->ch_mostat & UART_MCR_RTS)) {
@@ -1408,8 +1447,10 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 				ch->ch_tun.un_flags |= (UN_EMPTY);
 			}
 			/*
-			 * If DTR Toggle mode is on, turn on DTR now if not already set,
-			 * and make sure we get an event when the data transfer has completed.
+			 * If DTR Toggle mode is on,
+			 * turn on DTR now if not already set,
+			 * and make sure we get an event
+			 * when the data transfer has completed.
 			 */
 			if (ch->ch_digi.digi_flags & DIGI_DTR_TOGGLE) {
 				if (!(ch->ch_mostat & UART_MCR_DTR)) {
@@ -1464,8 +1505,10 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 			break;
 
 		/*
-		 * If RTS Toggle mode is on, turn on RTS now if not already set,
-		 * and make sure we get an event when the data transfer has completed.
+		 * If RTS Toggle mode is on,
+		 * turn on RTS now if not already set,
+		 * and make sure we get an event
+		 * when the data transfer has completed.
 		 */
 		if (ch->ch_digi.digi_flags & DIGI_RTS_TOGGLE) {
 			if (!(ch->ch_mostat & UART_MCR_RTS)) {
@@ -1476,8 +1519,10 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 		}
 
 		/*
-		 * If DTR Toggle mode is on, turn on DTR now if not already set,
-		 * and make sure we get an event when the data transfer has completed.
+		 * If DTR Toggle mode is on,
+		 * turn on DTR now if not already set,
+		 * and make sure we get an event
+		 * when the data transfer has completed.
 		 */
 		if (ch->ch_digi.digi_flags & DIGI_DTR_TOGGLE) {
 			if (!(ch->ch_mostat & UART_MCR_DTR)) {
@@ -1541,7 +1586,10 @@ static void neo_parse_modem(struct channel_t *ch, unsigned char signals)
 		}
 	}
 
-	/* Scrub off lower bits. They signify delta's, which I don't care about */
+	/*
+	* Scrub off lower bits.
+	* They signify delta's, which I don't care about
+	*/
 	msignals &= 0xf0;
 
 	if (msignals & UART_MSR_DCD)
-- 
2.7.4

[toc] | [next] | [standalone]

#1474516

On Thu, Sep 01, 2016 at 11:58:41PM +0800, Sean wrote:
> This patch fix a minor checkpath warming:
> 
> "WARNING: line over 80 characters"
> 
> Signed-off-by: Sean Wei <sean@sean.taipei>
> ---
>  drivers/staging/dgnc/dgnc_neo.c | 116 ++++++++++++++++++++++++++++------------
>  1 file changed, 82 insertions(+), 34 deletions(-)

Please slow down and take your time.

Now your Signed-off-by: line does not match your "From:" line in your
email header :(

Please take a day or so before resending this, there's no rush.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1474521

Sean,

On Thu, Sep 1, 2016 at 5:58 PM, Sean <sean@sean.taipei> wrote:
> This patch fix a minor checkpath warming:
>
> "WARNING: line over 80 characters"

Please don't do patches just because of the sake of checkpatch.pl.
Especially since "line over 80 characters" is only a warning, not an error.
Your changelog fails to explain why your change made the driver better.

> Signed-off-by: Sean Wei <sean@sean.taipei>
> ---
>  drivers/staging/dgnc/dgnc_neo.c | 116 ++++++++++++++++++++++++++++------------
>  1 file changed, 82 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
> index ba57e95..37fb556 100644
> --- a/drivers/staging/dgnc/dgnc_neo.c
> +++ b/drivers/staging/dgnc/dgnc_neo.c
> @@ -107,7 +107,7 @@ static inline void neo_set_cts_flow_control(struct channel_t *ch)
>         /* Turn off auto Xon flow control */
>         efr &= ~UART_17158_EFR_IXON;
>
> -       /* Why? Becuz Exar's spec says we have to zero it out before setting it */
> +       /* Becuz Exar's spec says we have to zero it out before setting it */

Okay, removing "Why?" silents checkpatch.pl, but the comment is still
crap/useless.
Same for the next few hunks.

[...]

> -               /* Only use the TXPrint baud rate if the terminal unit is NOT open */
> +               /*
> +                * Only use the TXPrint baud rate
> +                * if the terminal unit is NOT open
> +                */

Adding random new lines make the comment not better.

I'm not saying checkpatch.pl is completely useless, stuff in drivers/staging/
often needs to be adopted to the kernel coding style.
But please don't follow it blindly and try to understand what you can improve.

-- 
Thanks,
//richard

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web