Re: [PATCH V2 4/6] USB: EHCI: tegra: remove all power management

2013-06-05 Thread Alan Stern
On Tue, 4 Jun 2013, Stephen Warren wrote:

 From: Stephen Warren swar...@nvidia.com
 
 The PM routines in ehci-tegra.c use APIs such as ehci_reset(),
 ehci_halt(), and ehci_tdi_reset() that would need to be exported to
 convert ehci-tegra.c into a separate module from ehci-hcd.c. However,
 we'd prefer not to export them.
 
 Instead, simply remove all power management functionality. Runtime PM
 was disabled since it didn't work correctly, and system suspend isn't
 yet supported in a meaningful way. So, this change doesn't lose any
 functionality.
 
 Hopefully the power management logic can be reimplemented in a cleaner
 way in the future.
 
 Signed-off-by: Stephen Warren swar...@nvidia.com
 ---
 NOTE: This could do with a little more testing on a few more boards. If
 Alan is OK with this series, I'll go through and do that, and let you
 know when I've tested it enough to be applied. I saw no issues on the one
 board I tested with though.
 
 I'd appreciate any hints if this patch is actually removing more
 functionality than I think, and is going to break something horribly.

This is a good way to break the logjam.  It needs just a couple of
improvements:

 diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
 index 289b9b8..8f42b3a 100644
 --- a/drivers/usb/host/ehci-tegra.c
 +++ b/drivers/usb/host/ehci-tegra.c
 @@ -61,15 +61,6 @@ static void tegra_ehci_power_up(struct usb_hcd *hcd)
   tegra-host_resumed = 1;
  }
  
 -static void tegra_ehci_power_down(struct usb_hcd *hcd)
 -{
 - struct tegra_ehci_hcd *tegra = dev_get_drvdata(hcd-self.controller);
 -
 - tegra-host_resumed = 0;
 - usb_phy_set_suspend(hcd-phy, 1);
 - clk_disable_unprepare(tegra-clk);
 -}
 -

tegra_ehci_power_up() shouldn't be retained either.  And with these
functions removed, there's no need to keep the tegra-host_resumed
field.

Related to this, the tegra_ehci_shutdown() routine should be removed,
and the .shutdown entry in tegra_ehci_hc_driver should point to
ehci_shutdown.

 @@ -399,10 +369,6 @@ static const struct hc_driver tegra_ehci_hc_driver = {
   .map_urb_for_dma= tegra_ehci_map_urb_for_dma,
   .unmap_urb_for_dma  = tegra_ehci_unmap_urb_for_dma,
   .hub_control= tegra_ehci_hub_control,
 -#ifdef CONFIG_PM
 - .bus_suspend= ehci_bus_suspend,
 - .bus_resume = ehci_bus_resume,
 -#endif
  };

These two entries should be retained.  They refer to USB bus suspend
rather than controller suspend, and they don't use any Tegra-specific
code.

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 4/6] USB: EHCI: tegra: remove all power management

2013-06-04 Thread Stephen Warren
From: Stephen Warren swar...@nvidia.com

The PM routines in ehci-tegra.c use APIs such as ehci_reset(),
ehci_halt(), and ehci_tdi_reset() that would need to be exported to
convert ehci-tegra.c into a separate module from ehci-hcd.c. However,
we'd prefer not to export them.

Instead, simply remove all power management functionality. Runtime PM
was disabled since it didn't work correctly, and system suspend isn't
yet supported in a meaningful way. So, this change doesn't lose any
functionality.

Hopefully the power management logic can be reimplemented in a cleaner
way in the future.

Signed-off-by: Stephen Warren swar...@nvidia.com
---
NOTE: This could do with a little more testing on a few more boards. If
Alan is OK with this series, I'll go through and do that, and let you
know when I've tested it enough to be applied. I saw no issues on the one
board I tested with though.

I'd appreciate any hints if this patch is actually removing more
functionality than I think, and is going to break something horribly.

v2: New patch.
---
 drivers/usb/host/ehci-tegra.c | 225 --
 1 file changed, 225 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 289b9b8..8f42b3a 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -61,15 +61,6 @@ static void tegra_ehci_power_up(struct usb_hcd *hcd)
tegra-host_resumed = 1;
 }
 
