Re: [PATCH V2 05/10] USB: OHCI: Properly handle ohci-exynos suspend
On 14 June 2013 01:22, Alan Stern st...@rowland.harvard.edu wrote:
On Thu, 13 Jun 2013, Tomasz Figa wrote:
+ rc = ohci_suspend(hcd, do_wakeup);
+ if (rc == 0 do_wakeup HCD_WAKEUP_PENDING(hcd)) {
+ ohci_resume(hcd, false);
+ rc = -EBUSY;
+ }
I'm not into USB host subsystem, so I might just ask a stupid question.
Can't we make ohci_suspend check this for us, so the drivers would just
check for error code? It seems like in all your patches this part of code
is duplicated, looking as a good candidate to be generic.
Argh! You're right, of course.
I didn't see it, because the only existing place where this check is
made is in the PCI glue layer. Pushing it into the HCDs themselves is
obviously the right thing to do.
Manjanuth, let's do this. You can write a preliminary patch that puts
this check at the end of the ohci_suspend() routine, and then resubmit
your series.
Alan and Tomaszas you are correct.
Initially I also thought same, but later I analyzed this code is
not necessary for all bus glue; so I did not write in ohci_suspend()
routine.
After Alan explanation I am writing below code end of ohci_suspend()
routine.is it correct Alan.
if (do_wakeup HCD_WAKEUP_PENDING(hcd)) {
ohci_resume(hcd, false);
rc = -EBUSY;
}
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 05/10] USB: OHCI: Properly handle ohci-exynos suspend
On 18 June 2013 15:24, Manjunath Goudar manjunath.gou...@linaro.org wrote:
On 14 June 2013 01:22, Alan Stern st...@rowland.harvard.edu wrote:
On Thu, 13 Jun 2013, Tomasz Figa wrote:
+ rc = ohci_suspend(hcd, do_wakeup);
+ if (rc == 0 do_wakeup HCD_WAKEUP_PENDING(hcd)) {
+ ohci_resume(hcd, false);
+ rc = -EBUSY;
+ }
I'm not into USB host subsystem, so I might just ask a stupid question.
Can't we make ohci_suspend check this for us, so the drivers would just
check for error code? It seems like in all your patches this part of
code
is duplicated, looking as a good candidate to be generic.
Argh! You're right, of course.
I didn't see it, because the only existing place where this check is
made is in the PCI glue layer. Pushing it into the HCDs themselves is
obviously the right thing to do.
Manjanuth, let's do this. You can write a preliminary patch that puts
this check at the end of the ohci_suspend() routine, and then resubmit
your series.
Alan and Tomaszas you are correct.
Initially I also thought same, but later I analyzed this code is not
necessary for all bus glue; so I did not write in ohci_suspend() routine.
After Alan explanation I am writing below code end of ohci_suspend()
routine.is it correct Alan.
if (do_wakeup HCD_WAKEUP_PENDING(hcd)) {
ohci_resume(hcd, false);
rc = -EBUSY;
You probably need to return this error code.
--
With warm regards,
Sachin
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
first ubuntu saucy test rebuild
The first test rebuild of saucy salamander was started yesterday for the amd64, i386 and armhf architectures. Currently running, finished for main, universe will finish within the next ten days (armhf a bit earlier). Results can be seen at http://people.ubuntuwire.org/~wgrant/rebuild-ftbfs-test/test-rebuild-20130614-saucy.html The archive for the test rebuild is https://launchpad.net/ubuntu/+archive/test-rebuild-20130614/ Some common build failures are: - underlinking: symbols used in linked object files, which formerly were resolved by linked libraries. The fix almost always is to add the library (mentioned in the error message) to the link command. - build failures exposed by GCC-4.8. See http://gcc.gnu.org/gcc-4.8/porting_to.html for some guidance. Please help fixing the build failures for the final release. Matthias ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 05/10] USB: OHCI: Properly handle ohci-exynos suspend
On Tue, 18 Jun 2013, Manjunath Goudar wrote:
After Alan explanation I am writing below code end of ohci_suspend()
routine.is it correct Alan.
if (do_wakeup HCD_WAKEUP_PENDING(hcd)) {
ohci_resume(hcd, false);
rc = -EBUSY;
}
I'm glad you asked, because there is an important part I forgot about.
Just before the code above, it is also necessary to add:
synchronize_irq(hcd-irq);
The reason is because a wakeup interrupt might race with the suspend
call. If the suspend finishes first, we need to wait until the
interrupt has been handled before checking whether there is a pending
wakeup. That's what synchronize_irq() does; it waits until all the
outstanding interrupt requests have been handled.
(Also, as Sachin pointed out, you have to return rc instead of 0.)
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 1/6] USB: OHCI: make ohci-exynos a separate driver
On Wed, 12 Jun 2013, Manjunath Goudar wrote: Separate the Samsung OHCI EXYNOS 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: -exynos_ohci_hcd structure assignment error fixed. -Removed multiple usb_create_hcd() from prob funtion. -platform_set_drvdata() called before exynos_ohci_phy_enable(). -ohci_setup() removed because it is called in .reset member of the ohci_hc_driver structure --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -12,24 +12,39 @@ */ #include linux/clk.h +#include linux/dma-mapping.h +#include linux/io.h +#include linux/kernel.h +#include linux/module.h #include linux/of.h #include linux/platform_device.h #include linux/platform_data/usb-ohci-exynos.h #include linux/usb/phy.h #include linux/usb/samsung_usb_phy.h +#include linux/usb.h +#include linux/usb/hcd.h +#include linux/usb/otg.h + +#include ohci.h + +#define DRIVER_DESC OHCI exynos driver I think the word EXYNOS is supposed to be in all capital letters. Apart from that, Acked-by: Alan Stern st...@rowland.harvard.edu ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 2/6] USB: OHCI: make ohci-omap a separate driver
On Wed, 12 Jun 2013, Manjunath Goudar wrote:
Separate the TI OHCI OMAP1/2 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:
-omap_ohci_clock_power(0) called in usb_hcd_omap_remove().
-Removed ohci_setup() call from usb_hcd_omap_probe() and ohci_omap_reset().
You were supposed to remove the call from usb_hcd_omap_probe but leave
in the call from ohci_omap_reset.
-host_enabled and host_initialized variables aren't used for anything
thats what removed.
@@ -188,21 +195,21 @@ static void start_hnp(struct ohci_hcd *ohci)
/*-*/
-static int ohci_omap_init(struct usb_hcd *hcd)
+static int ohci_omap_reset(struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
struct omap_usb_config *config = hcd-self.controller-platform_data;
int need_transceiver = (config-otg != 0);
- int ret;
dev_dbg(hcd-self.controller, starting USB Controller\n);
- if (config-otg) {
+ if (config-otg || config-rwc) {
ohci_to_hcd(ohci)-self.otg_port = config-otg;
/* default/minimum OTG power budget: 8 mA */
ohci_to_hcd(ohci)-power_budget = 8;
These three lines are supposed to run only when config-otg is set, not
when config-rwc is set. The next two lines can run when either one is
set.
Also, ohci_to_hcd(ohci) should be replaced by hcd.
+ ohci-hc_control = OHCI_CTRL_RWC;
+ writel(OHCI_CTRL_RWC, ohci-regs-control);
}
-
Don't remove this blank line.
/* boards can use OTG transceivers in non-OTG modes */
need_transceiver = need_transceiver
|| machine_is_omap_h2() || machine_is_omap_h3();
@@ -238,9 +245,6 @@ static int ohci_omap_init(struct usb_hcd *hcd)
omap_1510_local_bus_init();
}
- if ((ret = ohci_init(ohci)) 0)
- return ret;
-
This should call ohci_setup instead of ohci_init.
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 2/6] USB: OHCI: make ohci-omap a separate driver
On Wed, 12 Jun 2013, Manjunath Goudar wrote:
Separate the TI OHCI OMAP1/2 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:
-omap_ohci_clock_power(0) called in usb_hcd_omap_remove().
-Removed ohci_setup() call from usb_hcd_omap_probe() and ohci_omap_reset().
-host_enabled and host_initialized variables aren't used for anything
thats what removed.
There's one more thing I just noticed.
@@ -188,21 +195,21 @@ static void start_hnp(struct ohci_hcd *ohci)
/*-*/
-static int ohci_omap_init(struct usb_hcd *hcd)
+static int ohci_omap_reset(struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
struct omap_usb_config *config = hcd-self.controller-platform_data;
int need_transceiver = (config-otg != 0);
- int ret;
dev_dbg(hcd-self.controller, starting USB Controller\n);
- if (config-otg) {
+ if (config-otg || config-rwc) {
ohci_to_hcd(ohci)-self.otg_port = config-otg;
/* default/minimum OTG power budget: 8 mA */
ohci_to_hcd(ohci)-power_budget = 8;
+ ohci-hc_control = OHCI_CTRL_RWC;
+ writel(OHCI_CTRL_RWC, ohci-regs-control);
This last line must not appear here, because ohci-regs doesn't get set
until ohci_setup() calls ohci_init(). Removing it entirely is safe
because ohci_run() does the same thing later on. Or you can move both
of these last two lines after the call to ohci_setup().
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 4/6] USB: OHCI: make ohci-spear a separate driver
On Wed, 12 Jun 2013, Manjunath Goudar wrote: Separate the ST OHCI SPEAr 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: -ohci_setup() removed because it is called in .reset member of the ohci_hc_driver structure. -debugging stuff isn't needed any more that's what removed. Acked-by: Alan Stern st...@rowland.harvard.edu ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 3/6] USB: OHCI: make ohci-omap3 a separate driver
On Wed, 12 Jun 2013, Manjunath Goudar wrote:
Separate the TI OHCI OMAP3 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:
-ohci_setup() removed because it is called in .reset member
of the ohci_hc_driver structure.
-The improper multi-line commenting style written in proper way.
('*' characters aligned in vertically).
Generally okay, just one problem...
@@ -185,7 +118,14 @@ static int ohci_hcd_omap3_probe(struct platform_device
*pdev)
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);
- ohci_hcd_init(hcd_to_ohci(hcd));
+ ohci = hcd_to_ohci(hcd);
+ /*
+ * RemoteWakeupConnected has to be set explicitly before
+ * calling ohci_run. The reset value of RWC is 0.
+ */
+ ohci-hc_control = OHCI_CTRL_RWC;
+ writel(OHCI_CTRL_RWC, ohci-regs-control);
ohci-regs doesn't get set until usb_add_hcd, so this line must not
appear here. You can simply remove it.
+ dev_dbg(hcd-self.controller, starting OHCI controller\n);
ret = usb_add_hcd(hcd, irq, 0);
if (ret) {
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
