Re: [PATCH V2 5/6] USB: OHCI: make ohci-at91 a separate driver

2013-06-22 Thread Alan Stern 
On Sun, 23 Jun 2013, Manjunath Goudar wrote:

  As a general rule, you should never change code that you don't
  understand.  Do you _know_ that it will be safe to call ohci_setup() or
  ohci_restart() at this point?
 
 
 From David Brownell comment I am understanding,instead of calling
 ohci_setup()
 or ohci_restart(),we can use directly below code.
 
 ohci-hc_control = OHCI_CTRL_RWC;
 ohci_writel (ohci, ohci-hc_control, ohci-regs-control);
 ohci-rh_state = OHCI_RH_HALTED;

Of course you can use that code; that's exactly what ohci_usb_reset() 
does now.

 the 3rd line code is written by you,I want to know what exactly it is doing.
 Is it required here?

Yes, it is required.  ohci-rh_state stores the current state of the
root hub.  When the controller is reset, the root hub goes into the 
HALTED state; this line records that fact.

  It might be a good idea to get in touch with the person who wrote that
  routine originally and ask why they used ohci_usb_reset().
 
  yes I will.
 
 Hi David,
 
 As I understood ohci_usb_reset() is calling for to notice
 disconnect,reconnect,
 or wakeup without the 48 MHz clock active.
 After fix power management hanging(869aa98c) issue by Patrice Vilchez,I
 think
 ohci_usb_reset() is not required to call. what is your opinion about this.

Sadly, David Brownell died in 2011.  I can tell you, though, that
commit 869aa98c does not eliminate the need to call ohci_usb_reset().

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 5/6] USB: OHCI: make ohci-at91 a separate driver

2013-06-21 Thread Alan Stern 
On Fri, 21 Jun 2013, Manjunath Goudar wrote:

 On 20 June 2013 22:23, Alan Stern st...@rowland.harvard.edu wrote:
 
  On Thu, 20 Jun 2013, Manjunath Goudar wrote:
 
 @@ -686,7 +631,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device
*pdev, pm_message_t mesg)
* REVISIT: some boards will be able to turn VBUS off...
*/
   if (at91_suspend_entering_slow_clock()) {
 - ohci_usb_reset (ohci);
 + ohci_restart(ohci);
   
Why did you change this?  Did we discuss it earlier?
   
  
   We are not discussed  regarding this,I think we need to call
   use ohci_resume() instead of ohci_restart().
 
  Why?  Don't you think the current code has a good reason for calling
  ohci_usb_reset()?
 
 
 Here ohci_usb_reset() is static function,that is what I am planing to call
 ohci_setup() or ohci_restart() because it is calling  ohci_usb_reset(),
 If not calling, we can make ohci_usb_reset() function as non-static
 function
 or use directly  ohci_usb_reset() function code here.
 
 Let me know which one is good approach.

As a general rule, you should never change code that you don't 
understand.  Do you _know_ that it will be safe to call ohci_setup() or 
ohci_restart() at this point?

It might be a good idea to get in touch with the person who wrote that
routine originally and ask why they used ohci_usb_reset().

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 5/6] USB: OHCI: make ohci-at91 a separate driver

2013-06-20 Thread Alan Stern 
On Thu, 20 Jun 2013, Manjunath Goudar wrote:

   @@ -686,7 +631,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device
  *pdev, pm_message_t mesg)
  * REVISIT: some boards will be able to turn VBUS off...
  */
 if (at91_suspend_entering_slow_clock()) {
   - ohci_usb_reset (ohci);
   + ohci_restart(ohci);
 
  Why did you change this?  Did we discuss it earlier?
 
 
 We are not discussed  regarding this,I think we need to call
 use ohci_resume() instead of ohci_restart().

Why?  Don't you think the current code has a good reason for calling 
ohci_usb_reset()?

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 5/6] USB: OHCI: make ohci-at91 a separate driver