-static void tegra_ehci_power_down(struct usb_hcd *hcd)
-{
-   struct tegra_ehci_hcd *tegra = dev_get_drvdata(hcd-self.controller);
-
-   tegra-host_resumed = 0;
-   usb_phy_set_suspend(hcd-phy, 1);
-   clk_disable_unprepare(tegra-clk);
-}
-
 static int tegra_ehci_internal_port_reset(
struct ehci_hcd *ehci,
u32 __iomem *portsc_reg
@@ -248,27 +239,6 @@ done:
return retval;
 }
 
-static void tegra_ehci_restart(struct usb_hcd *hcd)
-{
-   struct ehci_hcd *ehci = hcd_to_ehci(hcd);
-
-   ehci_reset(ehci);
-
-   /* setup the frame list and Async q heads */
-   ehci_writel(ehci, ehci-periodic_dma, ehci-regs-frame_list);
-   ehci_writel(ehci, (u32)ehci-async-qh_dma, ehci-regs-async_next);
-   /* setup the command register and set the controller in RUN mode */
-   ehci-command = ~(CMD_LRESET|CMD_IAAD|CMD_PSE|CMD_ASE|CMD_RESET);
-   ehci-command |= CMD_RUN;
-   ehci_writel(ehci, ehci-command, ehci-regs-command);
-
-   down_write(ehci_cf_port_reset_rwsem);
-   ehci_writel(ehci, FLAG_CF, ehci-regs-configured_flag);
-   /* flush posted writes */
-   ehci_readl(ehci, ehci-regs-command);
-   up_write(ehci_cf_port_reset_rwsem);
-}
-
 static void tegra_ehci_shutdown(struct usb_hcd *hcd)
 {
struct tegra_ehci_hcd *tegra = dev_get_drvdata(hcd-self.controller);
@@ -399,10 +369,6 @@ static const struct hc_driver tegra_ehci_hc_driver = {
.map_urb_for_dma= tegra_ehci_map_urb_for_dma,
.unmap_urb_for_dma  = tegra_ehci_unmap_urb_for_dma,
.hub_control= tegra_ehci_hub_control,
-#ifdef CONFIG_PM
-   .bus_suspend= ehci_bus_suspend,
-   .bus_resume = ehci_bus_resume,
-#endif
 };
 
 static int setup_vbus_gpio(struct platform_device *pdev,
@@ -432,182 +398,6 @@ static int setup_vbus_gpio(struct platform_device *pdev,
return err;
 }
 
-#ifdef CONFIG_PM
-
-static int controller_suspend(struct device *dev)
-{
-   struct tegra_ehci_hcd *tegra =
-   platform_get_drvdata(to_platform_device(dev));
-   struct ehci_hcd *ehci = tegra-ehci;
-   struct usb_hcd *hcd = ehci_to_hcd(ehci);
-   struct ehci_regs __iomem *hw = ehci-regs;
-   unsigned long flags;
-
-   if (time_before(jiffies, ehci-next_statechange))
-   msleep(10);
-
-   ehci_halt(ehci);
-
-   spin_lock_irqsave(ehci-lock, flags);
-   tegra-port_speed = (readl(hw-port_status[0])  26)  0x3;
-   clear_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
-   spin_unlock_irqrestore(ehci-lock, flags);
-
-   tegra_ehci_power_down(hcd);
-   return 0;
-}
-
-static int controller_resume(struct device *dev)
-{
-   struct tegra_ehci_hcd *tegra =
-   platform_get_drvdata(to_platform_device(dev));
-   struct ehci_hcd *ehci = tegra-ehci;
-   struct usb_hcd *hcd = ehci_to_hcd(ehci);
-   struct ehci_regs __iomem *hw = ehci-regs;
-   unsigned long val;
-
-   set_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
-   tegra_ehci_power_up(hcd);
-
-   if (tegra-port_speed  TEGRA_USB_PHY_PORT_SPEED_HIGH) {
-   /* Wait for the phy to detect new devices
-* before we restart the controller */
-   msleep(10);
-   goto restart;
-   }
-
-   /* Force the phy to keep data lines in suspend state */
-   tegra_ehci_phy_restore_start(hcd-phy, tegra-port_speed);
-
-   /* Enable host mode */
-   tdi_reset(ehci);
-
-   /* Enable Port