* [RFC PATCH 0/2] USB: OHCI: Start splitting up the driver
@ 2013-05-07 9:48 Manjunath Goudar
2013-05-07 9:48 ` [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module Manjunath Goudar
2013-05-07 9:48 ` [RFC PATCH 2/2] USB: OHCI: make ohci-pci a separate driver Manjunath Goudar
0 siblings, 2 replies; 8+ messages in thread
From: Manjunath Goudar @ 2013-05-07 9:48 UTC (permalink / raw)
To: linux-arm-kernel
This series of patches begins the process of splitting ohci-hcd up into
a core library module and independent pci driver modules.
Patch 1/2 prepares the way by exporting a few functions from ohci-hcd
and adding a new mechanism for platform-specific drivers to initialize
their hc_driver structures. This deserves to be done in the core
because almost all of the entries in these structures are pure
boilerplate -- practically none of the drivers need to override more
than three of the standard core values.
Patch 2/2 separate out ohci-pci into independent driver modules.
Manjunath Goudar (2):
USB: OHCI: prepare to make ohci-hcd a library module
USB: OHCI: make ohci-pci a separate driver
drivers/usb/host/Kconfig | 9 ++-
drivers/usb/host/Makefile | 3 +
drivers/usb/host/ohci-hcd.c | 142 +++++++++++++++++++++++++++++---------
drivers/usb/host/ohci-pci.c | 129 ++++++++++------------------------
drivers/usb/host/ohci-platform.c | 2 +-
drivers/usb/host/ohci-q.c | 13 ++++
drivers/usb/host/ohci.h | 30 +++++++-
7 files changed, 197 insertions(+), 131 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
2013-05-07 9:48 [RFC PATCH 0/2] USB: OHCI: Start splitting up the driver Manjunath Goudar
@ 2013-05-07 9:48 ` Manjunath Goudar
2013-05-07 9:48 ` [RFC PATCH 2/2] USB: OHCI: make ohci-pci a separate driver Manjunath Goudar
1 sibling, 0 replies; 8+ messages in thread
From: Manjunath Goudar @ 2013-05-07 9:48 UTC (permalink / raw)
To: linux-arm-kernel
This patch prepares ohci-hcd for being split up into a core
library and separate platform driver modules. A generic
ohci_hc_driver structure is created, containing all the "standard"
values, and a new mechanism is added whereby a driver module can
specify a set of overrides to those values. In addition the
ohci_restart(),ohci_suspend() and ohci_resume() routines need
to be EXPORTed for use by the drivers.
In V2:
-ohci_hcd_init(), ohci_run() and ohci_stop revert back to static routine.
-ohci_setup() and ohci_start() functions written to generic OHCI initialization
for all ohci bus glue.
Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg KH <greg@kroah.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: linux-usb at vger.kernel.org
---
drivers/usb/host/Makefile | 2 +
drivers/usb/host/ohci-hcd.c | 123 +++++++++++++++++++++++++++++++++-----
drivers/usb/host/ohci-pci.c | 2 +-
drivers/usb/host/ohci-platform.c | 2 +-
drivers/usb/host/ohci.h | 30 +++++++++-
5 files changed, 138 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 58c14c1..a38d76b 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -40,7 +40,9 @@ obj-$(CONFIG_USB_EHCI_TEGRA) +=ehci-tegra.o
obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.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
+
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 9e6de95..7922c61 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -79,13 +79,7 @@ static const char hcd_name [] = "ohci_hcd";
#include "pci-quirks.h"
static void ohci_dump (struct ohci_hcd *ohci, int verbose);
-static int ohci_init (struct ohci_hcd *ohci);
-static void ohci_stop (struct usb_hcd *hcd);
-
-#if defined(CONFIG_PM) || defined(CONFIG_PCI)
-static int ohci_restart (struct ohci_hcd *ohci);
-#endif
-
+static void ohci_stop(struct usb_hcd *hcd);
#ifdef CONFIG_PCI
static void sb800_prefetch(struct ohci_hcd *ohci, int on);
#else
@@ -520,7 +514,7 @@ done:
/* init memory, and kick BIOS/SMM off */
-static int ohci_init (struct ohci_hcd *ohci)
+int ohci_init(struct ohci_hcd *ohci)
{
int ret;
struct usb_hcd *hcd = ohci_to_hcd(ohci);
@@ -590,6 +584,7 @@ static int ohci_init (struct ohci_hcd *ohci)
return ret;
}
+EXPORT_SYMBOL_GPL(ohci_init);
/*-------------------------------------------------------------------------*/
@@ -597,11 +592,11 @@ static int ohci_init (struct ohci_hcd *ohci)
* resets USB and controller
* enable interrupts
*/
-static int ohci_run (struct ohci_hcd *ohci)
+static int ohci_run(struct usb_hcd *hcd)
{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
u32 mask, val;
int first = ohci->fminterval == 0;
- struct usb_hcd *hcd = ohci_to_hcd(ohci);
ohci->rh_state = OHCI_RH_HALTED;
@@ -767,7 +762,37 @@ retry:
return 0;
}
+/*------------------------------------------------------------------------*/
+
+/* ohci generic function for all OHCI bus glue */
+
+static int ohci_setup(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ int retval;
+
+ ohci->sbrn = HCD_USB11;
+
+ /* data structure init */
+ retval = ohci_init(ohci);
+ if (retval)
+ return retval;
+ ohci_usb_reset(ohci);
+ return 0;
+}
+static int ohci_start(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ int ret;
+ ret = ohci_run(hcd);
+ if (ret < 0) {
+ ohci_err(ohci, "can't start\n");
+ ohci_stop(hcd);
+ }
+ return ret;
+}
+/*-------------------------------------------------------------------------*/
/*-------------------------------------------------------------------------*/
/* an interrupt happens */
@@ -949,11 +974,12 @@ static void ohci_stop (struct usb_hcd *hcd)
#if defined(CONFIG_PM) || defined(CONFIG_PCI)
/* must not be called from interrupt context */
-static int ohci_restart (struct ohci_hcd *ohci)
+int ohci_restart(struct ohci_hcd *ohci)
{
int temp;
int i;
struct urb_priv *priv;
+ struct usb_hcd *hcd;
spin_lock_irq(&ohci->lock);
ohci->rh_state = OHCI_RH_HALTED;
@@ -1001,19 +1027,22 @@ static int ohci_restart (struct ohci_hcd *ohci)
ohci->ed_controltail = NULL;
ohci->ed_bulktail = NULL;
- if ((temp = ohci_run (ohci)) < 0) {
- ohci_err (ohci, "can't restart, %d\n", temp);
+ hcd = ohci_to_hcd(ohci);
+ temp = ohci_run(hcd);
+ if (temp < 0) {
+ ohci_err(ohci, "can't restart, %d\n", temp);
return temp;
}
ohci_dbg(ohci, "restart complete\n");
return 0;
}
+EXPORT_SYMBOL_GPL(ohci_restart);
#endif
#ifdef CONFIG_PM
-static int __maybe_unused ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
+int ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
unsigned long flags;
@@ -1031,9 +1060,9 @@ static int __maybe_unused ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
return 0;
}
+EXPORT_SYMBOL_GPL(ohci_suspend);
-
-static int __maybe_unused ohci_resume(struct usb_hcd *hcd, bool hibernated)
+int ohci_resume(struct usb_hcd *hcd, bool hibernated)
{
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
int port;
@@ -1081,11 +1110,73 @@ static int __maybe_unused ohci_resume(struct usb_hcd *hcd, bool hibernated)
return 0;
}
+EXPORT_SYMBOL_GPL(ohci_resume);
#endif
/*-------------------------------------------------------------------------*/
+/*
+ * Generic structure: This gets copied for platform drivers so that
+ * individual entries can be overridden as needed.
+ */
+
+static const struct hc_driver ohci_hc_driver = {
+ .description = hcd_name,
+ .product_desc = "OHCI Host Controller",
+ .hcd_priv_size = sizeof(struct ohci_hcd),
+
+ /*
+ * generic hardware linkage
+ */
+ .irq = ohci_irq,
+ .flags = HCD_MEMORY | HCD_USB11,
+
+ /*
+ * basic lifecycle operations
+ */
+ .reset = ohci_setup,
+ .start = ohci_start,
+ .stop = ohci_stop,
+ .shutdown = ohci_shutdown,
+
+ /*
+ * managing i/o requests and associated device resources
+ */
+ .urb_enqueue = ohci_urb_enqueue,
+ .urb_dequeue = ohci_urb_dequeue,
+ .endpoint_disable = ohci_endpoint_disable,
+
+ /*
+ * scheduling support
+ */
+ .get_frame_number = ohci_get_frame,
+
+ /*
+ * root hub support
+ */
+ .hub_status_data = ohci_hub_status_data,
+ .hub_control = ohci_hub_control,
+ .bus_suspend = ohci_bus_suspend,
+ .bus_resume = ohci_bus_resume,
+ .start_port_reset = ohci_start_port_reset,
+};
+
+void ohci_init_driver(struct hc_driver *drv,
+ const struct ohci_driver_overrides *over)
+{
+ /* Copy the generic table to drv and then apply the overrides */
+ *drv = ohci_hc_driver;
+
+ drv->product_desc = over->product_desc;
+ drv->hcd_priv_size += over->extra_priv_size;
+ if (over->reset)
+ drv->reset = over->reset;
+}
+EXPORT_SYMBOL_GPL(ohci_init_driver);
+
+/*-------------------------------------------------------------------------*/
+
MODULE_AUTHOR (DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE ("GPL");
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 951514e..0b34b59 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -288,7 +288,7 @@ static int ohci_pci_start (struct usb_hcd *hcd)
}
#endif /* CONFIG_PM */
- ret = ohci_run (ohci);
+ ret = ohci_run(hcd);
if (ret < 0) {
ohci_err (ohci, "can't start\n");
ohci_stop (hcd);
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index c3e7287..545c657 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -46,7 +46,7 @@ static int ohci_platform_start(struct usb_hcd *hcd)
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
int err;
- err = ohci_run(ohci);
+ err = ohci_run(hcd);
if (err < 0) {
ohci_err(ohci, "can't start\n");
ohci_stop(hcd);
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index d329914..455e9b1 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -357,7 +357,6 @@ struct ohci_hcd {
* I/O memory used to communicate with the HC (dma-consistent)
*/
struct ohci_regs __iomem *regs;
-
/*
* main memory used to communicate with the HC (dma-consistent).
* hcd adds to schedule for a live hc any time, but removals finish
@@ -373,7 +372,6 @@ struct ohci_hcd {
struct ed *periodic [NUM_INTS]; /* shadow int_table */
void (*start_hnp)(struct ohci_hcd *ohci);
-
/*
* memory management for queue data structures
*/
@@ -392,7 +390,7 @@ struct ohci_hcd {
unsigned long next_statechange; /* suspend/resume */
u32 fminterval; /* saved register */
unsigned autostop:1; /* rh auto stopping/stopped */
-
+ u8 sbrn;
unsigned long flags; /* for HC bugs */
#define OHCI_QUIRK_AMD756 0x01 /* erratum #4 */
#define OHCI_QUIRK_SUPERIO 0x02 /* natsemi */
@@ -718,3 +716,29 @@ static inline u32 roothub_status (struct ohci_hcd *hc)
{ return ohci_readl (hc, &hc->regs->roothub.status); }
static inline u32 roothub_portstatus (struct ohci_hcd *hc, int i)
{ return read_roothub (hc, portstatus [i], 0xffe0fce0); }
+
+/* Declarations of things exported for use by ohci platform drivers */
+
+struct ohci_driver_overrides {
+ const char *product_desc;
+ size_t extra_priv_size;
+ int (*reset)(struct usb_hcd *hcd);
+};
+
+extern void ohci_init_driver(struct hc_driver *drv,
+ const struct ohci_driver_overrides *over);
+extern int ohci_init(struct ohci_hcd *ohci);
+extern int ohci_restart(struct ohci_hcd *ohci);
+#ifdef CONFIG_PM
+extern int ohci_suspend(struct usb_hcd *hcd, bool do_wakeup);
+extern int ohci_resume(struct usb_hcd *hcd, bool hibernated);
+#else
+static inline int ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
+{
+ return 0;
+}
+static inline int ohci_resume(struct usb_hcd *hcd, bool hibernated)
+{
+ return 0;
+}
+#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 2/2] USB: OHCI: make ohci-pci a separate driver
2013-05-07 9:48 [RFC PATCH 0/2] USB: OHCI: Start splitting up the driver Manjunath Goudar
2013-05-07 9:48 ` [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module Manjunath Goudar
@ 2013-05-07 9:48 ` Manjunath Goudar
1 sibling, 0 replies; 8+ messages in thread
From: Manjunath Goudar @ 2013-05-07 9:48 UTC (permalink / raw)
To: linux-arm-kernel
This patch splits the PCI portion of ohci-hcd out into its
own separate driver module, called ohci-pci. Consistently with the
current practice, the decision whether to build this module is not
user-configurable. If OHCI_PCI are enabled then the module will
be built, always.
V2:
- few specific content of pci related code in ohci_pci_start function has been moved to ohci_pci_reset
and rest of the generic code is written in ohci_start of ohci-hcd.c file.
Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg KH <greg@kroah.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: linux-usb at vger.kernel.org
---
drivers/usb/host/Kconfig | 9 ++-
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ohci-hcd.c | 19 +------
drivers/usb/host/ohci-pci.c | 129 +++++++++++++------------------------------
drivers/usb/host/ohci-q.c | 13 +++++
5 files changed, 60 insertions(+), 111 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 7a1a248..3620ecce 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -333,8 +333,13 @@ config USB_ISP1362_HCD
To compile this driver as a module, choose M here: the
module will be called isp1362-hcd.
+config USB_OHCI_PCI
+ tristate
+ depends on USB_OHCI_HCD && PCI
+ default y
+
config USB_OHCI_HCD
- tristate "OHCI HCD support"
+ tristate "OHCI HCD (USB 1.1) support"
depends on USB && USB_ARCH_HAS_OHCI
select ISP1301_OMAP if MACH_OMAP_H2 || MACH_OMAP_H3
select USB_OTG_UTILS if ARCH_OMAP
@@ -402,7 +407,7 @@ config USB_OHCI_HCD_PPC_OF
default USB_OHCI_HCD_PPC_OF_BE || USB_OHCI_HCD_PPC_OF_LE
config USB_OHCI_HCD_PCI
- bool "OHCI support for PCI-bus USB controllers"
+ tristate "OHCI support for PCI-bus USB controllers"
depends on USB_OHCI_HCD && PCI && (STB03xxx || PPC_MPC52xx || USB_OHCI_HCD_PPC_OF)
default y
select USB_OHCI_LITTLE_ENDIAN
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index a38d76b..a20a8d9 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
obj-$(CONFIG_USB_ISP1362_HCD) += isp1362-hcd.o
obj-$(CONFIG_USB_OHCI_HCD) += ohci-hcd.o
+obj-$(CONFIG_USB_OHCI_PCI) += ohci-pci.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 7922c61..2e62e7d 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -95,7 +95,6 @@ static inline void sb800_prefetch(struct ohci_hcd *ohci, int on)
#include "ohci-mem.c"
#include "ohci-q.c"
-
/*
* On architectures with edge-triggered interrupts we must never return
* IRQ_NONE.
@@ -585,7 +584,6 @@ int ohci_init(struct ohci_hcd *ohci)
return ret;
}
EXPORT_SYMBOL_GPL(ohci_init);
-
/*-------------------------------------------------------------------------*/
/* Start an OHCI controller, set the BUS operational
@@ -1181,11 +1179,6 @@ MODULE_AUTHOR (DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE ("GPL");
-#ifdef CONFIG_PCI
-#include "ohci-pci.c"
-#define PCI_DRIVER ohci_pci_driver
-#endif
-
#if defined(CONFIG_ARCH_SA1100) && defined(CONFIG_SA1111)
#include "ohci-sa1111.c"
#define SA1111_DRIVER ohci_hcd_sa1111_driver
@@ -1281,7 +1274,7 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVER ohci_platform_driver
#endif
-#if !defined(PCI_DRIVER) && \
+#if !IS_ENABLED(CONFIG_USB_OHCI_PCI) && \
!defined(PLATFORM_DRIVER) && \
!defined(OMAP1_PLATFORM_DRIVER) && \
!defined(OMAP3_PLATFORM_DRIVER) && \
@@ -1356,12 +1349,6 @@ static int __init ohci_hcd_mod_init(void)
goto error_sa1111;
#endif
-#ifdef PCI_DRIVER
- retval = pci_register_driver(&PCI_DRIVER);
- if (retval < 0)
- goto error_pci;
-#endif
-
#ifdef SM501_OHCI_DRIVER
retval = platform_driver_register(&SM501_OHCI_DRIVER);
if (retval < 0)
@@ -1455,10 +1442,6 @@ static int __init ohci_hcd_mod_init(void)
platform_driver_unregister(&SM501_OHCI_DRIVER);
error_sm501:
#endif
-#ifdef PCI_DRIVER
- pci_unregister_driver(&PCI_DRIVER);
- error_pci:
-#endif
#ifdef SA1111_DRIVER
sa1111_driver_unregister(&SA1111_DRIVER);
error_sa1111:
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 0b34b59..20f6187 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -14,12 +14,19 @@
* This file is licenced under the GPL.
*/
-#ifndef CONFIG_PCI
-#error "This file is PCI bus glue. CONFIG_PCI must be defined."
-#endif
-
-#include <linux/pci.h>
#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#include "ohci.h"
+#include "pci-quirks.h"
+
+#define DRIVER_DESC "OHCI PCI platform driver"
+
+static const char hcd_name[] = "ohci-pci";
/*-------------------------------------------------------------------------*/
@@ -175,19 +182,6 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
return 0;
}
-static void sb800_prefetch(struct ohci_hcd *ohci, int on)
-{
- struct pci_dev *pdev;
- u16 misc;
-
- pdev = to_pci_dev(ohci_to_hcd(ohci)->self.controller);
- pci_read_config_word(pdev, 0x50, &misc);
- if (on == 0)
- pci_write_config_word(pdev, 0x50, misc & 0xfcff);
- else
- pci_write_config_word(pdev, 0x50, misc | 0x0300);
-}
-
/* List of quirks for OHCI */
static const struct pci_device_id ohci_pci_quirks[] = {
{
@@ -261,95 +255,27 @@ static int ohci_pci_reset (struct usb_hcd *hcd)
quirk = (void *)quirk_id->driver_data;
ret = quirk(hcd);
}
- }
- if (ret == 0) {
- ohci_hcd_init (ohci);
- return ohci_init (ohci);
- }
- return ret;
-}
-
-
-static int ohci_pci_start (struct usb_hcd *hcd)
-{
- struct ohci_hcd *ohci = hcd_to_ohci (hcd);
- int ret;
-
#ifdef CONFIG_PM /* avoid warnings about unused pdev */
- if (hcd->self.controller) {
- struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
-
/* RWC may not be set for add-in PCI cards, since boot
* firmware probably ignored them. This transfers PCI
* PM wakeup capabilities.
*/
if (device_can_wakeup(&pdev->dev))
ohci->hc_control |= OHCI_CTRL_RWC;
- }
#endif /* CONFIG_PM */
-
- ret = ohci_run(hcd);
- if (ret < 0) {
- ohci_err (ohci, "can't start\n");
- ohci_stop (hcd);
}
+ if (ret == 0)
+ return ohci_init(ohci);
return ret;
}
+static struct hc_driver __read_mostly ohci_pci_hc_driver;
-/*-------------------------------------------------------------------------*/
-
-static const struct hc_driver ohci_pci_hc_driver = {
- .description = hcd_name,
- .product_desc = "OHCI Host Controller",
- .hcd_priv_size = sizeof(struct ohci_hcd),
-
- /*
- * generic hardware linkage
- */
- .irq = ohci_irq,
- .flags = HCD_MEMORY | HCD_USB11,
-
- /*
- * basic lifecycle operations
- */
+static const struct ohci_driver_overrides overrides = {
+ .product_desc = "OHCI PCI host controller",
.reset = ohci_pci_reset,
- .start = ohci_pci_start,
- .stop = ohci_stop,
- .shutdown = ohci_shutdown,
-
-#ifdef CONFIG_PM
- .pci_suspend = ohci_suspend,
- .pci_resume = ohci_resume,
-#endif
-
- /*
- * managing i/o requests and associated device resources
- */
- .urb_enqueue = ohci_urb_enqueue,
- .urb_dequeue = ohci_urb_dequeue,
- .endpoint_disable = ohci_endpoint_disable,
-
- /*
- * scheduling support
- */
- .get_frame_number = ohci_get_frame,
-
- /*
- * root hub support
- */
- .hub_status_data = ohci_hub_status_data,
- .hub_control = ohci_hub_control,
-#ifdef CONFIG_PM
- .bus_suspend = ohci_bus_suspend,
- .bus_resume = ohci_bus_resume,
-#endif
- .start_port_reset = ohci_start_port_reset,
};
-/*-------------------------------------------------------------------------*/
-
-
static const struct pci_device_id pci_ids [] = { {
/* handle any USB OHCI controller */
PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_USB_OHCI, ~0),
@@ -377,3 +303,24 @@ static struct pci_driver ohci_pci_driver = {
},
#endif
};
+
+static int __init ohci_pci_init(void)
+{
+ if (usb_disabled())
+ return -ENODEV;
+
+ pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+
+ ohci_init_driver(&ohci_pci_hc_driver, &overrides);
+ return pci_register_driver(&ohci_pci_driver);
+}
+module_init(ohci_pci_init);
+
+static void __exit ohci_pci_cleanup(void)
+{
+ pci_unregister_driver(&ohci_pci_driver);
+}
+module_exit(ohci_pci_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index 88731b7..a7e8139 100644
--- a/drivers/usb/host/ohci-q.c
+++ b/drivers/usb/host/ohci-q.c
@@ -29,6 +29,19 @@ static void urb_free_priv (struct ohci_hcd *hc, urb_priv_t *urb_priv)
kfree (urb_priv);
}
+static void sb800_prefetch(struct ohci_hcd *ohci, int on)
+{
+ struct pci_dev *pdev;
+ u16 misc;
+
+ pdev = to_pci_dev(ohci_to_hcd(ohci)->self.controller);
+ pci_read_config_word(pdev, 0x50, &misc);
+ if (on == 0)
+ pci_write_config_word(pdev, 0x50, misc & 0xfcff);
+ else
+ pci_write_config_word(pdev, 0x50, misc | 0x0300);
+}
+
/*-------------------------------------------------------------------------*/
/*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
2013-05-07 9:50 ` [RFC PATCH 0/2] USB: OHCI: Start splitting up the driver Manjunath Goudar
@ 2013-05-07 9:50 ` Manjunath Goudar
2013-05-07 15:15 ` Alan Stern
0 siblings, 1 reply; 8+ messages in thread
From: Manjunath Goudar @ 2013-05-07 9:50 UTC (permalink / raw)
To: linux-arm-kernel
This patch prepares ohci-hcd for being split up into a core
library and separate platform driver modules. A generic
ohci_hc_driver structure is created, containing all the "standard"
values, and a new mechanism is added whereby a driver module can
specify a set of overrides to those values. In addition the
ohci_restart(),ohci_suspend() and ohci_resume() routines need
to be EXPORTed for use by the drivers.
In V2:
-ohci_hcd_init(), ohci_run() and ohci_stop revert back to static routine.
-ohci_setup() and ohci_start() functions written to generic OHCI initialization
for all ohci bus glue.
Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg KH <greg@kroah.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: linux-usb at vger.kernel.org
---
drivers/usb/host/Makefile | 2 +
drivers/usb/host/ohci-hcd.c | 123 +++++++++++++++++++++++++++++++++-----
drivers/usb/host/ohci-pci.c | 2 +-
drivers/usb/host/ohci-platform.c | 2 +-
drivers/usb/host/ohci.h | 30 +++++++++-
5 files changed, 138 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 58c14c1..a38d76b 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -40,7 +40,9 @@ obj-$(CONFIG_USB_EHCI_TEGRA) +=ehci-tegra.o
obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.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
+
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 9e6de95..7922c61 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -79,13 +79,7 @@ static const char hcd_name [] = "ohci_hcd";
#include "pci-quirks.h"
static void ohci_dump (struct ohci_hcd *ohci, int verbose);
-static int ohci_init (struct ohci_hcd *ohci);
-static void ohci_stop (struct usb_hcd *hcd);
-
-#if defined(CONFIG_PM) || defined(CONFIG_PCI)
-static int ohci_restart (struct ohci_hcd *ohci);
-#endif
-
+static void ohci_stop(struct usb_hcd *hcd);
#ifdef CONFIG_PCI
static void sb800_prefetch(struct ohci_hcd *ohci, int on);
#else
@@ -520,7 +514,7 @@ done:
/* init memory, and kick BIOS/SMM off */
-static int ohci_init (struct ohci_hcd *ohci)
+int ohci_init(struct ohci_hcd *ohci)
{
int ret;
struct usb_hcd *hcd = ohci_to_hcd(ohci);
@@ -590,6 +584,7 @@ static int ohci_init (struct ohci_hcd *ohci)
return ret;
}
+EXPORT_SYMBOL_GPL(ohci_init);
/*-------------------------------------------------------------------------*/
@@ -597,11 +592,11 @@ static int ohci_init (struct ohci_hcd *ohci)
* resets USB and controller
* enable interrupts
*/
-static int ohci_run (struct ohci_hcd *ohci)
+static int ohci_run(struct usb_hcd *hcd)
{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
u32 mask, val;
int first = ohci->fminterval == 0;
- struct usb_hcd *hcd = ohci_to_hcd(ohci);
ohci->rh_state = OHCI_RH_HALTED;
@@ -767,7 +762,37 @@ retry:
return 0;
}
+/*------------------------------------------------------------------------*/
+
+/* ohci generic function for all OHCI bus glue */
+
+static int ohci_setup(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ int retval;
+
+ ohci->sbrn = HCD_USB11;
+
+ /* data structure init */
+ retval = ohci_init(ohci);
+ if (retval)
+ return retval;
+ ohci_usb_reset(ohci);
+ return 0;
+}
+static int ohci_start(struct usb_hcd *hcd)
+{
+ struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ int ret;
+ ret = ohci_run(hcd);
+ if (ret < 0) {
+ ohci_err(ohci, "can't start\n");
+ ohci_stop(hcd);
+ }
+ return ret;
+}
+/*-------------------------------------------------------------------------*/
/*-------------------------------------------------------------------------*/
/* an interrupt happens */
@@ -949,11 +974,12 @@ static void ohci_stop (struct usb_hcd *hcd)
#if defined(CONFIG_PM) || defined(CONFIG_PCI)
/* must not be called from interrupt context */
-static int ohci_restart (struct ohci_hcd *ohci)
+int ohci_restart(struct ohci_hcd *ohci)
{
int temp;
int i;
struct urb_priv *priv;
+ struct usb_hcd *hcd;
spin_lock_irq(&ohci->lock);
ohci->rh_state = OHCI_RH_HALTED;
@@ -1001,19 +1027,22 @@ static int ohci_restart (struct ohci_hcd *ohci)
ohci->ed_controltail = NULL;
ohci->ed_bulktail = NULL;
- if ((temp = ohci_run (ohci)) < 0) {
- ohci_err (ohci, "can't restart, %d\n", temp);
+ hcd = ohci_to_hcd(ohci);
+ temp = ohci_run(hcd);
+ if (temp < 0) {
+ ohci_err(ohci, "can't restart, %d\n", temp);
return temp;
}
ohci_dbg(ohci, "restart complete\n");
return 0;
}
+EXPORT_SYMBOL_GPL(ohci_restart);
#endif
#ifdef CONFIG_PM
-static int __maybe_unused ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
+int ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
unsigned long flags;
@@ -1031,9 +1060,9 @@ static int __maybe_unused ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
return 0;
}
+EXPORT_SYMBOL_GPL(ohci_suspend);
-
-static int __maybe_unused ohci_resume(struct usb_hcd *hcd, bool hibernated)
+int ohci_resume(struct usb_hcd *hcd, bool hibernated)
{
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
int port;
@@ -1081,11 +1110,73 @@ static int __maybe_unused ohci_resume(struct usb_hcd *hcd, bool hibernated)
return 0;
}
+EXPORT_SYMBOL_GPL(ohci_resume);
#endif
/*-------------------------------------------------------------------------*/
+/*
+ * Generic structure: This gets copied for platform drivers so that
+ * individual entries can be overridden as needed.
+ */
+
+static const struct hc_driver ohci_hc_driver = {
+ .description = hcd_name,
+ .product_desc = "OHCI Host Controller",
+ .hcd_priv_size = sizeof(struct ohci_hcd),
+
+ /*
+ * generic hardware linkage
+ */
+ .irq = ohci_irq,
+ .flags = HCD_MEMORY | HCD_USB11,
+
+ /*
+ * basic lifecycle operations
+ */
+ .reset = ohci_setup,
+ .start = ohci_start,
+ .stop = ohci_stop,
+ .shutdown = ohci_shutdown,
+
+ /*
+ * managing i/o requests and associated device resources
+ */
+ .urb_enqueue = ohci_urb_enqueue,
+ .urb_dequeue = ohci_urb_dequeue,
+ .endpoint_disable = ohci_endpoint_disable,
+
+ /*
+ * scheduling support
+ */
+ .get_frame_number = ohci_get_frame,
+
+ /*
+ * root hub support
+ */
+ .hub_status_data = ohci_hub_status_data,
+ .hub_control = ohci_hub_control,
+ .bus_suspend = ohci_bus_suspend,
+ .bus_resume = ohci_bus_resume,
+ .start_port_reset = ohci_start_port_reset,
+};
+
+void ohci_init_driver(struct hc_driver *drv,
+ const struct ohci_driver_overrides *over)
+{
+ /* Copy the generic table to drv and then apply the overrides */
+ *drv = ohci_hc_driver;
+
+ drv->product_desc = over->product_desc;
+ drv->hcd_priv_size += over->extra_priv_size;
+ if (over->reset)
+ drv->reset = over->reset;
+}
+EXPORT_SYMBOL_GPL(ohci_init_driver);
+
+/*-------------------------------------------------------------------------*/
+
MODULE_AUTHOR (DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE ("GPL");
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 951514e..0b34b59 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -288,7 +288,7 @@ static int ohci_pci_start (struct usb_hcd *hcd)
}
#endif /* CONFIG_PM */
- ret = ohci_run (ohci);
+ ret = ohci_run(hcd);
if (ret < 0) {
ohci_err (ohci, "can't start\n");
ohci_stop (hcd);
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index c3e7287..545c657 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -46,7 +46,7 @@ static int ohci_platform_start(struct usb_hcd *hcd)
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
int err;
- err = ohci_run(ohci);
+ err = ohci_run(hcd);
if (err < 0) {
ohci_err(ohci, "can't start\n");
ohci_stop(hcd);
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index d329914..455e9b1 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -357,7 +357,6 @@ struct ohci_hcd {
* I/O memory used to communicate with the HC (dma-consistent)
*/
struct ohci_regs __iomem *regs;
-
/*
* main memory used to communicate with the HC (dma-consistent).
* hcd adds to schedule for a live hc any time, but removals finish
@@ -373,7 +372,6 @@ struct ohci_hcd {
struct ed *periodic [NUM_INTS]; /* shadow int_table */
void (*start_hnp)(struct ohci_hcd *ohci);
-
/*
* memory management for queue data structures
*/
@@ -392,7 +390,7 @@ struct ohci_hcd {
unsigned long next_statechange; /* suspend/resume */
u32 fminterval; /* saved register */
unsigned autostop:1; /* rh auto stopping/stopped */
-
+ u8 sbrn;
unsigned long flags; /* for HC bugs */
#define OHCI_QUIRK_AMD756 0x01 /* erratum #4 */
#define OHCI_QUIRK_SUPERIO 0x02 /* natsemi */
@@ -718,3 +716,29 @@ static inline u32 roothub_status (struct ohci_hcd *hc)
{ return ohci_readl (hc, &hc->regs->roothub.status); }
static inline u32 roothub_portstatus (struct ohci_hcd *hc, int i)
{ return read_roothub (hc, portstatus [i], 0xffe0fce0); }
+
+/* Declarations of things exported for use by ohci platform drivers */
+
+struct ohci_driver_overrides {
+ const char *product_desc;
+ size_t extra_priv_size;
+ int (*reset)(struct usb_hcd *hcd);
+};
+
+extern void ohci_init_driver(struct hc_driver *drv,
+ const struct ohci_driver_overrides *over);
+extern int ohci_init(struct ohci_hcd *ohci);
+extern int ohci_restart(struct ohci_hcd *ohci);
+#ifdef CONFIG_PM
+extern int ohci_suspend(struct usb_hcd *hcd, bool do_wakeup);
+extern int ohci_resume(struct usb_hcd *hcd, bool hibernated);
+#else
+static inline int ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
+{
+ return 0;
+}
+static inline int ohci_resume(struct usb_hcd *hcd, bool hibernated)
+{
+ return 0;
+}
+#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
2013-05-07 9:50 ` [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module Manjunath Goudar
@ 2013-05-07 15:15 ` Alan Stern
0 siblings, 0 replies; 8+ messages in thread
From: Alan Stern @ 2013-05-07 15:15 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 7 May 2013, Manjunath Goudar wrote:
> This patch prepares ohci-hcd for being split up into a core
> library and separate platform driver modules. A generic
> ohci_hc_driver structure is created, containing all the "standard"
> values, and a new mechanism is added whereby a driver module can
> specify a set of overrides to those values. In addition the
> ohci_restart(),ohci_suspend() and ohci_resume() routines need
> to be EXPORTed for use by the drivers.
This patch still has several problems. For example, the description
doesn't mention the fact that ohci_init() was EXPORTed.
In fact, why was ohci_init() EXPORTed? I don't see any reason for
this. ohci_pci.c doesn't need to call it; just insert a call to
ohci_init() at the beginning of ohci_restart().
> In V2:
> -ohci_hcd_init(), ohci_run() and ohci_stop revert back to static routine.
They don't "revert" since they have never been non-static. You should
say something more like "ohci_hcd_init(), ohci_run(), and ohci_stop()
are not made non-static."
> -ohci_setup() and ohci_start() functions written to generic OHCI initialization
> for all ohci bus glue.
Fix the grammar in that sentence. And you should mention these new
functions in the main part of the patch description, not just down here.
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 58c14c1..a38d76b 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -40,7 +40,9 @@ obj-$(CONFIG_USB_EHCI_TEGRA) +=ehci-tegra.o
> obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.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
> +
You do not need to add these blank lines in this patch. If you want,
you can add them in the ohci-pci patch.
> obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
> obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
> obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index 9e6de95..7922c61 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -79,13 +79,7 @@ static const char hcd_name [] = "ohci_hcd";
> #include "pci-quirks.h"
>
> static void ohci_dump (struct ohci_hcd *ohci, int verbose);
> -static int ohci_init (struct ohci_hcd *ohci);
> -static void ohci_stop (struct usb_hcd *hcd);
I thought ohci_stop() wasn't going to be changed in this patch. Why
was this line updated?
> -
> -#if defined(CONFIG_PM) || defined(CONFIG_PCI)
> -static int ohci_restart (struct ohci_hcd *ohci);
> -#endif
> -
> +static void ohci_stop(struct usb_hcd *hcd);
> #ifdef CONFIG_PCI
> static void sb800_prefetch(struct ohci_hcd *ohci, int on);
> #else
> @@ -520,7 +514,7 @@ done:
>
> /* init memory, and kick BIOS/SMM off */
>
> -static int ohci_init (struct ohci_hcd *ohci)
> +int ohci_init(struct ohci_hcd *ohci)
> {
> int ret;
> struct usb_hcd *hcd = ohci_to_hcd(ohci);
> @@ -590,6 +584,7 @@ static int ohci_init (struct ohci_hcd *ohci)
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(ohci_init);
>
> /*-------------------------------------------------------------------------*/
>
> @@ -597,11 +592,11 @@ static int ohci_init (struct ohci_hcd *ohci)
> * resets USB and controller
> * enable interrupts
> */
> -static int ohci_run (struct ohci_hcd *ohci)
> +static int ohci_run(struct usb_hcd *hcd)
Why did you change the signature of this function? By doing so, you
just broke all the bus glue files. (Except for ohci_pci and
ohci_platform, which explicitly get fixed below.)
Since this function remains static, there's no reason to change it.
> {
> + struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> u32 mask, val;
> int first = ohci->fminterval == 0;
> - struct usb_hcd *hcd = ohci_to_hcd(ohci);
>
> ohci->rh_state = OHCI_RH_HALTED;
>
> @@ -767,7 +762,37 @@ retry:
>
> return 0;
> }
> +/*------------------------------------------------------------------------*/
> +
> +/* ohci generic function for all OHCI bus glue */
> +
> +static int ohci_setup(struct usb_hcd *hcd)
> +{
> + struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> + int retval;
> +
> + ohci->sbrn = HCD_USB11;
What is this doing here? Why did you add this "sbrn" member to struct
ohci_hcd?
> +
> + /* data structure init */
> + retval = ohci_init(ohci);
> + if (retval)
> + return retval;
> + ohci_usb_reset(ohci);
Why is this call here? Doesn't ohci_init() already call
ohci_usb_reset()?
> + return 0;
> +}
>
> +static int ohci_start(struct usb_hcd *hcd)
> +{
> + struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> + int ret;
There should be a blank line between the declarations and the
executable statements.
> + ret = ohci_run(hcd);
> + if (ret < 0) {
> + ohci_err(ohci, "can't start\n");
> + ohci_stop(hcd);
> + }
> + return ret;
> +}
> +/*-------------------------------------------------------------------------*/
> /*-------------------------------------------------------------------------*/
You should not duplicate this comment line.
> diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
> index 951514e..0b34b59 100644
> --- a/drivers/usb/host/ohci-pci.c
> +++ b/drivers/usb/host/ohci-pci.c
> @@ -288,7 +288,7 @@ static int ohci_pci_start (struct usb_hcd *hcd)
> }
> #endif /* CONFIG_PM */
>
> - ret = ohci_run (ohci);
> + ret = ohci_run(hcd);
If you hadn't changed ohci_run(), this wouldn't be needed.
> if (ret < 0) {
> ohci_err (ohci, "can't start\n");
> ohci_stop (hcd);
> diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
> index c3e7287..545c657 100644
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -46,7 +46,7 @@ static int ohci_platform_start(struct usb_hcd *hcd)
> struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> int err;
>
> - err = ohci_run(ohci);
> + err = ohci_run(hcd);
Likewise here.
> if (err < 0) {
> ohci_err(ohci, "can't start\n");
> ohci_stop(hcd);
> diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
> index d329914..455e9b1 100644
> --- a/drivers/usb/host/ohci.h
> +++ b/drivers/usb/host/ohci.h
> @@ -357,7 +357,6 @@ struct ohci_hcd {
> * I/O memory used to communicate with the HC (dma-consistent)
> */
> struct ohci_regs __iomem *regs;
> -
You shouldn't make unrelated changes, like removing this blank line or
the one below.
> /*
> * main memory used to communicate with the HC (dma-consistent).
> * hcd adds to schedule for a live hc any time, but removals finish
> @@ -373,7 +372,6 @@ struct ohci_hcd {
> struct ed *periodic [NUM_INTS]; /* shadow int_table */
>
> void (*start_hnp)(struct ohci_hcd *ohci);
> -
> /*
> * memory management for queue data structures
> */
> @@ -392,7 +390,7 @@ struct ohci_hcd {
> unsigned long next_statechange; /* suspend/resume */
> u32 fminterval; /* saved register */
> unsigned autostop:1; /* rh auto stopping/stopped */
> -
> + u8 sbrn;
> unsigned long flags; /* for HC bugs */
> #define OHCI_QUIRK_AMD756 0x01 /* erratum #4 */
> #define OHCI_QUIRK_SUPERIO 0x02 /* natsemi */
> @@ -718,3 +716,29 @@ static inline u32 roothub_status (struct ohci_hcd *hc)
> { return ohci_readl (hc, &hc->regs->roothub.status); }
> static inline u32 roothub_portstatus (struct ohci_hcd *hc, int i)
> { return read_roothub (hc, portstatus [i], 0xffe0fce0); }
> +
> +/* Declarations of things exported for use by ohci platform drivers */
> +
> +struct ohci_driver_overrides {
> + const char *product_desc;
> + size_t extra_priv_size;
> + int (*reset)(struct usb_hcd *hcd);
> +};
> +
> +extern void ohci_init_driver(struct hc_driver *drv,
> + const struct ohci_driver_overrides *over);
> +extern int ohci_init(struct ohci_hcd *ohci);
> +extern int ohci_restart(struct ohci_hcd *ohci);
> +#ifdef CONFIG_PM
> +extern int ohci_suspend(struct usb_hcd *hcd, bool do_wakeup);
> +extern int ohci_resume(struct usb_hcd *hcd, bool hibernated);
> +#else
> +static inline int ohci_suspend(struct usb_hcd *hcd, bool do_wakeup)
> +{
> + return 0;
> +}
> +static inline int ohci_resume(struct usb_hcd *hcd, bool hibernated)
> +{
> + return 0;
> +}
> +#endif
The #else part of this isn't needed, and I doubt very much that it
would work correctly if it was needed.
Alan Stern
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
[not found] <CAJFYCKGSctw1_nXoETYwyVkgAncHyBb8dmZ9aBV-av=8S0rw4A@mail.gmail.com>
@ 2013-05-09 15:54 ` Alan Stern
2013-05-09 19:25 ` Arnd Bergmann
0 siblings, 1 reply; 8+ messages in thread
From: Alan Stern @ 2013-05-09 15:54 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 9 May 2013, Manjunath Goudar wrote:
> > > @@ -767,7 +762,37 @@ retry:
> > >
> > > return 0;
> > > }
> > >
> > +/*------------------------------------------------------------------------*/
> > > +
> > > +/* ohci generic function for all OHCI bus glue */
> > > +
> > > +static int ohci_setup(struct usb_hcd *hcd)
> > > +{
> > > + struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> > > + int retval;
> > > +
> > > + ohci->sbrn = HCD_USB11;
> >
> > What is this doing here? Why did you add this "sbrn" member to struct
> > ohci_hcd?
> >
> > not required so reverted.
>
> > > +
> > > + /* data structure init */
> > > + retval = ohci_init(ohci);
> > > + if (retval)
> > > + return retval;
> > > + ohci_usb_reset(ohci);
> >
> > Why is this call here? Doesn't ohci_init() already call
> > ohci_usb_reset()?
> >
> >
> ohci_init is not called in ohci_usb_reset() its called in ohci_restart() so
> I think ohci_init needed.
It sounds like you did not understand what I wrote.
ohci_setup does not need to call ohci_usb_reset. ohci_setup calls
ohci_init, and ohci_init calls ohci_usb_reset.
> > The #else part of this isn't needed, and I doubt very much that it
> > would work correctly if it was needed.
> >
> > As Arnd suggestion,so I written else part.
The existing code doesn't have a #else part, and I don't think you need
to add one.
Alan Stern
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
2013-05-09 15:54 ` Alan Stern
@ 2013-05-09 19:25 ` Arnd Bergmann
2013-05-09 19:36 ` Alan Stern
0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2013-05-09 19:25 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 09 May 2013, Alan Stern wrote:
> > > The #else part of this isn't needed, and I doubt very much that it
> > > would work correctly if it was needed.
> >
> > As Arnd suggestion,so I written else part.
>
> The existing code doesn't have a #else part, and I don't think you need
> to add one.
I had commented in an earlier (Linaro internal) review that function
declarations should not be hidden inside of an #ifdef unless there
is an #else clause defining an alternative.
I did not want to suggest with that adding an #else for the fun of it,
of course it should only be there if actually needed.
Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module
2013-05-09 19:25 ` Arnd Bergmann
@ 2013-05-09 19:36 ` Alan Stern
0 siblings, 0 replies; 8+ messages in thread
From: Alan Stern @ 2013-05-09 19:36 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 9 May 2013, Arnd Bergmann wrote:
> On Thursday 09 May 2013, Alan Stern wrote:
> > > > The #else part of this isn't needed, and I doubt very much that it
> > > > would work correctly if it was needed.
> > >
> > > As Arnd suggestion,so I written else part.
> >
> > The existing code doesn't have a #else part, and I don't think you need
> > to add one.
>
> I had commented in an earlier (Linaro internal) review that function
> declarations should not be hidden inside of an #ifdef unless there
> is an #else clause defining an alternative.
>
> I did not want to suggest with that adding an #else for the fun of it,
> of course it should only be there if actually needed.
Well, empty inline function definitions don't have any runtime cost, so
there's nothing really wrong with it. But it does give the impression
that it's okay to try calling these routines when CONFIG_PM isn't
enabled, which isn't true. Either the result won't be what you
expected, or else you shouldn't be calling them in the first place.
Alan Stern
PS: If the proposed __pm annotation is added (see
http://marc.info/?l=linux-pm&m=136811978510598&w=2) then this whole
discussion will become moot. :-)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-05-09 19:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-07 9:48 [RFC PATCH 0/2] USB: OHCI: Start splitting up the driver Manjunath Goudar
2013-05-07 9:48 ` [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module Manjunath Goudar
2013-05-07 9:48 ` [RFC PATCH 2/2] USB: OHCI: make ohci-pci a separate driver Manjunath Goudar
[not found] <1365746856-7772-2-git-send-email-manjunath.goudar@linaro.org>
2013-05-07 9:50 ` [RFC PATCH 0/2] USB: OHCI: Start splitting up the driver Manjunath Goudar
2013-05-07 9:50 ` [RFC PATCH 1/2] USB: OHCI: prepare to make ohci-hcd a library module Manjunath Goudar
2013-05-07 15:15 ` Alan Stern
[not found] <CAJFYCKGSctw1_nXoETYwyVkgAncHyBb8dmZ9aBV-av=8S0rw4A@mail.gmail.com>
2013-05-09 15:54 ` Alan Stern
2013-05-09 19:25 ` Arnd Bergmann
2013-05-09 19:36 ` Alan Stern
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).