[PATCH V3 6/6] USB: OHCI: make ohci-s3c2410 a separate driver

Alan Stern stern at rowland.harvard.edu
Thu Jul 18 16:38:57 EDT 2013


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




More information about the linux-arm-kernel mailing list