From patchwork Fri Jan 11 18:27:47 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Stern X-Patchwork-Id: 1967031 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 418FC3FF0F for ; Fri, 11 Jan 2013 18:30:55 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TtjKg-0003vB-5X; Fri, 11 Jan 2013 18:27:58 +0000 Received: from iolanthe.rowland.org ([192.131.102.54]) by merlin.infradead.org with smtp (Exim 4.76 #1 (Red Hat Linux)) id 1TtjKY-0003tj-AK for linux-arm-kernel@lists.infradead.org; Fri, 11 Jan 2013 18:27:51 +0000 Received: (qmail 4655 invoked by uid 2102); 11 Jan 2013 13:27:47 -0500 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 11 Jan 2013 13:27:47 -0500 Date: Fri, 11 Jan 2013 13:27:47 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Roger Quadros Subject: Re: [PATCH 07/14] usb: ehci-omap: Instantiate PHY devices if required In-Reply-To: <50F037C9.40306@ti.com> Message-ID: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130111_132750_514555_BDC303EB X-CRM114-Status: GOOD ( 26.40 ) X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: sameo@linux.intel.com, tony@atomide.com, gregkh@linuxfoundation.org, USB list , Kernel development list , Felipe Balbi , kishon@ti.com, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Fri, 11 Jan 2013, Roger Quadros wrote: > Apart from what you mentioned I did some more trivial changes. e.g. > > + !IS_ENABLED(CONFIG_USB_EHCI_HCD_OMAP) && \ > instead of > + !defined(CONFIG_USB_EHCI_HCD_OMAP) && \ Ah, that's a very good catch. There's another entry needing the same thing. I'll put that in a separate preliminary patch, and also put the ehci->priv addition in there instead of including it with the ehci-mxc update. > use ehci_hcd_omap_driver instead of ehci_omap_driver Now fixed. > I tried using ehci->priv in ehci-omap driver but noticed that the > private data gets corrupted after the EHCI controller is running and has > enumerated a few devices. > > If I disable USB_DEBUG then things are fine. Could it be possible > that someone is overflowing data when USB_DEBUG is enabled? > > My implementation is pasted below. (May contain whitespace errors due to > MS exchange). Patch 2 attached in case. > > What was happening there is that omap_priv->phy was not the same during > remove() as it was set to during probe(). I don't understand -- your second patch doesn't use ehci->priv at all. How can the private data be getting corrupted? Below is an updated version of your second patch. You'll need to resolve one or two merge errors because it's not based on the same starting point as yours. (And it totally omits the part affecting usb-omap.h.) But it will show you what needs to be done in order to use ehci->priv. > Would be nice if you could check if the same happens with ehci-mxc. I can't -- I don't have an ARM-based system. But if you still see problems, I can test with ehci-pci. Alan Stern Index: usb-3.7/drivers/usb/host/ehci-omap.c =================================================================== --- usb-3.7.orig/drivers/usb/host/ehci-omap.c +++ usb-3.7/drivers/usb/host/ehci-omap.c @@ -69,6 +69,10 @@ static const char hcd_name[] = "ehci-oma /*-------------------------------------------------------------------------*/ +struct omap_ehci_hcd { + struct usb_phy **phy; /* one PHY for each port */ + int nports; +}; static inline void ehci_write(void __iomem *base, u32 reg, u32 val) { @@ -177,7 +181,8 @@ static void disable_put_regulator( static struct hc_driver __read_mostly ehci_omap_hc_driver; static const struct ehci_driver_overrides ehci_omap_overrides __initdata = { - .reset = omap_ehci_init, + .reset = omap_ehci_init, + .extra_priv_size = sizeof(struct omap_ehci_hcd), }; /** @@ -193,6 +198,8 @@ static int ehci_hcd_omap_probe(struct pl struct ehci_hcd_omap_platform_data *pdata = dev->platform_data; struct resource *res; struct usb_hcd *hcd; + struct omap_ehci_hcd *omap_hcd; + struct usb_phy *phy; void __iomem *regs; int ret = -ENODEV; int irq; @@ -207,6 +214,11 @@ static int ehci_hcd_omap_probe(struct pl return -ENODEV; } + if (!pdata) { + dev_err(dev, "Missing platform data\n"); + return -ENODEV; + } + irq = platform_get_irq_byname(pdev, "ehci-irq"); if (irq < 0) { dev_err(dev, "EHCI irq failed\n"); @@ -238,8 +250,24 @@ static int ehci_hcd_omap_probe(struct pl hcd->rsrc_len = resource_size(res); hcd->regs = regs; + omap_hcd = (struct omap_ehci_hcd *) (hcd_to_ehci(hcd))->priv; + + omap_hcd->nports = pdata->nports; + i = sizeof(struct usb_phy *) * omap_hcd->nports; + omap_hcd->phy = devm_kzalloc(&pdev->dev, i, GFP_KERNEL); + if (!omap_hcd->phy) { + dev_err(dev, "Memory allocation failed\n"); + ret = -ENOMEM; + goto err_alloc_phy; + } + + platform_set_drvdata(pdev, hcd); + /* get ehci regulator and enable */ - for (i = 0 ; i < OMAP3_HS_USB_PORTS ; i++) { + for (i = 0 ; i < omap_hcd->nports ; i++) { + struct platform_device *phy_pdev; + struct usbhs_phy_config *phy_config; + if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY) { pdata->regulator[i] = NULL; continue; @@ -253,6 +281,33 @@ static int ehci_hcd_omap_probe(struct pl } else { regulator_enable(pdata->regulator[i]); } + + /* instantiate PHY */ + if (!pdata->phy_config[i]) { + dev_dbg(dev, "missing phy_config for port %d\n", i); + continue; + } + + phy_config = pdata->phy_config[i]; + phy_pdev = platform_device_register_data(&pdev->dev, + phy_config->name, i, phy_config->pdata, + phy_config->pdata_size); + if (IS_ERR(phy_pdev)) { + dev_dbg(dev, "error creating PHY device for port %d\n", + i); + } + + phy = usb_get_phy_from_dev(&phy_pdev->dev); + if (IS_ERR(phy)) { + dev_dbg(dev, "could not get USB PHY for port %d\n", i); + platform_device_unregister(phy_pdev); + continue; + } + + usb_phy_init(phy); + omap_hcd->phy[i] = phy; + /* bring PHY out of suspend */ + usb_phy_set_suspend(omap_hcd->phy[i], 0); } pm_runtime_enable(dev); @@ -282,6 +336,14 @@ static int ehci_hcd_omap_probe(struct pl err_pm_runtime: disable_put_regulator(pdata); pm_runtime_put_sync(dev); + for (i = 0 ; i < omap_hcd->nports ; i++) { + phy = omap_hcd->phy[i]; + if (!phy) + continue; + platform_device_unregister(to_platform_device(phy->dev)); + } + +err_alloc_phy: usb_put_hcd(hcd); err_io: @@ -300,13 +362,26 @@ err_io: */ static int ehci_hcd_omap_remove(struct platform_device *pdev) { - struct device *dev = &pdev->dev; - struct usb_hcd *hcd = dev_get_drvdata(dev); - struct ehci_hcd_omap_platform_data *pdata = dev->platform_data; + struct device *dev = &pdev->dev; + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct omap_ehci_hcd *omap_hcd; + int i; usb_remove_hcd(hcd); disable_put_regulator(dev->platform_data); iounmap(hcd->regs); + + omap_hcd = (struct omap_ehci_hcd *) (hcd_to_ehci(hcd))->priv; + for (i = 0; i < omap_hcd->nports; i++) { + struct usb_phy *phy = omap_hcd->phy[i]; + + if (!phy) + continue; + + usb_phy_shutdown(phy); + platform_device_unregister(to_platform_device(phy->dev)); + } + usb_put_hcd(hcd); pm_runtime_put_sync(dev);