[PATCH] usb: phy: tegra: Avoid use of sizeof(void)
From: Thierry Reding tred...@nvidia.com The PHY configuration is stored in an opaque config field, but when allocating the structure, its proper size needs to be known. In the case of UTMI, the proper structure is tegra_utmip_config of which a local variable already exists, so we can use that to obtain the size from. Fixes the following warning from the sparse checker: drivers/usb/phy/phy-tegra-usb.c:882:17: warning: expression using sizeof(void) Fixes: 81d5dfe6d8b3 (usb: phy: tegra: Read UTMIP parameters from device tree) Cc: sta...@vger.kernel.org Signed-off-by: Thierry Reding tred...@nvidia.com --- drivers/usb/phy/phy-tegra-usb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c index 13b4fa287da8..886f1807a67b 100644 --- a/drivers/usb/phy/phy-tegra-usb.c +++ b/drivers/usb/phy/phy-tegra-usb.c @@ -878,8 +878,8 @@ static int utmi_phy_probe(struct tegra_usb_phy *tegra_phy, return -ENOMEM; } - tegra_phy-config = devm_kzalloc(pdev-dev, - sizeof(*tegra_phy-config), GFP_KERNEL); + tegra_phy-config = devm_kzalloc(pdev-dev, sizeof(*config), +GFP_KERNEL); if (!tegra_phy-config) { dev_err(pdev-dev, unable to allocate memory for USB UTMIP config\n); -- 2.0.1 -- 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 2/4] ARM: tegra: Add resets has-utmi-pad-registers flag to all USB PHYs
On Thu, Jul 10, 2014 at 06:12:48PM +0300, Tuomas Tynkkynen wrote: Thierry, Since Stephen's on a vacation, I'd like to double-check with you that the DT changes looks good. Greg has applied these to the USB tree today. Yes, looks sane to me. Not sure how much people will like to see the DTS changes go in through the USB tree, but I don't see a lot of potential for conflicts, so chances are nobody will notice anyway. Thierry pgpzbDGyOaGXo.pgp Description: PGP signature
[PATCH] usb: phy: tegra: Do not include asm/mach-types.h
From: Thierry Reding tred...@nvidia.com It is no longer needed and keeping it will break 64-bit ARM builds. Signed-off-by: Thierry Reding tred...@nvidia.com --- drivers/usb/phy/phy-tegra-usb.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c index cd36cb43ecc0..cd9d25e804b2 100644 --- a/drivers/usb/phy/phy-tegra-usb.c +++ b/drivers/usb/phy/phy-tegra-usb.c @@ -33,7 +33,6 @@ #include linux/usb/otg.h #include linux/usb/ulpi.h #include linux/usb/of.h -#include asm/mach-types.h #include linux/usb/ehci_def.h #include linux/usb/tegra_usb_phy.h #include linux/regulator/consumer.h -- 2.0.1 -- 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 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
On Fri, Jun 06, 2014 at 06:32:42PM +0530, Vivek Gautam wrote: On Wed, Jun 4, 2014 at 6:43 PM, Thierry Reding thierry.red...@gmail.com wrote: On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote: On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam gautam.vi...@samsung.com wrote: [...] diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 9cf80cb..dec691d 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -120,10 +120,9 @@ skip_phy: hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, hcd-rsrc_len); - if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); - err = -ENOMEM; + hcd-regs = devm_ioremap_resource(pdev-dev, res); Here, we replaced devm_ioremap() call with devm_ioremap_resource(), which internally requests the memory region I guess this could lead to problems if drivers haven't been written to cleanly split the register ranges that they access, since now two overlapping regions may be requested and cause the drivers to fail. Sorry i did not understand completely. Wouldn't the request_mem_region() fail for an already busy resource ? So devm_ioremap_resource() will in fact prevent the drivers from requesting the same memory region twice until the first request frees the region. Isn't it ? Yes exactly. What I was trying to say is that since drivers weren't requesting the resources before they may be using overlapping regions. Now that this patch changes these drivers to also request the resources they will fail if the regions overlap with those of other drivers. and then does a devm_ioremap() or devm_ioremap_nocache() based on the check for IORESOURCE_CACHEABLE flag. But this flag is not set for the resource of this device. So should we be explicitly setting the flag in driver ? I don't think it makes much sense to map these registers cached anyway. Drivers will likely expect writes to this region to take effect without needing any kind of flushing. These hcd-regs are going to be used by the controller, so wouldn't there be a performance difference when the requested address space is cacheable/non-cacheable ? The issue here is that if the region is mapped cacheable then register writes may not immediately take effect and that's almost certainly not what the driver will expect. I don't think it ever makes sense to map registers cacheable. Thierry pgpFDjlZl5Xiq.pgp Description: PGP signature
Re: [PATCH v2 6/6] usb: host: ohci-exynos: Use devm_ioremap_resource instead of devm_ioremap
On Wed, Jun 04, 2014 at 03:41:20PM +0530, Vivek Gautam wrote: Hi, On Sat, May 10, 2014 at 5:30 PM, Vivek Gautam gautam.vi...@samsung.com wrote: Using devm_ioremap_resource() API should actually be preferred over devm_ioremap(), since the former request the mem region first and then gives back the ioremap'ed memory pointer. devm_ioremap_resource() calls request_mem_region(), therby preventing other drivers to make any overlapping call to the same region. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com Although this patch and rest in the series are merged. But i have got a doubt, so making this thread alive. --- drivers/usb/host/ohci-exynos.c |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c index 9cf80cb..dec691d 100644 --- a/drivers/usb/host/ohci-exynos.c +++ b/drivers/usb/host/ohci-exynos.c @@ -120,10 +120,9 @@ skip_phy: hcd-rsrc_start = res-start; hcd-rsrc_len = resource_size(res); - hcd-regs = devm_ioremap(pdev-dev, res-start, hcd-rsrc_len); - if (!hcd-regs) { - dev_err(pdev-dev, Failed to remap I/O memory\n); - err = -ENOMEM; + hcd-regs = devm_ioremap_resource(pdev-dev, res); Here, we replaced devm_ioremap() call with devm_ioremap_resource(), which internally requests the memory region I guess this could lead to problems if drivers haven't been written to cleanly split the register ranges that they access, since now two overlapping regions may be requested and cause the drivers to fail. and then does a devm_ioremap() or devm_ioremap_nocache() based on the check for IORESOURCE_CACHEABLE flag. But this flag is not set for the resource of this device. So should we be explicitly setting the flag in driver ? I don't think it makes much sense to map these registers cached anyway. Drivers will likely expect writes to this region to take effect without needing any kind of flushing. Thierry pgphdTmeFKZ9N.pgp Description: PGP signature
Re: [RFC PATCH 06/10] usb: xhci: Add Tegra XHCI host-controller driver
On Thu, May 15, 2014 at 10:17:10AM +0200, Arnd Bergmann wrote: On Wednesday 14 May 2014 17:33:02 Andrew Bresticker wrote: [...] + /* Create child xhci-plat device */ + memset(xhci_resources, 0, sizeof(xhci_resources)); + res = platform_get_resource(to_platform_device(dev), IORESOURCE_IRQ, 0); + if (!res) { + dev_err(dev, Missing XHCI IRQ\n); + ret = -ENODEV; + goto out; + } + xhci_resources[0].start = res-start; + xhci_resources[0].end = res-end; + xhci_resources[0].flags = res-flags; + xhci_resources[0].name = res-name; + res = platform_get_resource(to_platform_device(dev), IORESOURCE_MEM, 0); + if (!res) { + dev_err(dev, Missing XHCI registers\n); + ret = -ENODEV; + goto out; + } + xhci_resources[1].start = res-start; + xhci_resources[1].end = res-end; + xhci_resources[1].flags = res-flags; + xhci_resources[1].name = res-name; + + xhci = platform_device_alloc(xhci-hcd, PLATFORM_DEVID_AUTO); + if (!xhci) { + dev_err(dev, Failed to allocate XHCI host\n); + ret = -ENOMEM; + goto out; + } This does not feel appropriate at all: Rather than creating a child device, you should have a specific driver that hooks into functions exported by the xhci core. See Documentation/driver-model/design-patterns.txt I don't think Documentation/driver-model/design-patterns.txt documents this. Perhaps this is what you had in mind? http://lwn.net/Articles/336262/ Thierry pgpVk903MVVuN.pgp Description: PGP signature
Re: [RFC PATCH 06/10] usb: xhci: Add Tegra XHCI host-controller driver
On Thu, May 15, 2014 at 01:18:22PM -0700, Andrew Bresticker wrote: Arnd, On Thu, May 15, 2014 at 1:17 AM, Arnd Bergmann a...@arndb.de wrote: On Wednesday 14 May 2014 17:33:02 Andrew Bresticker wrote: + +int tegra_xhci_register_mbox_notifier(struct notifier_block *nb) +{ + int ret; + + mutex_lock(tegra_xhci_mbox_lock); + ret = raw_notifier_chain_register(tegra_xhci_mbox_notifiers, nb); + mutex_unlock(tegra_xhci_mbox_lock); + + return ret; +} +EXPORT_SYMBOL(tegra_xhci_register_mbox_notifier); + +void tegra_xhci_unregister_mbox_notifier(struct notifier_block *nb) +{ + mutex_lock(tegra_xhci_mbox_lock); + raw_notifier_chain_unregister(tegra_xhci_mbox_notifiers, nb); + mutex_unlock(tegra_xhci_mbox_lock); +} +EXPORT_SYMBOL(tegra_xhci_unregister_mbox_notifier); What driver would use these? It's used by just this driver (the host) and the PHY driver (next patch in series). My feeling is that if you have a mailbox that is used by multiple drivers, you should use a proper mailbox driver to operate them, and have the drivers register with that API instead of a custom one. Ok, will do. + /* Create child xhci-plat device */ + memset(xhci_resources, 0, sizeof(xhci_resources)); + res = platform_get_resource(to_platform_device(dev), IORESOURCE_IRQ, 0); + if (!res) { + dev_err(dev, Missing XHCI IRQ\n); + ret = -ENODEV; + goto out; + } + xhci_resources[0].start = res-start; + xhci_resources[0].end = res-end; + xhci_resources[0].flags = res-flags; + xhci_resources[0].name = res-name; + res = platform_get_resource(to_platform_device(dev), IORESOURCE_MEM, 0); + if (!res) { + dev_err(dev, Missing XHCI registers\n); + ret = -ENODEV; + goto out; + } + xhci_resources[1].start = res-start; + xhci_resources[1].end = res-end; + xhci_resources[1].flags = res-flags; + xhci_resources[1].name = res-name; + + xhci = platform_device_alloc(xhci-hcd, PLATFORM_DEVID_AUTO); + if (!xhci) { + dev_err(dev, Failed to allocate XHCI host\n); + ret = -ENOMEM; + goto out; + } This does not feel appropriate at all: Rather than creating a child device, you should have a specific driver that hooks into functions exported by the xhci core. See Documentation/driver-model/design-patterns.txt This is how DWC3, currently the only in-tree non-PCI XHCI host driver, is structured - see drivers/usb/dwc3/host.c. The recently proposed Armada XHCI driver [1] just adds clock support and a hook in xhci-plat's probe() to do the platform-specific initialization. Ugh... that very much sounds like the midlayer mistake. Doing that is not going to scale in the long run. Everybody will just keep adding quirks to the initialization until it's become a huge mess. Tegra's XHCI driver initialization is quite a bit more complicated, mainly due to the need for external firmware and specific ordering (e.g. firmware messages should only be enabled after the HCD is created). I could do away with the xhci-plat sub-device and just create a Tegra hc_driver, but it seems silly to have three XHCI platform drivers structured in three different ways. USB folks, do you have an opinion on how this should be done? The tendency in other subsystems (and I think this is also true to some degree for USB, though I'm less familiar with it) is to make common functions available as a library of helpers so that other drivers can use them (either directly or by wrapping them in platform-specific implementations). That allows you to keep the platform-specific code where it belongs: in the platform-specific driver. Thierry pgpV_zC8AoU9b.pgp Description: PGP signature
[PATCH] USB: EHCI: tegra: Drop unused defines
From: Thierry Reding tred...@nvidia.com Since commit 2d22b42db02f usb: phy: registering Tegra USB PHY as platform driver the driver no longer relies on the hard-coded physical addresses to determine the association between PHY and EHCI port, so these defines can be dropped. Signed-off-by: Thierry Reding tred...@nvidia.com --- drivers/usb/host/ehci-tegra.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index af28b748e87a..27ac6ad53c3d 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -38,10 +38,6 @@ #include ehci.h -#define TEGRA_USB_BASE 0xC500 -#define TEGRA_USB2_BASE0xC5004000 -#define TEGRA_USB3_BASE0xC5008000 - #define PORT_WAKE_BITS (PORT_WKOC_E|PORT_WKDISC_E|PORT_WKCONN_E) #define TEGRA_USB_DMA_ALIGN 32 -- 1.8.4.2 -- 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] USB2NET : SR9800 : One chip USB2.0 USB2NET SR9800 Device Driver Support
On Mon, Feb 10, 2014 at 01:33:39PM +0800, liujunliang_...@163.com wrote: From: Liu Junliang liujunliang_...@163.com Signed-off-by: Liu Junliang liujunliang_...@163.com --- drivers/net/usb/Kconfig | 16 + drivers/net/usb/Makefile |1 + drivers/net/usb/sr9800.c | 873 ++ drivers/net/usb/sr9800.h | 202 +++ 4 files changed, 1092 insertions(+), 0 deletions(-) create mode 100644 drivers/net/usb/sr9800.c create mode 100644 drivers/net/usb/sr9800.h diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig index 47b0f73..2551bf6 100644 --- a/drivers/net/usb/Kconfig +++ b/drivers/net/usb/Kconfig @@ -291,6 +291,22 @@ config USB_NET_SR9700 This option adds support for CoreChip-sz SR9700 based USB 1.1 10/100 Ethernet adapters. +config USB_NET_SR9800 + tristate CoreChip-sz SR9800 based USB 2.0 10/100 ethernet devices + depends on USB_USBNET + select CRC32 + default y Why is this selected by default? I can see that some of the other USB network drivers are also selected by default, but not all of them. Is there some rule of thumb as to which should default to y and which shouldn't? Thierry pgpLkbJA6W7a4.pgp Description: PGP signature
Re: [PATCH] usb: phy-tegra-usb.c: wrong pointer check for remap UTMI
On Wed, Dec 04, 2013 at 10:02:44AM +0800, Chris Ruehl wrote: usb: phy-tegra-usb.c: wrong pointer check for remap UTMI A wrong pointer was used to test the result of devm_ioremap() Signed-off-by: Chris Ruehl chris.ru...@gtsys.com.hk Acked-by: Venu Byravarasu vbyravar...@nvidia.com --- drivers/usb/phy/phy-tegra-usb.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Thierry Reding tred...@nvidia.com pgpztN5vkmcF9.pgp Description: PGP signature
Re: [PATCH 27/31] USB: EHCI: tegra: use reset framework
On Fri, Nov 15, 2013 at 01:54:22PM -0700, Stephen Warren wrote: From: Stephen Warren swar...@nvidia.com Tegra's clock driver now provides an implementation of the common reset API (include/linux/reset.h). Use this instead of the old Tegra- specific API; that will soon be removed. Cc: tred...@nvidia.com Cc: pdeschrij...@nvidia.com Cc: linux-te...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Alan Stern st...@rowland.harvard.edu Cc: linux-usb@vger.kernel.org Signed-off-by: Stephen Warren swar...@nvidia.com --- This patch is part of a series with strong internal depdendencies. I'm looking for an ack so that I can take the entire series through the Tegra and arm-soc trees. The series will be part of a stable branch that can be merged into other subsystems if needed to avoid/resolve dependencies. --- drivers/usb/host/ehci-tegra.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) Reviewed-by: Thierry Reding tred...@nvidia.com pgpVJB4hRtMwz.pgp Description: PGP signature
Re: [PATCH 2/2] ARM: dts: USB for Tegra114 Dalmore
On Wed, Jul 31, 2013 at 05:29:40PM -0600, Stephen Warren wrote: On 07/31/2013 04:20 PM, Sergei Shtylyov wrote: On 08/01/2013 02:06 AM, Stephen Warren wrote: ... That's really horrible design. Yup. Both USB PHY and EHCI controller registers really are interleaved in one range. But the standard EHCI register space has no holes IIRC, so they can't be really that much interleaved as you're describing (unless you have some non-standard registers of course)... Yes, there are certainly non-standard registers. ... Don't they cause numerous resource conflicts while device nodes being instantiated as the platform devices? No; the driver knows that the HW is screwy and there's lots of register-range sharing going on, so it simply maps the registers, rather than reserving the physical address range and mapping it. Yes, it's clear that the driver should take special measures, I was asking about the platform device creation phase. What do you see in /proc/iomem? The drivers don't request the memory region since doing so would cause conflicts. Hence, the regions don't show up in /proc/iomem. This actually isn't that uncommon for DT-based drivers anyway; many use e.g. of_iomap() which IIRC just looks up the resource and maps it without registering the usage. Not being uncommon isn't a good argument. The problem with doing this is that it sets a bad example and makes it easier for others to do the same thing. I can see that for some drivers providing a proper abstraction or encapsulation might be more complicated than necessary. But I've also seen this kind of shortcut taken quite often lately and especially often in DT-based drivers. Am I the only one concerned about this development? Thierry pgpwwthg9BPHw.pgp Description: PGP signature
Re: [PATCH v2 1/2] ARM: DTS: tegra: Add USB entries for Tegra30
On Thu, Aug 01, 2013 at 06:39:10PM +0200, Stephen Warren wrote: On 08/01/2013 09:00 AM, Tuomas Tynkkynen wrote: Add device tree entries for the 3 USB controllers and PHYs and enable the third controller on Cardhu and Beaver boards. Fix VBUS regulator entries on Beaver. The GPIO pins were wrong. Also, internal pullups need to be enabled on those pins. Signed-off-by: Tuomas Tynkkynen ttynkky...@nvidia.com --- v2: Use internal pullups on the VBUS regulator GPIOs. Thanks, this version looks good. Thierry, can you please validate that the gpv group pull strength change doesn't have any negative affect on your PCIe patches. Thanks. PCIe on Beaver seems to behave the same way whether that patch is applied or not, so: Tested-by: Thierry Reding tred...@nvidia.com I wonder if perhaps a similar change can be made to Cardhu to see if that helps with the PCIe link disappearing. I'll see if I can find out what the implications are and what the correct values would be for Cardhu. Thierry pgp4948o23WYq.pgp Description: PGP signature
[PATCH] USB: EHCI: tegra: Fix oops in error cleanup
Under some circumstances it happens that the connected PHY can't be powered up properly, in which case the cleanup path currently crashes because it checks the tegra-transceiver field using !IS_ERR(), which will succeed because it is in fact NULL. Dereferencing that pointer causes an oops in tegra_ehci_probe(). This patch fixes the issue by adding an additional label into the cleanup path to separately take down the PHY and the transceiver. Signed-off-by: Thierry Reding thierry.red...@gmail.com --- Note: This is based on top of Stephen's latest patch series to allow building the driver as a module: [PATCH V3 REPOST 0/7] USB: tegra: support building as a module drivers/usb/host/ehci-tegra.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index 8dac5e4..6ee7ef7 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -478,15 +478,15 @@ static int tegra_ehci_probe(struct platform_device *pdev) err = usb_add_hcd(hcd, irq, IRQF_SHARED); if (err) { dev_err(pdev-dev, Failed to add USB HCD\n); - goto cleanup_phy; + goto cleanup_transceiver; } return err; -cleanup_phy: +cleanup_transceiver: if (!IS_ERR(tegra-transceiver)) otg_set_host(tegra-transceiver-otg, NULL); - +cleanup_phy: usb_phy_shutdown(hcd-phy); cleanup_clk_en: clk_disable_unprepare(tegra-clk); -- 1.8.2 -- 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 V3 REPOST 0/7] USB: tegra: support building as a module
On Thu, Jun 13, 2013 at 11:24:06AM -0600, Stephen Warren wrote: From: Stephen Warren swar...@nvidia.com I'm reposting this because I originally thought Felipe would apply it to his PHY tree, since it's based on other work there. Now that tree has been merged into Greg's main USB tree, I believe this series can be applied there instead. Hence, resending so it shows up in Greg's inbox. ehci-tegra is currently built into the main ehci-hcd driver, rather than being a separate module. This causes issues with multi-platform ARM kernels. This series separates ehci-tegra into its own module to avoid those problems. Manjunath Goudar originally wrote most of this series. I've since cleaned it up, rebased it on Venu's recent changes to the Tegra USB driver, and tested it. Manjunath Goudar (3): usb: phy: export ulpi_viewport_access_ops USB: EHCI: export ehci_handshake for ehci-hcd sub-drivers USB: EHCI: make ehci-tegra a separate driver Stephen Warren (4): usb: phy: add MODULE_LICENSE to phy-tegra-usb.c USB: EHCI: tegra: remove all power management USB: EHCI: tegra: fix circular module dependencies USB: EHCI: tegra: make use of ehci-priv drivers/usb/host/Kconfig| 2 +- drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-hcd.c | 22 +- drivers/usb/host/ehci-hub.c | 4 +- drivers/usb/host/ehci-tegra.c | 474 drivers/usb/host/ehci.h | 2 + drivers/usb/phy/phy-tegra-usb.c | 43 +++- drivers/usb/phy/phy-ulpi-viewport.c | 2 + include/linux/usb/tegra_usb_phy.h | 4 - 9 files changed, 165 insertions(+), 389 deletions(-) The series: Tested-by: Thierry Reding thierry.red...@gmail.com pgpAES0C1Gu9E.pgp Description: PGP signature
[PATCH v2] usb: host: ehci-tegra: Fix oops in error cleanup
The cleanup path checks whether the transceiver was properly initialized using IS_ERR(). However it can also happen that the cleanup path is run before the transceiver was initialized (or the operating mode isn't set to TEGRA_USB_OTG) and is therefore NULL. Add a separate label for error unwinding and initialize the transceiver field to ERR_PTR(-ENODEV) when the operating mode isn't TEGRA_USB_OTG to allow for consistent checking. Signed-off-by: Thierry Reding thie...@gilfi.de Acked-by: Stephen Warren swar...@nvidia.com Acked-by: Venu Byravarasu vbyravar...@nvidia.com --- Note that this has a conflict with Felipe's tree, but it should be trivial to solve. Changes in v2: - add Acked-bys from Stephen and Venu - rebase on Greg's usb-next branch --- drivers/usb/host/ehci-tegra.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c index 83d190a..4f3cfb8 100644 --- a/drivers/usb/host/ehci-tegra.c +++ b/drivers/usb/host/ehci-tegra.c @@ -760,7 +760,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) err = usb_phy_set_suspend(hcd-phy, 0); if (err) { dev_err(pdev-dev, Failed to power on the phy\n); - goto fail; + goto fail_phy; } tegra-host_resumed = 1; @@ -770,7 +770,7 @@ static int tegra_ehci_probe(struct platform_device *pdev) if (!irq) { dev_err(pdev-dev, Failed to get IRQ\n); err = -ENODEV; - goto fail; + goto fail_phy; } #ifdef CONFIG_USB_OTG_UTILS @@ -779,6 +779,8 @@ static int tegra_ehci_probe(struct platform_device *pdev) devm_usb_get_phy(pdev-dev, USB_PHY_TYPE_USB2); if (!IS_ERR_OR_NULL(tegra-transceiver)) otg_set_host(tegra-transceiver-otg, hcd-self); + } else { + tegra-transceiver = ERR_PTR(-ENODEV); } #endif @@ -803,6 +805,7 @@ fail: if (!IS_ERR_OR_NULL(tegra-transceiver)) otg_set_host(tegra-transceiver-otg, NULL); #endif +fail_phy: usb_phy_shutdown(hcd-phy); fail_io: clk_disable_unprepare(tegra-clk); -- 1.8.2 -- 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 1/3] usb: phy: samsung: Convert to devm_ioremap_resource()
On Mon, Mar 04, 2013 at 02:05:41PM +0530, Sachin Kamat wrote: Use the newly introduced devm_ioremap_resource() instead of devm_request_and_ioremap() which provides more consistent error handling. devm_ioremap_resource() provides its own error messages; so all explicit error messages can be removed from the failure code paths. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Reviewed-by: Thierry Reding thierry.red...@avionic-design.de pgpR2wexD4ouX.pgp Description: PGP signature
Re: [PATCH 2/3] usb: phy: omap-usb3: Convert to devm_ioremap_resource()
On Mon, Mar 04, 2013 at 02:05:42PM +0530, Sachin Kamat wrote: Use the newly introduced devm_ioremap_resource() instead of devm_request_and_ioremap() which provides more consistent error handling. devm_ioremap_resource() provides its own error messages; so all explicit error messages can be removed from the failure code paths. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Cc: Kishon Vijay Abraham I kis...@ti.com --- drivers/usb/phy/omap-usb3.c |8 +++- 1 files changed, 3 insertions(+), 5 deletions(-) Reviewed-by: Thierry Reding thierry.red...@avionic-design.de pgpECQm4gcqfC.pgp Description: PGP signature
Re: [PATCH 3/3] usb: phy: omap-control-usb: Convert to devm_ioremap_resource()
On Mon, Mar 04, 2013 at 02:05:43PM +0530, Sachin Kamat wrote: Use the newly introduced devm_ioremap_resource() instead of devm_request_and_ioremap() which provides more consistent error handling. devm_ioremap_resource() provides its own error messages; so all explicit error messages can be removed from the failure code paths. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Cc: Kishon Vijay Abraham I kis...@ti.com --- drivers/usb/phy/omap-control-usb.c | 24 +--- 1 files changed, 9 insertions(+), 15 deletions(-) Reviewed-by: Thierry Reding thierry.red...@avionic-design.de pgpY3qVaPsu4D.pgp Description: PGP signature
Re: [GIT PATCH] USB patches for 3.9-rc1
On Thu, Feb 21, 2013 at 01:58:39PM -0800, Greg KH wrote: On Thu, Feb 21, 2013 at 12:25:24PM -0800, Linus Torvalds wrote: On Thu, Feb 21, 2013 at 10:40 AM, Greg KH gre...@linuxfoundation.org wrote: USB patches for 3.9-rc1 Here's the big USB merge for 3.9-rc1 Nothing major, lots of gadget fixes, and of course, xhci stuff. Ok, so there were a couple of conflicts with Thierry Reding's series to convert devm_request_and_ioremap() users into devm_ioremap_resource(), where some of the old users had been converted to use other helper functions (eg omap_get_control_dev()). That's fine. I left the omap_get_control_dev() users alone, but I do want to note that omap_control_usb_probe() itself now uses that devm_request_and_ioremap() function. And I did *not* extend the merge to do that kind of conversion in the helper function, so I'm assuming Thierry might want to extend his work. Assuming people care enough.. Yes, his plan was to do another sweep of the calls and hopefully remove the old api in 3.10 or so once that is all cleaned up. Given that even devm_request_and_ioremap() is rather new and people have been busy sending patches to use it I had expected that the initial series wouldn't catch all uses once it had been merged. grepping is easy and I even have a semantic patch to help with the conversion so I'll keep an eye out for any new occurrences. Thierry pgpBB0r8hc_2i.pgp Description: PGP signature
[PATCH 29/33] usb: Convert to devm_ioremap_resource()
Convert all uses of devm_request_and_ioremap() to the newly introduced devm_ioremap_resource() which provides more consistent error handling. devm_ioremap_resource() provides its own error messages so all explicit error messages can be removed from the failure code paths. Signed-off-by: Thierry Reding thierry.red...@avionic-design.de Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Alan Stern st...@rowland.harvard.edu Cc: Felipe Balbi ba...@ti.com Cc: linux-usb@vger.kernel.org --- drivers/usb/chipidea/usbmisc_imx6q.c | 6 +++--- drivers/usb/gadget/bcm63xx_udc.c | 13 +++-- drivers/usb/gadget/s3c-hsotg.c | 7 +++ drivers/usb/gadget/s3c-hsudc.c | 7 +++ drivers/usb/host/ehci-atmel.c| 7 +++ drivers/usb/host/ehci-grlib.c| 9 - drivers/usb/host/ehci-mxc.c | 7 +++ drivers/usb/host/ehci-platform.c | 7 --- drivers/usb/host/ehci-ppc-of.c | 8 drivers/usb/host/ehci-sead3.c| 8 drivers/usb/host/ehci-sh.c | 7 +++ drivers/usb/host/ehci-vt8500.c | 8 drivers/usb/host/ehci-xilinx-of.c| 8 drivers/usb/host/ohci-nxp.c | 7 +++ drivers/usb/host/ohci-platform.c | 7 --- drivers/usb/host/ohci-s3c2410.c | 7 +++ drivers/usb/musb/musb_dsps.c | 7 +++ drivers/usb/musb/omap2430.c | 4 +--- drivers/usb/otg/mxs-phy.c| 6 +++--- drivers/usb/phy/mv_u3d_phy.c | 8 +++- drivers/usb/phy/omap-usb2.c | 8 +++- drivers/usb/renesas_usbhs/common.c | 9 - 22 files changed, 76 insertions(+), 89 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx6q.c b/drivers/usb/chipidea/usbmisc_imx6q.c index 845efe2..a1bce39 100644 --- a/drivers/usb/chipidea/usbmisc_imx6q.c +++ b/drivers/usb/chipidea/usbmisc_imx6q.c @@ -98,9 +98,9 @@ static int usbmisc_imx6q_probe(struct platform_device *pdev) spin_lock_init(data-lock); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - data-base = devm_request_and_ioremap(pdev-dev, res); - if (!data-base) - return -EADDRNOTAVAIL; + data-base = devm_ioremap_resource(pdev-dev, res); + if (IS_ERR(data-base)) + return PTR_ERR(data-base); data-clk = devm_clk_get(pdev-dev, NULL); if (IS_ERR(data-clk)) { diff --git a/drivers/usb/gadget/bcm63xx_udc.c b/drivers/usb/gadget/bcm63xx_udc.c index 47a4993..8cc8253 100644 --- a/drivers/usb/gadget/bcm63xx_udc.c +++ b/drivers/usb/gadget/bcm63xx_udc.c @@ -2351,19 +2351,20 @@ static int bcm63xx_udc_probe(struct platform_device *pdev) dev_err(dev, error finding USBD resource\n); return -ENXIO; } - udc-usbd_regs = devm_request_and_ioremap(dev, res); + + udc-usbd_regs = devm_ioremap_resource(dev, res); + if (IS_ERR(udc-usbd_regs)) + return PTR_ERR(udc-usbd_regs); res = platform_get_resource(pdev, IORESOURCE_MEM, 1); if (!res) { dev_err(dev, error finding IUDMA resource\n); return -ENXIO; } - udc-iudma_regs = devm_request_and_ioremap(dev, res); - if (!udc-usbd_regs || !udc-iudma_regs) { - dev_err(dev, error requesting resources\n); - return -ENXIO; - } + udc-iudma_regs = devm_ioremap_resource(dev, res); + if (IS_ERR(udc-iudma_regs)) + return PTR_ERR(udc-iudma_regs); spin_lock_init(udc-lock); INIT_WORK(udc-ep0_wq, bcm63xx_ep0_process); diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index 439c3f9..de80fa6 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -3525,10 +3525,9 @@ static int s3c_hsotg_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - hsotg-regs = devm_request_and_ioremap(pdev-dev, res); - if (!hsotg-regs) { - dev_err(dev, cannot map registers\n); - ret = -ENXIO; + hsotg-regs = devm_ioremap_resource(pdev-dev, res); + if (IS_ERR(hsotg-regs)) { + ret = PTR_ERR(hsotg-regs); goto err_clk; } diff --git a/drivers/usb/gadget/s3c-hsudc.c b/drivers/usb/gadget/s3c-hsudc.c index 52379b1..94ca33b 100644 --- a/drivers/usb/gadget/s3c-hsudc.c +++ b/drivers/usb/gadget/s3c-hsudc.c @@ -1295,10 +1295,9 @@ static int s3c_hsudc_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - hsudc-regs = devm_request_and_ioremap(pdev-dev, res); - if (!hsudc-regs) { - dev_err(dev, error mapping device register area\n); - ret = -EBUSY; + hsudc-regs = devm_ioremap_resource(pdev-dev, res); + if (IS_ERR(hsudc-regs)) { + ret = PTR_ERR(hsudc-regs); goto err_res; } diff --git