Re: [PATCH 01/08] staging:dgap: remove digi debug and tracing code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/28/2014 04:53 PM, Dan Carpenter wrote:
I have some comments on this patch.  It's all minor stuff you can fix in
a later patch.  Don't redo this one.  Staging has more relaxed standards
on whitespace than mm/.  We don't want to slow you down by making you
redo stuff and I don't want to review it more than once if I can avoid
that.

On Fri, Feb 28, 2014 at 12:42:08PM -0500, Mark Hounschell wrote:
The original debug and tracing code is no longer required.
This patch removes it

Signed-off-by: Mark Hounschell <markh@xxxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
-	if ((tty->count == 1) && (un->un_open_count != 1)) {
+	if ((tty->count == 1) && (un->un_open_count != 1))
  		/*
  		 * Uh, oh.  tty->count is 1, which means that the tty
  		 * structure will be freed.  un_open_count should always
@@ -2648,27 +2479,19 @@ static void dgap_tty_close(struct tty_struct *tty, struct file *file)
  		 * one, we've got real problems, since it means the
  		 * serial port won't be shutdown.
  		 */
-		APR(("tty->count is 1, un open count is %d\n", un->un_open_count));
  		un->un_open_count = 1;
-	}

Multi-line indent blocks get curly braces for readability.

@@ -4909,8 +4587,8 @@ static void dgap_do_wait_for_bios(struct board_t *brd)
  	/* Gave up on board after too long of time taken */
  	err1 = readw(addr + SEQUENCE);
  	err2 = readw(addr + ERROR);
-	APR(("***WARNING*** %s failed diagnostics.  Error #(%x,%x).\n",
-		brd->name, err1, err2));
+	pr_warn("dgap: %s failed diagnostics.  Error #(%x,%x).\n",
+		brd->name, err1, err2);
  	brd->state = BOARD_FAILED;
  	brd->dpastatus = BD_NOBIOS;
  }

In the end we're going to want all these things to be dev_warn() and not
pr_warn().  You can remove the "dgap:" prefix because dev_warn() adds a
prefix.  pr_warn() has a way to add a prefix as well, but it's moot
since this will eventually be dev_warn().

Speaking of pr_warn(), also this:

+               if ((brd->type == PPCM) && (true_count == 64))
+                       pr_warn("dgap: %s configured for %d ports, has %d ports.\nPlease make SURE the EBI cable running from the card\nto each EM module is plugged into EBIIN!\n",
+                               brd->name, brd->nasync, true_count);

You can't put newlines in the middle of a print statement.  The lines
are supposed to be prefixed or it messes up dmesg slightly.

@@ -5980,11 +5623,8 @@ static int dgap_param(struct tty_struct *tty)

  		if ((iindex >= 0) && (iindex < 4) && (jindex >= 0) && (jindex < 16)) {
  			baud = bauds[iindex][jindex];
-		} else {
-			DPR_IOCTL(("baud indices were out of range (%d)(%d)",
-				iindex, jindex));
+		} else
  			baud = 0;
-		}


I think "} else" will trigger a new checkpatch.pl warning.

regards,
dan carpenter

OK. I'll follow up on this next week.

Thanks
Mark

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux