linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] usb: ehci: ehci-vt8500 bus glue as separate module
@ 2013-02-13  7:31 manjunath.goudar at linaro.org
  2013-02-13  7:31 ` [PATCH] USB: EHCI: make ehci-vt8500 a separate driver manjunath.goudar at linaro.org
  2013-02-13  9:58 ` [PATCH V2] usb: ehci: ehci-vt8500 bus glue as separate module Arnd Bergmann
  0 siblings, 2 replies; 3+ messages in thread
From: manjunath.goudar at linaro.org @ 2013-02-13  7:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Manjunath Goudar <manjunath.goudar@linaro.org>

Separate the vt8500 On-Chip host controller driver from ehci-hcd
host code into its own static driver module

Manjunath Goudar (1):
  USB: EHCI: make ehci-vt8500 a separate driver

 drivers/usb/host/Kconfig       |    8 +++++
 drivers/usb/host/Makefile      |    1 +
 drivers/usb/host/ehci-hcd.c    |    6 +---
 drivers/usb/host/ehci-vt8500.c |   73 ++++++++++++++++++----------------------
 4 files changed, 43 insertions(+), 45 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] USB: EHCI: make ehci-vt8500 a separate driver
  2013-02-13  7:31 [PATCH V2] usb: ehci: ehci-vt8500 bus glue as separate module manjunath.goudar at linaro.org
@ 2013-02-13  7:31 ` manjunath.goudar at linaro.org
  2013-02-13  9:58 ` [PATCH V2] usb: ehci: ehci-vt8500 bus glue as separate module Arnd Bergmann
  1 sibling, 0 replies; 3+ messages in thread
From: manjunath.goudar at linaro.org @ 2013-02-13  7:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Manjunath Goudar <manjunath.goudar@linaro.org>

Separate the vt8500 host controller driver from ehci-hcd host code
into its own driver module.

Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
Cc: Greg KH <greg@kroah.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Tony Prisk <linux@prisktech.co.nz>
Cc: Alexey Charkov <alchark@gmail.com>
Cc: linux-usb at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
---
 drivers/usb/host/Kconfig       |    8 +++++
 drivers/usb/host/Makefile      |    1 +
 drivers/usb/host/ehci-hcd.c    |    6 +---
 drivers/usb/host/ehci-vt8500.c |   73 ++++++++++++++++++----------------------
 4 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 2aa4ece..4e15b87 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -162,6 +162,14 @@ config USB_EHCI_HCD_OMAP
 	  Enables support for the on-chip EHCI controller on
 	  OMAP3 and later chips.
 
+config USB_EHCI_HCD_VT8500
+        tristate "Support for VT8500 on-chip EHCI USB controller"
+        depends on USB_EHCI_HCD && ARCH_VT8500
+        default y
+        ---help---
+          Enables support for the on-chip EHCI controller on
+          VT8500 chips.
+
 config USB_EHCI_HCD_SPEAR
         tristate "Support for ST SPEAr on-chip EHCI USB controller"
         depends on USB_EHCI_HCD && PLAT_SPEAR
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index d593017..1fe0579 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_USB_OXU210HP_HCD)	+= oxu210hp-hcd.o
 obj-$(CONFIG_USB_EHCI_HCD_AT91) += ehci-atmel.o
 obj-$(CONFIG_USB_EHCI_S5P)      += ehci-s5p.o
 obj-$(CONFIG_USB_EHCI_MV)       += ehci-mv.o
+obj-$(CONFIG_USB_EHCI_HCD_VT8500)+= ehci-vt8500.o
 obj-$(CONFIG_USB_ISP116X_HCD)	+= isp116x-hcd.o
 obj-$(CONFIG_USB_ISP1362_HCD)	+= isp1362-hcd.o
 obj-$(CONFIG_USB_OHCI_HCD)	+= ohci-hcd.o
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 9cf4d73..eefa3d7 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1287,11 +1287,6 @@ MODULE_LICENSE ("GPL");
 #define PLATFORM_DRIVER		ehci_octeon_driver
 #endif
 
-#ifdef CONFIG_ARCH_VT8500
-#include "ehci-vt8500.c"
-#define	PLATFORM_DRIVER		vt8500_ehci_driver
-#endif
-
 #ifdef CONFIG_USB_EHCI_MSM
 #include "ehci-msm.c"
 #define PLATFORM_DRIVER		ehci_msm_driver
@@ -1331,6 +1326,7 @@ MODULE_LICENSE ("GPL");
 	!IS_ENABLED(CONFIG_ARCH_AT91) && \
 	!IS_ENABLED(CONFIG_USB_EHCI_S5P) && \
 	!IS_ENABLED(CONFIG_USB_EHCI_MV) && \
+	!IS_ENABLED(CONFIG_ARCH_VT8500) && \
 	!defined(PS3_SYSTEM_BUS_DRIVER) && \
 	!defined(OF_PLATFORM_DRIVER) && \
 	!defined(XILINX_OF_PLATFORM_DRIVER)
