Re: [PATCH 3/3] USB: ehci-fsl: Add power management support (resume after deep sleep)

[Scott Wood <scottwood@freescale.com>]

On Wed, Sep 23, 2009 at 10:52:44PM +0400, Anton Vorontsov wrote:
+#ifdef CONFIG_SUSPEND
+struct ehci_fsl {
+       struct ehci_hcd ehci;
+
+       /* Saved USB PHY settings, need to restore after deep sleep. */
+       u32 usb_ctrl;
+};

This doesn't seem like the right place to define this... what if we later
need something else that isn't for suspend?  And you could get rid of
EHCI_FSL_PRIV_SIZE.

> +static int ehci_fsl_drv_suspend(struct device *dev)
> +{
> +	struct usb_hcd *hcd = dev_get_drvdata(dev);
> +	struct ehci_fsl *ehci_fsl = hcd_to_ehci_fsl(hcd);
> +	void __iomem *non_ehci = hcd->regs;
> +
> +	if (!fsl_deep_sleep())
> +		return 0;

We'll also need to do this if we support suspend to disk...  is there any
good way for the driver to determine that that's what's being done?  Or
more generally, for the platform to communicate to the drivers which ones
are going to lose state without one-off hacks like fsl_deep_sleep().

> +	/* Power up ports (avoids devices disconnect). */
> +	port_nm = HCS_N_PORTS(ehci->hcs_params);
> +	while (port_nm--) {
> +		u32 port_sc;
> +
> +		port_sc = ehci_readl(ehci, &ehci->regs->port_status[port_nm]);
> +		port_sc |= PORT_POWER;
> +		ehci_writel(ehci, port_sc, &ehci->regs->port_status[port_nm]);
> +	}
> +	mdelay(30);

Instead of mdelay, can we somehow hold off any USB operations for a
while?

-Scott