On Tue, 25 Jun 2013, Manjunath Goudar wrote: > Separate the Samsung OHCI S3CXXXX host controller driver from ohci-hcd > host code so that it can be built as a separate driver module. > This work is part of enabling multi-platform kernels on ARM; > it would be nice to have in 3.11. This patch looks very good. I have only two very small nits: > diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c > index e125770..b0f6644 100644 > --- a/drivers/usb/host/ohci-s3c2410.c > +++ b/drivers/usb/host/ohci-s3c2410.c > @@ -19,17 +19,34 @@ > * This file is licenced under the GPL. > */ > > -#include <linux/platform_device.h> > #include <linux/clk.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > #include <linux/platform_data/usb-ohci-s3c2410.h> > +#include <linux/usb.h> > +#include <linux/usb/hcd.h> > + > +#include "ohci.h" > + > > #define valid_port(idx) ((idx) == 1 || (idx) == 2) > > /* clock device associated with the hcd */ > > + > +#define DRIVER_DESC "OHCI S3CXXXX driver" > + > +static const char hcd_name[] = "ohci-s3cxxxx"; > + > static struct clk *clk; > static struct clk *usb_clk; > > +static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq, > + u16 wValue, u16 wIndex, char *buf, u16 wLength); > +static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf); > + > /* forward definitions */ Do you understand what "forward definitions" means? It refers to declarations of functions whose actual code is given later. In other words, your declarations of orig_ohci_hub_control and orig_ohci_hub_status_data belong just after this comment, not just before it. > @@ -93,7 +110,7 @@ ohci_s3c2410_hub_status_data(struct usb_hcd *hcd, char *buf) > int orig; > int portno; > > - orig = ohci_hub_status_data(hcd, buf); > + orig = orig_ohci_hub_status_data(hcd, buf); Since you're changing the line anyway, you should get rid of the extra space before the '='. After you make those two changes, you can add my Acked-by. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