diff --git a/drivers/usb/host/ehci-vt8500.c b/drivers/usb/host/ehci-vt8500.c
index 11695d5..98d65bd 100644
--- a/drivers/usb/host/ehci-vt8500.c
+++ b/drivers/usb/host/ehci-vt8500.c
@@ -16,52 +16,25 @@
  *
  */
 
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 
-static const struct hc_driver vt8500_ehci_hc_driver = {
-	.description		= hcd_name,
-	.product_desc		= "VT8500 EHCI",
-	.hcd_priv_size		= sizeof(struct ehci_hcd),
+#include "ehci.h"
 
-	/*
-	 * generic hardware linkage
-	 */
-	.irq			= ehci_irq,
-	.flags			= HCD_MEMORY | HCD_USB2,
-
-	/*
-	 * basic lifecycle operations
-	 */
-	.reset			= ehci_setup,
-	.start			= ehci_run,
-	.stop			= ehci_stop,
-	.shutdown		= ehci_shutdown,
+#define DRIVER_DESC "vt8500 On-Chip EHCI Host driver"
 
-	/*
-	 * managing i/o requests and associated device resources
-	 */
-	.urb_enqueue		= ehci_urb_enqueue,
-	.urb_dequeue		= ehci_urb_dequeue,
-	.endpoint_disable	= ehci_endpoint_disable,
-	.endpoint_reset		= ehci_endpoint_reset,
+static const char hcd_name[] = "ehci-vt8500";
 
-	/*
-	 * scheduling support
-	 */
-	.get_frame_number	= ehci_get_frame,
+static struct hc_driver __read_mostly vt8500_ehci_hc_driver;
 
-	/*
-	 * root hub support
-	 */
-	.hub_status_data	= ehci_hub_status_data,
-	.hub_control		= ehci_hub_control,
-	.bus_suspend		= ehci_bus_suspend,
-	.bus_resume		= ehci_bus_resume,
-	.relinquish_port	= ehci_relinquish_port,
-	.port_handed_over	= ehci_port_handed_over,
-
-	.clear_tt_buffer_complete	= ehci_clear_tt_buffer_complete,
+static const struct ehci_driver_overrides ehci_vt8500_overrides __initdata = {
+	.reset = ehci_setup,
 };
 
 static u64 vt8500_ehci_dma_mask = DMA_BIT_MASK(32);
@@ -140,11 +113,31 @@ static struct platform_driver vt8500_ehci_driver = {
 	.remove		= vt8500_ehci_drv_remove,
 	.shutdown	= usb_hcd_platform_shutdown,
 	.driver = {
-		.name	= "vt8500-ehci",
+		.name	= hcd_name,
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(vt8500_ehci_ids),
 	}
 };
 
+static int __init ehci_vt8500_init(void)
+{
+	if (usb_disabled())
+		return -ENODEV;
+
+	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+	ehci_init_driver(&vt8500_ehci_hc_driver, &ehci_vt8500_overrides);
+	return platform_driver_register(&vt8500_ehci_driver);
+}
+module_init(ehci_vt8500_init);
+
+static void __exit ehci_vt8500_cleanup(void)
+{
+	platform_driver_unregister(&vt8500_ehci_driver);
+}
+module_exit(ehci_vt8500_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_AUTHOR("Alexey Charkov");
+MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:vt8500-ehci");
 MODULE_DEVICE_TABLE(of, vt8500_ehci_ids);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH V2] usb: ehci: ehci-vt8500 bus glue as separate module
  2013-02-13  7:31 [PATCH V2] usb: ehci: ehci-vt8500 bus glue as separate module manjunath.goudar at linaro.org
  2013-02-13  7:31 ` [PATCH] USB: EHCI: make ehci-vt8500 a separate driver manjunath.goudar at linaro.org
@ 2013-02-13  9:58 ` Arnd Bergmann
  1 sibling, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2013-02-13  9:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 13 February 2013, manjunath.goudar at linaro.org wrote:
> From: Manjunath Goudar <manjunath.goudar@linaro.org>
> 
> Separate the vt8500 On-Chip host controller driver from ehci-hcd
> host code into its own static driver module

Hi Manjunath,

I see you are getting better with the patch submission procedure. I don't
know what you did different this time, but the patch actually ended
up coming out as a reply to the introductory mail this time. Whatever
you did there, please keep doing it.

There are a few things that you should improve though:

1. I did not see any replies to the review comments you got from the
other reviewers, which means either you did not reply, or you were
doing 'reply to author' rather than 'reply to all'. Please make it a
habit to reply to each reviewer, and keep the Cc list. You don't always
have to reply to each comment in one email, but when people spend time
to look at your patches and point out mistakes, they deserve to
know how you are addressing their comments.

2. For this specific patch, you sent out a single patch that is
supposed to be part of a longer series, and I expect that it would
not apply by itself. It's normally better wait until you have
received a number of comments to different patches, and then
send out the entire series again, in order to make it possible
to actually try them out. If you want to quickly send out a 
new version of a single patch out of a larger series, it's
better to reply with the new version to the old patch.

3. You really need to improve the text in the cover letter.
Remember that you cannot overcommmunicate here, and more information
about the patch can only help. In this case, the cover letter
has the exact same contents as the actual patch, which is not
helpful. Instead you could have put a text like

| This is the third version of the ehci cleanup series, which
| I based on Greg's usb-next branch from Feb 12. I addressed
| review comments from ...


	Arnd

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-02-13  9:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-13  7:31 [PATCH V2] usb: ehci: ehci-vt8500 bus glue as separate module manjunath.goudar at linaro.org
2013-02-13  7:31 ` [PATCH] USB: EHCI: make ehci-vt8500 a separate driver manjunath.goudar at linaro.org
2013-02-13  9:58 ` [PATCH V2] usb: ehci: ehci-vt8500 bus glue as separate module Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).