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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
