Re: [PATCH V3 RESEND] phy: bcm-ns-usb3: new driver for USB 3.0 PHY on Northstar
Hi Rafal, some comments in line. On 16-05-22 03:09 PM, Rafał Miłecki wrote: > Northstar is a family of SoCs used in home routers. They have USB 2.0 > and 3.0 controllers with PHYs that need to be properly initialized. > This driver provides PHY init support in a generic way and can be bound > with XHCI controller driver. > This patch adds 2 defines in bcma header that will be reused in bcma > driver. Using common header will allow avoiding code duplication. > > Signed-off-by: Rafał Miłecki > --- > V2: Redesign the driver to don't depend on bcma. This will allow reusing it on > Northstar+ which doesn't use bcma. This requires providing two different > registers ranges in DT which was documented in bindings info. > V3: Use readl and writel > Fix MODULE_LICENSE to match header (v2) > Mention C0 series in Documentation > RESEND: I can see that my PHY driver for USB 2.0: > [PATCH V4] phy: bcm-ns-usb2: new driver for USB 2.0 PHY on Northstar > sent on 14 Apr 2016 was accepted, however: > [PATCH V3] phy: bcm-ns-usb3: new driver for USB 3.0 PHY on Northstar > sent the very same day wasn't. > I'm sending a simply rebased version hoping it can be accepted or > commented somehow (e.g. what needs to be changed). > --- > .../devicetree/bindings/phy/bcm-ns-usb3-phy.txt| 23 ++ > drivers/phy/Kconfig| 9 + > drivers/phy/Makefile | 1 + > drivers/phy/phy-bcm-ns-usb3.c | 267 + > include/linux/bcma/bcma_driver_chipcommon.h| 3 + > 5 files changed, 303 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt > create mode 100644 drivers/phy/phy-bcm-ns-usb3.c > > diff --git a/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt b/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt > new file mode 100644 > index 000..09aeba9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt > @@ -0,0 +1,23 @@ > +Driver for Broadcom Northstar USB 3.0 PHY > + > +Required properties: > + > +- compatible: one of: "brcm,ns-ax-usb3-phy", "brcm,ns-bx-usb3-phy". > +- reg: register mappings for DMP (Device Management Plugin) and ChipCommon B > + MMI. > +- reg-names: "dmp" and "ccb-mii" > + > +Initialization of USB 3.0 PHY depends on Northstar version. There are currently > +three known series: Ax, Bx and Cx. > +Known A0: BCM4707 rev 0 > +Known B0: BCM4707 rev 4, BCM53573 rev 2 > +Known B1: BCM4707 rev 6 > +Known C0: BCM47094 rev 0 > + > +Example: > +usb3-phy { > +compatible = "brcm,ns-ax-usb3-phy"; > +reg = <0x18105000 0x1000>, <0x18003000 0x1000>; > +reg-names = "dmp", "ccb-mii"; > +#phy-cells = <0>; > +}; > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index f2b458f..6967398 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -24,6 +24,15 @@ config PHY_BCM_NS_USB2 > Enable this to support Broadcom USB 2.0 PHY connected to the USB > controller on Northstar family. > > +config PHY_BCM_NS_USB3 > +tristate "Broadcom Northstar USB 3.0 PHY Driver" > +depends on ARCH_BCM_IPROC || COMPILE_TEST > +depends on HAS_IOMEM && OF > +select GENERIC_PHY > +help > + Enable this to support Broadcom USB 3.0 PHY connected to the USB > + controller on Northstar family. > + > config PHY_BERLIN_USB > tristate "Marvell Berlin USB PHY Driver" > depends on ARCH_BERLIN && RESET_CONTROLLER && HAS_IOMEM && OF > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index 0de09e1..fa17ae3 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -4,6 +4,7 @@ > > obj-$(CONFIG_GENERIC_PHY)+= phy-core.o > obj-$(CONFIG_PHY_BCM_NS_USB2)+= phy-bcm-ns-usb2.o > +obj-$(CONFIG_PHY_BCM_NS_USB3)+= phy-bcm-ns-usb3.o > obj-$(CONFIG_PHY_BERLIN_USB)+= phy-berlin-usb.o > obj-$(CONFIG_PHY_BERLIN_SATA)+= phy-berlin-sata.o > obj-$(CONFIG_PHY_DM816X_USB)+= phy-dm816x-usb.o > diff --git a/drivers/phy/phy-bcm-ns-usb3.c b/drivers/phy/phy-bcm-ns-usb3.c > new file mode 100644 > index 000..869bc20 > --- /dev/null > +++ b/drivers/phy/phy-bcm-ns-usb3.c > @@ -0,0 +1,267 @@ > +/* > + * Broadcom Northstar USB 3.0 PHY Driver > + * > + * Copyright (C) 2016 Rafał Miłecki My thought is this code must have originated from Broadcom source code. Where is the copyright notice/license from the original code? > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define BCM_NS_USB3_MII_MNG_TIMEOUT_US1000/* usecs */ > + > +enum bcm_ns_family { > +BCM_NS_U
Re: [RFC v2 4/5] DT bindings documentation for Synopsys UDC platform driver
Hi Rob, On 17-01-19 09:36 AM, Rob Herring wrote: On Tue, Jan 17, 2017 at 01:35:07PM +0530, Raviteja Garimella wrote: This patch adds device tree bindings documentation for Synopsys USB device controller platform driver. Bindings describe h/w, not drivers. Signed-off-by: Raviteja Garimella --- .../devicetree/bindings/usb/snps,dw-ahb-udc.txt| 27 ++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt diff --git a/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt new file mode 100644 index 000..0c18327 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt @@ -0,0 +1,27 @@ +Synopsys USB Device controller. + +The device node is used for Synopsys Designware Cores AHB +Subsystem Device Controller (UDC). + +This device node is used by UDCs integrated it Broadcom's +Northstar2 and Cygnus SoC's. You need compatible strings for these in addition. We don't need compatibility strings when an IP block is integrated into an SoC. Otherwise each time we add the IP block to a new SoC we would need to update ever linux driver that supports that SoC. That doesn't make sense? Cygnus and Northstar2 use existing drivers for such block as UARTs, SPI controllers, NAND controllers, etc, etc. We haven't added compatibility strings for those drivers and won't be. Perhaps comment above can be: This device node is used by UDCs integrated it such as Broadcom's Northstar2 and Cygnus SoC's. + +Required properties: + - compatible: should be "snps,dw-ahb-udc" This is a different IP than DWC2? + - reg: Offset and length of UDC register set + - interrupts: description of interrupt line + - phys: phandle to phy node. + - extcon: phandle to the extcon device. This is optional and + not required for those that don't require extcon support. + Extcon support will be required if the UDC is connected to + a Dual Role Device Phy that supports both Host and Device + mode based on the external cable. Drop this. It should be a part of the phy. Also, I don't care to see new users of extcon binding because it needs redoing. We may need extcon support to support DRD Phy though. We have to work within the framework that exists in linux today. If modified in the future adapt to it as needed? Regards, Scott -- 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: [RFC v2 4/5] DT bindings documentation for Synopsys UDC platform driver
Hi Florian, On 17-01-19 11:40 AM, Florian Fainelli wrote: On 01/19/2017 11:30 AM, Scott Branden wrote: Hi Rob, On 17-01-19 09:36 AM, Rob Herring wrote: On Tue, Jan 17, 2017 at 01:35:07PM +0530, Raviteja Garimella wrote: This patch adds device tree bindings documentation for Synopsys USB device controller platform driver. Bindings describe h/w, not drivers. Signed-off-by: Raviteja Garimella --- .../devicetree/bindings/usb/snps,dw-ahb-udc.txt| 27 ++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt diff --git a/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt new file mode 100644 index 000..0c18327 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt @@ -0,0 +1,27 @@ +Synopsys USB Device controller. + +The device node is used for Synopsys Designware Cores AHB +Subsystem Device Controller (UDC). + +This device node is used by UDCs integrated it Broadcom's +Northstar2 and Cygnus SoC's. You need compatible strings for these in addition. We don't need compatibility strings when an IP block is integrated into an SoC. Otherwise each time we add the IP block to a new SoC we would need to update ever linux driver that supports that SoC. That doesn't make sense? You probably do need such a thing, here is how the compatible strings for IP blocks integrated into SoCs could be used: - provide a compatible strings which describes exactly the integration of this peripheral into a given SoC, e.g: brcm,udc-ns2, the reason for that is that you want to be able to capture the specific IP block integration into a specific SoC and all its quirks - if the block has its own revision scheme (and it can be relied on), provide it: brcm,udc-v1.2 and that is probably the most meaningful compatible string for a client program here - have a some kind of fallback/catchall compatible string that describes the block: brcm,udc which may also work just fine, although is not preferred Defining compatible strings is meant to avoid making (possibly incompatible) Device Tree binding changes in the future, and you always have the liberty as a client program (OS, bootloader) to match only the compatible strings you care about, from the most specific (which includes the exact SoC) to the least specific. The key thing is that, if the full set of compatible strings are present and available, you can retroactively fix your driver to be more specific, very much less so your Device Tree blob (although there is disagreement). The driver stands alone from the SoC and does not need compatibility strings per SoC. New SoCs will use the exact same block. We don't add compatibility strings to any other drivers when we add the same block to a new SoC. Yes, if the version of the IP changes then a version or feature compatibility string is added to the driver. -- 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: [RFC v2 4/5] DT bindings documentation for Synopsys UDC platform driver
On 17-01-19 01:55 PM, Ray Jui wrote: On 1/19/2017 12:17 PM, Florian Fainelli wrote: On 01/19/2017 12:07 PM, Scott Branden wrote: Hi Florian, On 17-01-19 11:40 AM, Florian Fainelli wrote: On 01/19/2017 11:30 AM, Scott Branden wrote: Hi Rob, On 17-01-19 09:36 AM, Rob Herring wrote: On Tue, Jan 17, 2017 at 01:35:07PM +0530, Raviteja Garimella wrote: This patch adds device tree bindings documentation for Synopsys USB device controller platform driver. Bindings describe h/w, not drivers. Signed-off-by: Raviteja Garimella --- .../devicetree/bindings/usb/snps,dw-ahb-udc.txt| 27 ++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt diff --git a/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt new file mode 100644 index 000..0c18327 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/snps,dw-ahb-udc.txt @@ -0,0 +1,27 @@ +Synopsys USB Device controller. + +The device node is used for Synopsys Designware Cores AHB +Subsystem Device Controller (UDC). + +This device node is used by UDCs integrated it Broadcom's +Northstar2 and Cygnus SoC's. You need compatible strings for these in addition. We don't need compatibility strings when an IP block is integrated into an SoC. Otherwise each time we add the IP block to a new SoC we would need to update ever linux driver that supports that SoC. That doesn't make sense? You probably do need such a thing, here is how the compatible strings for IP blocks integrated into SoCs could be used: - provide a compatible strings which describes exactly the integration of this peripheral into a given SoC, e.g: brcm,udc-ns2, the reason for that is that you want to be able to capture the specific IP block integration into a specific SoC and all its quirks - if the block has its own revision scheme (and it can be relied on), provide it: brcm,udc-v1.2 and that is probably the most meaningful compatible string for a client program here - have a some kind of fallback/catchall compatible string that describes the block: brcm,udc which may also work just fine, although is not preferred Defining compatible strings is meant to avoid making (possibly incompatible) Device Tree binding changes in the future, and you always have the liberty as a client program (OS, bootloader) to match only the compatible strings you care about, from the most specific (which includes the exact SoC) to the least specific. The key thing is that, if the full set of compatible strings are present and available, you can retroactively fix your driver to be more specific, very much less so your Device Tree blob (although there is disagreement). The driver stands alone from the SoC and does not need compatibility strings per SoC. New SoCs will use the exact same block. Even if you take the exact same block and put it in a different SoC, that's still an integration work that 99% of the time goes just fine because the validation worked great, and the 1% of the time where you need to capture an integration bug, you are glad this SoC-specific compatible string exists such that you can work around it in the driver. That's a very conservative estimate. Based on my experience, it's more like 50/50, i.e., roughly half of the time we found SoC integration specific quirks or workaround are needed. 50% is an exaggeration for sure. Maybe a driver you are has that issue but that is not the case with most drivers. We have many IP blocks in the SoC - both internal and externally sourced IP. We integrate SP805 timer driver into many SoCs and never specify a SoC specific compatibility string with it (nor should we). That being said - if your driver needs to know SoC specifics is should not need to have an SoC specific compatibility string added per driver. Why can your driver just not query that information from the upper level SoC specific info already present in device tree? Each SoC is already specified in device tree at the upper level already. Example: arch/arm/boot/dts/bcm7445.dtsi has this compatibility info already present in its device tree: compatible = "brcm,bcm7445", "brcm,brcmstb"; If needed, a driver should query this info rather than adding SoC specific compatibility strings to every single device tree entry. We should only add driver revision numbers as needed, not SoC specific names. That way drivers don't change when the (same revision) of the IP block is added to a new SoCs. And then if a SoC specific workaround is needed the upper level compatibility string can be queried should be utilized. It already exists today and is available for use to all drivers. One way to solve that is to use SoC specific compatible strings because that presents itself as a self-contained and standardized way, or you can have your driver cal
Re: [PATCH v2 5/6] DT bindings documentation for Broadcom IPROC USB Device controller.
Hi Raviteja, updates inline On 17-02-21 03:43 AM, Raviteja Garimella wrote: The device node is used for UDCs integrated into Broadcom's iProc family of SoCs'. The UDC is based on Synopsys Designware Cores AHB Subsystem USB Device Controller IP. Signed-off-by: Raviteja Garimella --- Documentation/devicetree/bindings/usb/iproc-udc.txt | 21 + 1 file changed, 21 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/iproc-udc.txt diff --git a/Documentation/devicetree/bindings/usb/iproc-udc.txt b/Documentation/devicetree/bindings/usb/iproc-udc.txt new file mode 100644 index 000..c2ce803 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/iproc-udc.txt @@ -0,0 +1,21 @@ +Broadcom IPROC USB Device controller. + +The device node is used for UDCs integrated into Broadcom's +iProc family (Northstar2, Cygnus) of SoCs'. The UDC is based +on Synopsys Designware Cores AHB Subsystem Device Controller +IP. + +Required properties: + - compatible: Add the compatibility strings for supported platforms. + For Broadcom NS2 platform, add "brcm,ns2-udc". For Broadcom NS2 platform, add "brcm,ns2-udc", "brcm,iproc-udc". + For Broadcom Cygnus platform, add "brcm,cygnus-udc". For Broadcom Cygnus platform, add "brcm,cygnus-udc", "brcm,iproc-udc". + - reg: Offset and length of UDC register set + - interrupts: description of interrupt line + - phys: phandle to phy node. + +Example: + udc_dwc: usb@664e { + compatible = "brcm,ns2-udc", "brcm,cygnus-udc"; compatible = "brcm,ns2-udc", "brcm,iproc-udc"; + reg = <0x664e 0x2000>; + interrupts = ; + phys = <&usbdrd_phy>; Regards, Scott -- 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] UDC: Add Synopsys UDC Platform driver
Hi Raviteja, One compatibility string to add. Comment inline On 17-02-21 03:43 AM, Raviteja Garimella wrote: This patch adds platform driver support for Synopsys UDC. A new driver file (snps_udc_plat.c) is created for this purpose where the platform driver registration is done based on OF node. Currently, UDC integrated into Broadcom's iProc SoCs (Northstar2 and Cygnus) work with this driver. New members are added to the UDC data structure for having platform device support along with extcon and phy support. Kconfig and Makefiles are modified to select platform driver for compilation. Signed-off-by: Raviteja Garimella --- drivers/usb/gadget/udc/Kconfig | 16 +- drivers/usb/gadget/udc/Makefile| 1 + drivers/usb/gadget/udc/amd5536udc.h| 14 ++ drivers/usb/gadget/udc/snps_udc_core.c | 54 -- drivers/usb/gadget/udc/snps_udc_plat.c | 343 + 5 files changed, 408 insertions(+), 20 deletions(-) create mode 100644 drivers/usb/gadget/udc/snps_udc_plat.c diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index 72e6234..5bc3305 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -240,7 +240,7 @@ config USB_MV_U3D controller, which support super speed USB peripheral. config USB_SNP_CORE - depends on USB_AMD5536UDC + depends on (USB_SNP_UDC_PLAT || USB_AMD5536UDC) depends on PCI tristate help @@ -254,6 +254,20 @@ config USB_SNP_CORE This IP is different to the High Speed OTG IP that can be enabled by selecting USB_DWC2 or USB_DWC3 options. +config USB_SNP_UDC_PLAT + tristate "Synopsys USB 2.0 Device controller" + depends on (USB_GADGET && OF) + select USB_GADGET_DUALSPEED + select USB_SNP_CORE + default ARCH_BCM_IPROC + help + This adds Platform Device support for Synopsys Designware core + AHB subsystem USB2.0 Device Controller (UDC). + + This driver works with UDCs integrated into Broadcom's Northstar2 + and Cygnus SoCs. + + If unsure, say N. # # Controllers available in both integrated and discrete versions # diff --git a/drivers/usb/gadget/udc/Makefile b/drivers/usb/gadget/udc/Makefile index 4f4fd62..ea9e1c7 100644 --- a/drivers/usb/gadget/udc/Makefile +++ b/drivers/usb/gadget/udc/Makefile @@ -37,4 +37,5 @@ obj-$(CONFIG_USB_FOTG210_UDC) += fotg210-udc.o obj-$(CONFIG_USB_MV_U3D) += mv_u3d_core.o obj-$(CONFIG_USB_GR_UDC) += gr_udc.o obj-$(CONFIG_USB_GADGET_XILINX)+= udc-xilinx.o +obj-$(CONFIG_USB_SNP_UDC_PLAT) += snps_udc_plat.o obj-$(CONFIG_USB_BDC_UDC) += bdc/ diff --git a/drivers/usb/gadget/udc/amd5536udc.h b/drivers/usb/gadget/udc/amd5536udc.h index c252457..7884281 100644 --- a/drivers/usb/gadget/udc/amd5536udc.h +++ b/drivers/usb/gadget/udc/amd5536udc.h @@ -16,6 +16,7 @@ /* debug control */ /* #define UDC_VERBOSE */ +#include #include #include @@ -28,6 +29,9 @@ #define UDC_HSA0_REV 1 #define UDC_HSB1_REV 2 +/* Broadcom chip rev. */ +#define UDC_BCM_REV 10 + /* * SETUP usb commands * needed, because some SETUP's are handled in hw, but must be passed to @@ -112,6 +116,7 @@ #define UDC_DEVCTL_BRLEN_MASK 0x00ff #define UDC_DEVCTL_BRLEN_OFS 16 +#define UDC_DEVCTL_SRX_FLUSH 14 #define UDC_DEVCTL_CSR_DONE13 #define UDC_DEVCTL_DEVNAK 12 #define UDC_DEVCTL_SD 10 @@ -564,7 +569,15 @@ struct udc { u16 cur_intf; u16 cur_alt; + /* for platform device and extcon support */ struct device *dev; + struct phy *udc_phy; + struct extcon_dev *edev; + struct extcon_specific_cable_nb extcon_nb; + struct notifier_block nb; + struct delayed_work drd_work; + struct workqueue_struct *drd_wq; + u32 conn_type; }; #define to_amd5536_udc(g) (container_of((g), struct udc, gadget)) @@ -580,6 +593,7 @@ int udc_enable_dev_setup_interrupts(struct udc *dev); int udc_mask_unused_interrupts(struct udc *dev); irqreturn_t udc_irq(int irq, void *pdev); void gadget_release(struct device *pdev); +void empty_req_queue(struct udc_ep *ep); void udc_basic_init(struct udc *dev); void free_dma_pools(struct udc *dev); int init_dma_pools(struct udc *dev); diff --git a/drivers/usb/gadget/udc/snps_udc_core.c b/drivers/usb/gadget/udc/snps_udc_core.c index 5f95a65..98de074 100644 --- a/drivers/usb/gadget/udc/snps_udc_core.c +++ b/drivers/usb/gadget/udc/snps_udc_core.c @@ -41,7 +41,6 @@ #include "amd5536udc.h" static void udc_tasklet_disconnect(unsigned long); -static void empty_req_queue(struct udc_ep *); static void udc_setup_endpoints(struct udc *dev); static vo
Re: [PATCH 1/2] usb: ohci-platform: add support for multiple phys per controller
On 14-12-05 01:28 PM, arun.ramamur...@broadcom.com wrote: From: Arun Ramamurthy Added support for cases where one controller is connected to multiple phys Signed-off-by: Arun Ramamurthy Reviewed-by: Ray Jui Reviewed-by: Scott Branden --- drivers/usb/host/ohci-platform.c | 70 k 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c index 4369299..eef82f1 100644 --- a/drivers/usb/host/ohci-platform.c +++ b/drivers/usb/host/ohci-platform.c @@ -38,7 +38,8 @@ struct ohci_platform_priv { struct clk *clks[OHCI_MAX_CLKS]; struct reset_control *rst; - struct phy *phy;x + struct phy *phys; nice little vi error above. We need a new patchset with the x removed. + int num_phys; }; static const char hcd_name[] = "ohci-platform"; @@ -61,7 +62,7 @@ static int ohci_platform_power_on(struct platform_device *dev) { struct usb_hcd *hcd = platform_get_drvdata(dev); struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd); - int clk, ret; + int clk, ret, phy_num; for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) { ret = clk_prepare_enable(priv->clks[clk]); @@ -69,20 +70,28 @@ static int ohci_platform_power_on(struct platform_device *dev) goto err_disable_clks; } - if (priv->phy) { - ret = phy_init(priv->phy); - if (ret) - goto err_disable_clks; - - ret = phy_power_on(priv->phy); - if (ret) + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { + ret = phy_init(&priv->phys[phy_num]); + if (ret) { + if (phy_num == 0) + goto err_disable_clks; + else + goto err_exit_phy; + } + ret = phy_power_on(&priv->phys[phy_num]); + if (ret) { + phy_exit(&priv->phys[phy_num]); goto err_exit_phy; + } } return 0; err_exit_phy: - phy_exit(priv->phy); + while (--phy_num >= 0) { + phy_power_off(&priv->phys[phy_num]); + phy_exit(&priv->phys[phy_num]); + }; err_disable_clks: while (--clk >= 0) clk_disable_unprepare(priv->clks[clk]); @@ -94,11 +103,11 @@ static void ohci_platform_power_off(struct platform_device *dev) { struct usb_hcd *hcd = platform_get_drvdata(dev); struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd); - int clk; + int clk, phy_num; - if (priv->phy) { - phy_power_off(priv->phy); - phy_exit(priv->phy); + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { + phy_power_off(&priv->phys[phy_num]); + phy_exit(&priv->phys[phy_num]); } for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--) @@ -127,7 +136,9 @@ static int ohci_platform_probe(struct platform_device *dev) struct usb_ohci_pdata *pdata = dev_get_platdata(&dev->dev); struct ohci_platform_priv *priv; struct ohci_hcd *ohci; - int err, irq, clk = 0; + struct phy *temp_phy; + const char *phy_name; + int err, irq, phy_num, clk = 0; if (usb_disabled()) return -ENODEV; @@ -175,12 +186,29 @@ static int ohci_platform_probe(struct platform_device *dev) if (of_property_read_bool(dev->dev.of_node, "big-endian")) ohci->flags |= OHCI_QUIRK_BE_MMIO | OHCI_QUIRK_BE_DESC; - priv->phy = devm_phy_get(&dev->dev, "usb"); - if (IS_ERR(priv->phy)) { - err = PTR_ERR(priv->phy); - if (err == -EPROBE_DEFER) - goto err_put_hcd; - priv->phy = NULL; + priv->num_phys = of_count_phandle_with_args + (dev->dev.of_node, "phys", "#phy-cells"); + if (priv->num_phys > 0) { + priv->phys = devm_kcalloc(&dev->dev, priv->num_phys, + sizeof(struct phy), GFP_KERNEL); + for (phy_num = 0; phy_num < priv->num_phys; phy_num++) { + err = of_property_read_string_index( + dev->dev.of_node, + "phy-names", phy_num, + &phy_name); +
[PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
From: Roman Bacik USB OTG driver in isochronous mode has to set the parity of the receiving microframe. The parity is set to even by default. This causes problems for an audio gadget, if the host starts transmitting on odd microframes. This fix uses Incomplete Periodic Transfer interrupt to toggle between even and odd parity until the Transfer Complete interrupt is received. Signed-off-by: Roman Bacik Reviewed-by: Abhinav Ratna Reviewed-by: Srinath Mannam Reviewed-by: Scott Branden Signed-off-by: Scott Branden --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 48 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0ed87620..954d1cd 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -150,6 +150,7 @@ struct s3c_hsotg_ep { unsigned intperiodic:1; unsigned intisochronous:1; unsigned intsend_zlp:1; + unsigned intparity_set:1; charname[10]; }; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4d47b7c..28e4393 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1954,6 +1954,8 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx, ints &= ~DXEPINT_XFERCOMPL; if (ints & DXEPINT_XFERCOMPL) { + if (hs_ep->isochronous && !hs_ep->parity_set) + hs_ep->parity_set = 1; if (hs_ep->isochronous && hs_ep->interval == 1) { if (ctrl & DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR; @@ -2316,7 +2318,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST | GINTSTS_RESETDET | GINTSTS_ENUMDONE | GINTSTS_OTGINT | GINTSTS_USBSUSP | - GINTSTS_WKUPINT, + GINTSTS_WKUPINT | + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTMSK); if (using_dma(hsotg)) @@ -2581,6 +2584,48 @@ irq_retry: s3c_hsotg_dump(hsotg); } + if (gintsts & GINTSTS_INCOMPL_SOIN) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOIN\n", __func__); + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg->eps_in[idx]; + if (hs_ep->isochronous && !hs_ep->parity_set) { + u32 epctl_reg = DIEPCTL(idx); + u32 ctrl = readl(hsotg->regs + epctl_reg); + + if (ctrl & DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg->regs + epctl_reg); + } + } + writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS); + } + + if (gintsts & GINTSTS_INCOMPL_SOOUT) { + u32 idx; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOOUT\n", __func__); + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg->eps_out[idx]; + if (hs_ep->isochronous && !hs_ep->parity_set) { + u32 epctl_reg = DOEPCTL(idx); + u32 ctrl = readl(hsotg->regs + epctl_reg); + + if (ctrl & DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg->regs + epctl_reg); + } + } + writel(GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTSTS); + } + /* * if we've had fifo events, we should try and go around the * loop again to see if there's any point in returning yet. @@ -2667,6 +2712,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep, hs_ep->periodic = 0; hs_ep->halted = 0; hs_ep->interval = desc->bInterval; + hs_ep->parity_set = 0; if (hs_ep->interval > 1 && hs_ep->mc > 1) dev_err(hsotg->dev, "MC > 1 when interval is not 1\n"); diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h index d0a5ed8..553f246 100644 --- a/drivers/usb
[PATCH 0/1] USB DWC2 parity fix in isochronous mode
This patch contains a fix for a real world interop problem found when using the Synopsis DWC2 USB controller with isochronous audio as detailed in the commit message. Roman Bacik (1): usb: dwc2: gadget: parity fix in isochronous mode drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 48 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) -- 2.4.6 -- 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 1/1] usb: dwc2: gadget: parity fix in isochronous mode
From: Roman Bacik USB OTG driver in isochronous mode has to set the parity of the receiving microframe. The parity is set to even by default. This causes problems for an audio gadget, if the host starts transmitting on odd microframes. This fix uses Incomplete Periodic Transfer interrupt to toggle between even and odd parity until the Transfer Complete interrupt is received. Signed-off-by: Roman Bacik Reviewed-by: Abhinav Ratna Reviewed-by: Srinath Mannam Signed-off-by: Scott Branden --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 51 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0ed87620..a5634fd 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -150,6 +150,7 @@ struct s3c_hsotg_ep { unsigned intperiodic:1; unsigned intisochronous:1; unsigned intsend_zlp:1; + unsigned inthas_correct_parity:1; charname[10]; }; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4d47b7c..fac3e2f 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1954,6 +1954,7 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx, ints &= ~DXEPINT_XFERCOMPL; if (ints & DXEPINT_XFERCOMPL) { + hs_ep->has_correct_parity = 1; if (hs_ep->isochronous && hs_ep->interval == 1) { if (ctrl & DXEPCTL_EOFRNUM) ctrl |= DXEPCTL_SETEVENFR; @@ -2316,7 +2317,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST | GINTSTS_RESETDET | GINTSTS_ENUMDONE | GINTSTS_OTGINT | GINTSTS_USBSUSP | - GINTSTS_WKUPINT, + GINTSTS_WKUPINT | + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTMSK); if (using_dma(hsotg)) @@ -2581,6 +2583,52 @@ irq_retry: s3c_hsotg_dump(hsotg); } + if (gintsts & GINTSTS_INCOMPL_SOIN) { + u32 idx, epctl_reg, ctrl; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOIN\n", __func__); + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg->eps_in[idx]; + + if (!hs_ep->isochronous || hs_ep->has_correct_parity) + continue; + + epctl_reg = DIEPCTL(idx); + ctrl = readl(hsotg->regs + epctl_reg); + + if (ctrl & DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg->regs + epctl_reg); + } + writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS); + } + + if (gintsts & GINTSTS_INCOMPL_SOOUT) { + u32 idx, epctl_reg, ctrl; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOOUT\n", __func__); + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) { + hs_ep = hsotg->eps_out[idx]; + + if (!hs_ep->isochronous || hs_ep->has_correct_parity) + continue; + + epctl_reg = DOEPCTL(idx); + ctrl = readl(hsotg->regs + epctl_reg); + + if (ctrl & DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg->regs + epctl_reg); + } + writel(GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTSTS); + } + /* * if we've had fifo events, we should try and go around the * loop again to see if there's any point in returning yet. @@ -2667,6 +2715,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep, hs_ep->periodic = 0; hs_ep->halted = 0; hs_ep->interval = desc->bInterval; + hs_ep->has_correct_parity = 0; if (hs_ep->interval > 1 && hs_ep->mc > 1) dev_err(hsotg->dev, "MC > 1 when interval is not 1\n"); diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h index d0a5ed8..553f246 100644 --- a/drivers/usb/dwc2/hw.h +++ b/drivers/usb/dwc2/hw.h @@ -142,6 +142,7 @@ #define GINTSTS_RESETDET (1 << 23) #define GINTSTS_FET_SUSP
[PATCH v2 0/1] USB DWC2 parity fix in isochronous mode
This patch contains a fix for a real world interop problem found when using the Synopsis DWC2 USB controller with isochronous audio as detailed in the commit message. Changes from v1: - Address code review comments as per previous responses: - renamed parity_set to has_correct_parity and reorder some logic Roman Bacik (1): usb: dwc2: gadget: parity fix in isochronous mode drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 51 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 52 insertions(+), 1 deletion(-) -- 2.5.0 -- 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 v3 1/1] usb: dwc2: gadget: parity fix in isochronous mode
From: Roman Bacik USB OTG driver in isochronous mode has to set the parity of the receiving microframe. The parity is set to even by default. This causes problems for an audio gadget, if the host starts transmitting on odd microframes. This fix uses Incomplete Periodic Transfer interrupt to toggle between even and odd parity until the Transfer Complete interrupt is received. Signed-off-by: Roman Bacik Reviewed-by: Abhinav Ratna Reviewed-by: Srinath Mannam Signed-off-by: Scott Branden --- drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 58 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 0ed87620..a5634fd 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -150,6 +150,7 @@ struct s3c_hsotg_ep { unsigned intperiodic:1; unsigned intisochronous:1; unsigned intsend_zlp:1; + unsigned inthas_correct_parity:1; charname[10]; }; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 4d47b7c..efaab76 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1916,6 +1916,19 @@ static void s3c_hsotg_complete_in(struct dwc2_hsotg *hsotg, s3c_hsotg_complete_request(hsotg, hs_ep, hs_req, 0); } +static void s3c_hsotg_change_ep_iso_parity(struct dwc2_hsotg *hsotg, + u32 epctl_reg) +{ + u32 ctrl; + + ctrl = readl(hsotg->regs + epctl_reg); + if (ctrl & DXEPCTL_EOFRNUM) + ctrl |= DXEPCTL_SETEVENFR; + else + ctrl |= DXEPCTL_SETODDFR; + writel(ctrl, hsotg->regs + epctl_reg); +} + /** * s3c_hsotg_epint - handle an in/out endpoint interrupt * @hsotg: The driver state @@ -1954,12 +1967,9 @@ static void s3c_hsotg_epint(struct dwc2_hsotg *hsotg, unsigned int idx, ints &= ~DXEPINT_XFERCOMPL; if (ints & DXEPINT_XFERCOMPL) { + hs_ep->has_correct_parity = 1; if (hs_ep->isochronous && hs_ep->interval == 1) { - if (ctrl & DXEPCTL_EOFRNUM) - ctrl |= DXEPCTL_SETEVENFR; - else - ctrl |= DXEPCTL_SETODDFR; - writel(ctrl, hsotg->regs + epctl_reg); + s3c_hsotg_change_ep_iso_parity(hsotg, epctl_reg); } dev_dbg(hsotg->dev, @@ -2316,7 +2326,8 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST | GINTSTS_RESETDET | GINTSTS_ENUMDONE | GINTSTS_OTGINT | GINTSTS_USBSUSP | - GINTSTS_WKUPINT, + GINTSTS_WKUPINT | + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTMSK); if (using_dma(hsotg)) @@ -2581,6 +2592,40 @@ irq_retry: s3c_hsotg_dump(hsotg); } + if (gintsts & GINTSTS_INCOMPL_SOIN) { + u32 idx, epctl_reg; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOIN\n", __func__); + for (idx = 1; idx < hsotg->num_of_eps; idx++) { + hs_ep = hsotg->eps_in[idx]; + + if (!hs_ep->isochronous || hs_ep->has_correct_parity) + continue; + + epctl_reg = DIEPCTL(idx); + s3c_hsotg_change_ep_iso_parity(hsotg, epctl_reg); + } + writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS); + } + + if (gintsts & GINTSTS_INCOMPL_SOOUT) { + u32 idx, epctl_reg; + struct s3c_hsotg_ep *hs_ep; + + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOOUT\n", __func__); + for (idx = 1; idx < hsotg->num_of_eps; idx++) { + hs_ep = hsotg->eps_out[idx]; + + if (!hs_ep->isochronous || hs_ep->has_correct_parity) + continue; + + epctl_reg = DOEPCTL(idx); + s3c_hsotg_change_ep_iso_parity(hsotg, epctl_reg); + } + writel(GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTSTS); + } + /* * if we've had fifo events, we should try and go around the * loop again to see if there's any point in returning yet. @@ -2667,6 +2712,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep, hs_ep->periodic = 0; hs_ep->halted = 0; hs_ep->interval = desc->bInterval; + hs_ep->has_correct_parity = 0; if (hs_ep->interval &
[PATCH v3 0/1] USB DWC2 parity fix in isochronous mode
This patch contains a fix for a real world interop problem found when using the Synopsis DWC2 USB controller with isochronous audio as detailed in the commit message. Changes from v2: - created s2c_hsotg_chage_ep_iso_parity function to call function in 3 places in code - used hsotg->num_of_eps instead of MAX_EPS_CHANNELS Changes from v1: - Address code review comments as per previous responses: - renamed parity_set to has_correct_parity and reorder some logic Roman Bacik (1): usb: dwc2: gadget: parity fix in isochronous mode drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 58 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 54 insertions(+), 6 deletions(-) -- 2.5.0 -- 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 0/1] USB DWC2 parity fix in isochronous mode
Hi John, Could you please review the v3 Patch. I believe we have address all of your comments? On 15-09-10 06:13 PM, Scott Branden wrote: This patch contains a fix for a real world interop problem found when using the Synopsis DWC2 USB controller with isochronous audio as detailed in the commit message. Changes from v2: - created s2c_hsotg_chage_ep_iso_parity function to call function in 3 places in code - used hsotg->num_of_eps instead of MAX_EPS_CHANNELS Changes from v1: - Address code review comments as per previous responses: - renamed parity_set to has_correct_parity and reorder some logic Roman Bacik (1): usb: dwc2: gadget: parity fix in isochronous mode drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 58 ++- drivers/usb/dwc2/hw.h | 1 + 3 files changed, 54 insertions(+), 6 deletions(-) -- 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 0/1] USB DWC2 parity fix in isochronous mode
On 15-09-23 01:18 AM, John Youn wrote: On 9/23/2015 1:11 AM, John Youn wrote: On 9/23/2015 12:36 AM, Scott Branden wrote: Hi John, Could you please review the v3 Patch. I believe we have address all of your comments? Yes I've been meaning to test it on our platforms. I should be able to get to it tomorrow. Also if you could rebase off of Felipe's testing/next branch it would make it easier when it comes time to merge it in. Could you point me at Felipe's git repo as I am unfamiliar with the location? Thanks, Scott Regards, John -- 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