All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Paul B Schroeder <pschroeder@uplogix.com>
Cc: linux-usb-devel@lists.sourceforge.net,
	linux-kernel@vger.kernel.org,
	"'TechSupport'" <techsupport@moschip.com>,
	AJN Rao <ajnrao@moschip.com>, AJN Rao <ajn@moschip.com>
Subject: Re: [PATCH] Moschip 7840 USB-Serial Driver
Date: Tue, 29 Aug 2006 00:51:03 -0700	[thread overview]
Message-ID: <20060829075103.GA5952@kroah.com> (raw)
In-Reply-To: <44F3ED45.7080502@uplogix.com>

On Tue, Aug 29, 2006 at 02:31:17AM -0500, Paul B Schroeder wrote:
> Worked with the tech support folks at Moschip Semiconductor to get the 
> driver working on the latest kernel.  We've been using it for a bit now and 
> it appears to be working well.  They're okay with kernel inclusion of the 
> driver.  I've cleaned it up a bit and put this patch together.
> 
> The patch is against 2.6.18-rc5.  Please apply, it can be found here:
> http://paul.schroeder.name/DEV/kernel/mos7840.patch

Great, but can you submit it as per the Documentation/SubmittingPatches
file?

Also, I've included the following text at the top of the file in my port
of the mos7720 driver from the same developers.  It also applies to this
driver and you should add it too:

 * Originally based on drivers/usb/serial/io_edgeport.c which is:
 *      Copyright (C) 2000 Inside Out Networks, All rights reserved.
 *      Copyright (C) 2001-2002 Greg Kroah-Hartman <greg@kroah.com>

You can see the same function names and comments, as well as variable
usages where they don't really need to be used.

Can you also please clean up those functions to be more kernel-like, and
fix the variable names to follow the coding style rules of the kernel?

The #defines at the start of the file also can be removed, as they are
not needed (TRUE and FALSE don't belong in a driver, use 0 and -ERR
instead).

Also, look at the variables in struct moschip_port.  A lot of them can
be simply removed as they are never used.  The wait queues are one
example of these.  And the use of the "__" type of variables should be
examined, as I don't think it's necessary as these variables never cross
the kernel/user boundry.

There are also a lot of global variables that should be static.

I'll go over the rest when you submit it in an email message so it can
be quoted.

It's a great start though, I remember what the original version looked
like :)

thanks,

greg k-h

  reply	other threads:[~2006-08-29  7:52 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-29  7:31 [PATCH] Moschip 7840 USB-Serial Driver Paul B Schroeder
2006-08-29  7:51 ` Greg KH [this message]
2006-08-29  9:34 ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060829075103.GA5952@kroah.com \
    --to=greg@kroah.com \
    --cc=ajn@moschip.com \
    --cc=ajnrao@moschip.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb-devel@lists.sourceforge.net \
    --cc=pschroeder@uplogix.com \
    --cc=techsupport@moschip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.