Re: [PATCH] usb: host: Faraday fotg210-hcd driver

From: Greg KH
Date: Tue Jun 18 2013 - 10:56:56 EST


On Tue, Jun 18, 2013 at 10:42:09AM +0800, Yuan-Hsin Chen wrote:
> Hi,
>
> On Tue, Jun 18, 2013 at 4:39 AM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Wed, Jun 05, 2013 at 05:15:43PM +0000, Yuan-Hsin Chen wrote:
> >> FOTG210 is an OTG controller which can be configured as an
> >> USB2.0 host. FOTG210 host is an ehci-like controller with
> >> some differences. First, register layout of FOTG210 is
> >> incompatible with EHCI. Furthermore, FOTG210 is lack of
> >> siTDs which means iTDs are used for both HS and FS ISO
> >> transfer.
> >>
> >> Signed-off-by: Yuan-Hsin Chen <yhchen@xxxxxxxxxxxxxxxx>
> >> ---
> >> drivers/usb/Makefile | 1 +
> >> drivers/usb/host/Kconfig | 12 +
> >> drivers/usb/host/Makefile | 1 +
> >> drivers/usb/host/fotg210-hcd.c | 5967 ++++++++++++++++++++++++++++++++++++++++
> >> drivers/usb/host/fotg210.h | 746 +++++
> >> 5 files changed, 6727 insertions(+), 0 deletions(-)
> >> create mode 100644 drivers/usb/host/fotg210-hcd.c
> >> create mode 100644 drivers/usb/host/fotg210.h
> >
> > You obviously didn't even run this through checkpatch.pl, did you?
> >
> > $ ./scripts/checkpatch.pl --terse ../usb.mbox | tail -n 1
> > total: 138 errors, 618 warnings, 6742 lines checked
> >
> > Please fix all of these if you wish us to at least start reviewing the
> > patch. Your internal Q/A should have caught this first, please be more
> > careful in the future.
> >
>
> Actually I did run checkpatch.pl and found that almost all errors and
> warnings are from ehci core (ehci-hcd.c, ehci-hub.c and so on) where
> my driver borrowed most of code.
>
> $ ./scripts/checkpatch.pl --terse -f drivers/usb/host/ehci-hub.c | tail -n 1
> total: 18 errors, 69 warnings, 1138 lines checked
>
> $ ./scripts/checkpatch.pl --terse -f drivers/usb/host/ehci-hcd.c | tail -n 1
> total: 17 errors, 78 warnings, 1403 lines checked
>
> So you're saying that I should fix them, is that right?

At the very least, yes, you should. And those tiny numbers of errors do
not line up with the much larger number of errors in your patch
submission, so it isn't just a "copy and paste" issue here.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/