2013-06-19 Thread Alan Stern 
On Wed, 12 Jun 2013, Manjunath Goudar wrote:

 Separate the  TI OHCI Atmel 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.
 
 V2:
  -Set non-standard fields in ohci_at91_hc_driver manually, rather than
   relying on an expanded struct ohci_driver_overrides.
  -Save orig_ohci_hub_control and orig_ohci_hub_status_data rather than
   relying on ohci_hub_control and hub_status_data being exported.
  -ohci_setup() has been removed because it is called in .reset member
   of the ohci_hc_driver structure.

 @@ -111,6 +125,8 @@ static void usb_hcd_at91_remove (struct usb_hcd *, struct 
 platform_device *);
  static int usb_hcd_at91_probe(const struct hc_driver *driver,
   struct platform_device *pdev)
  {
 + struct at91_usbh_data   *board;
 + struct ohci_hcd *ohci;

Variables are supposed to be not aligned at all (in which case you 
don't use tabs) or all aligned the same way.  In this case you put a 
tab before the *board; therefore the *ohci should line up with it.

No, this isn't an artifact of my email program.  They really are not
aligned.

 @@ -163,8 +179,11 @@ static int usb_hcd_at91_probe(const struct hc_driver 
 *driver,
   goto err5;
   }
  
 + board = hcd-self.controller-platform_data;
 + ohci = hcd_to_ohci(hcd);
 + ohci-num_ports = board-ports;
   at91_start_hc(pdev);
 - ohci_hcd_init(hcd_to_ohci(hcd));
 + ohci_setup(hcd);

Didn't you say above that this version of the patch removes the call to 
ohci_setup()?

 @@ -686,7 +631,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev, 
 pm_message_t mesg)
* REVISIT: some boards will be able to turn VBUS off...
*/
   if (at91_suspend_entering_slow_clock()) {
 - ohci_usb_reset (ohci);
 + ohci_restart(ohci);

Why did you change this?  Did we discuss it earlier?

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 5/6] USB: OHCI: make ohci-at91 a separate driver

2013-06-12 Thread Manjunath Goudar 
Separate the  TI OHCI Atmel 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.

V2:
 -Set non-standard fields in ohci_at91_hc_driver manually, rather than
  relying on an expanded struct ohci_driver_overrides.
 -Save orig_ohci_hub_control and orig_ohci_hub_status_data rather than
  relying on ohci_hub_control and hub_status_data being exported.
 -ohci_setup() has been removed because it is called in .reset member
  of the ohci_hc_driver structure.

Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Cc: Arnd Bergmann a...@arndb.de
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Greg KH g...@kroah.com
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/host/Kconfig |8 +++
 drivers/usb/host/Makefile|1 +
 drivers/usb/host/ohci-at91.c |  148 +++---
 drivers/usb/host/ohci-hcd.c  |   18 -
 4 files changed, 74 insertions(+), 101 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 46c2f42..e4dc9ab 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -390,6 +390,14 @@ config USB_OHCI_HCD_SPEAR
   Enables support for the on-chip OHCI controller on
   ST SPEAr chips.
 
+config USB_OHCI_HCD_AT91
+tristate  Support for Atmel on-chip OHCI USB controller
+depends on USB_OHCI_HCD  ARCH_AT91
+default y
+---help---
+  Enables support for the on-chip OHCI controller on
+  Atmel chips.
+
 config USB_OHCI_HCD_OMAP3
tristate OHCI support for OMAP3 and later chips
depends on (ARCH_OMAP3 || ARCH_OMAP4)
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 26cb6b3..f3e02c0 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_USB_OHCI_EXYNOS) += ohci-exynos.o
 obj-$(CONFIG_USB_OHCI_HCD_OMAP1)   += ohci-omap.o
 obj-$(CONFIG_USB_OHCI_HCD_OMAP3)   += ohci-omap3.o
 obj-$(CONFIG_USB_OHCI_HCD_SPEAR)   += ohci-spear.o
+obj-$(CONFIG_USB_OHCI_HCD_AT91)+= ohci-at91.o
 
 obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
 obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 2ee1496..fb2f127 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -13,27 +13,41 @@
  */
 
 #include linux/clk.h
-#include linux/platform_device.h
+#include linux/dma-mapping.h
 #include linux/of_platform.h
 #include linux/of_gpio.h
+#include linux/platform_device.h
 #include linux/platform_data/atmel.h
+#include linux/io.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/usb.h
+#include linux/usb/hcd.h
 
 #include mach/hardware.h
 #include asm/gpio.h
 
 #include mach/cpu.h
 
-#ifndef CONFIG_ARCH_AT91
-#error CONFIG_ARCH_AT91 must be defined.
-#endif
+
+#include ohci.h
 
 #define valid_port(index)  ((index) = 0  (index)  AT91_MAX_USBH_PORTS)
 #define at91_for_each_port(index)  \
for ((index) = 0; (index)  AT91_MAX_USBH_PORTS; (index)++)
 
 /* interface and function clocks; sometimes also an AHB clock */
+
+#define DRIVER_DESC OHCI Atmel driver
+
+static const char hcd_name[] = ohci-atmel;
+
+static struct hc_driver __read_mostly ohci_at91_hc_driver;
 static struct clk *iclk, *fclk, *hclk;
 static int clocked;
+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);
 
 extern int usb_disabled(void);
 
@@ -111,6 +125,8 @@ static void usb_hcd_at91_remove (struct usb_hcd *, struct 
platform_device *);
 static int usb_hcd_at91_probe(const struct hc_driver *driver,
struct platform_device *pdev)
 {
+   struct at91_usbh_data   *board;
+   struct ohci_hcd *ohci;
int retval;
struct usb_hcd *hcd = NULL;
 
@@ -163,8 +179,11 @@ static int usb_hcd_at91_probe(const struct hc_driver 
*driver,
goto err5;
}
 
+   board = hcd-self.controller-platform_data;
+   ohci = hcd_to_ohci(hcd);
+   ohci-num_ports = board-ports;
at91_start_hc(pdev);
-   ohci_hcd_init(hcd_to_ohci(hcd));
+   ohci_setup(hcd);
 
retval = usb_add_hcd(hcd, pdev-resource[1].start, IRQF_SHARED);
if (retval == 0)
@@ -221,36 +240,6 @@ static void usb_hcd_at91_remove(struct usb_hcd *hcd,
 }
 
 /*-*/
-
-static int
-ohci_at91_reset (struct usb_hcd *hcd)
-{
-   struct at91_usbh_data   *board = hcd-self.controller-platform_data;
-   struct ohci_hcd *ohci = hcd_to_ohci (hcd);
-   int ret;
-
-   if ((ret = ohci_init(ohci))  0)
-   return ret;
-
-   ohci-num_ports = board-ports;
-