[PATCH v4 2/3] USB3/DWC3: Add property "snps,incr-burst-type-adjustment" for INCR burst type
Property "snps,incr-burst-type-adjustment = , ..." for USB3.0 DWC3.
When Just one value means INCRx mode with fix burst type.
When more than one value, means undefined length burst mode, USB controller
can use the length less than or equal to the largest enabled burst length.
While enabling undefined length INCR burst type and INCR16 burst type,
get better write performance on NXP Layerscape platform:
around 3% improvement (from 364MB/s to 375MB/s).
Signed-off-by: Changming Huang
---
Changes in v4:
- change definition for this property.
Changes in v3:
- add new property for INCR burst in usb node.
Documentation/devicetree/bindings/usb/dwc3.txt |6 ++
arch/arm/boot/dts/ls1021a.dtsi |1 +
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi |3 +++
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi |2 ++
4 files changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index e3e6983..a68dbfc 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -55,6 +55,11 @@ Optional properties:
fladj_30mhz_sdbnd signal is invalid or incorrect.
- tx-fifo-resize: determines if the FIFO *has* to be reallocated.
+ - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0
+ register, undefined length INCR burst type enable and INCRx type.
+ When just one value, which means INCRX burst mode. When more than one
+ value, which means undefined length INCR burst type enabled.
+ The values can be 1, 4, 8, 16, 32, 64, 128 and 256.
This is usually a subnode to DWC3 glue to which it is connected.
@@ -63,4 +68,5 @@ dwc3@4a03 {
reg = <0x4a03 0xcfff>;
interrupts = <0 92 4>
usb-phy = <&usb2_phy>, <&usb3,phy>;
+ snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>;
};
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 368e219..6eee0d5 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -627,6 +627,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>;
};
pcie@340 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 97d331e..04ffd66 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -482,6 +482,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>;
};
usb1: usb3@300 {
@@ -491,6 +492,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>;
};
usb2: usb3@310 {
@@ -500,6 +502,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>;
};
sata: sata@320 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index d058e56..902cc93 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -710,6 +710,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>;
};
usb1: usb3@311 {
@@ -720,6 +721,7 @@
dr_mode = "host";
snps,quirk-frame-length-adjustment = <0x20>;
snps,dis_rxdet_inp3_quirk;
+ snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>;
};
ccn@400 {
--
1.7.9.5
--
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 v4 1/3] USB3/DWC3: Add definition for global soc bus configuration register
Add the macro definition for global soc bus configuration register 0/1 Signed-off-by: Changming Huang --- Changes in v4: - no change Changes in v3: - no change Changes in v2: - split the patch - add more macro definition for soc bus configuration register drivers/usb/dwc3/core.h | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index de5a857..065aa6f 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -161,6 +161,32 @@ /* Bit fields */ +/* Global SoC Bus Configuration Register 0 */ +#define AXI3_CACHE_TYPE_AW 0x8 /* write allocate */ +#define AXI3_CACHE_TYPE_AR 0x4 /* read allocate */ +#define AXI3_CACHE_TYPE_SNP0x2 /* cacheable */ +#define AXI3_CACHE_TYPE_BUF0x1 /* bufferable */ +#define DWC3_GSBUSCFG0_DATARD_SHIFT28 +#define DWC3_GSBUSCFG0_DESCRD_SHIFT24 +#define DWC3_GSBUSCFG0_DATAWR_SHIFT20 +#define DWC3_GSBUSCFG0_DESCWR_SHIFT16 +#define DWC3_GSBUSCFG0_SNP_MASK0x +#define DWC3_GSBUSCFG0_DATABIGEND (1 << 11) +#define DWC3_GSBUSCFG0_DESCBIGEND (1 << 10) +#define DWC3_GSBUSCFG0_INCR256BRSTENA (1 << 7) /* INCR256 burst */ +#define DWC3_GSBUSCFG0_INCR128BRSTENA (1 << 6) /* INCR128 burst */ +#define DWC3_GSBUSCFG0_INCR64BRSTENA (1 << 5) /* INCR64 burst */ +#define DWC3_GSBUSCFG0_INCR32BRSTENA (1 << 4) /* INCR32 burst */ +#define DWC3_GSBUSCFG0_INCR16BRSTENA (1 << 3) /* INCR16 burst */ +#define DWC3_GSBUSCFG0_INCR8BRSTENA(1 << 2) /* INCR8 burst */ +#define DWC3_GSBUSCFG0_INCR4BRSTENA(1 << 1) /* INCR4 burst */ +#define DWC3_GSBUSCFG0_INCRBRSTENA (1 << 0) /* undefined length enable */ +#define DWC3_GSBUSCFG0_INCRBRST_MASK 0xff + +/* Global SoC Bus Configuration Register 1 */ +#define DWC3_GSBUSCFG1_1KPAGEENA (1 << 12) /* 1K page boundary enable */ +#define DWC3_GSBUSCFG1_PTRANSLIMIT_MASK0xf00 + /* Global Debug Queue/FIFO Space Available Register */ #define DWC3_GDBGFIFOSPACE_NUM(n) ((n) & 0x1f) #define DWC3_GDBGFIFOSPACE_TYPE(n) (((n) << 5) & 0x1e0) -- 1.7.9.5 -- 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 v4 3/3] USB3/DWC3: Enable undefined length INCR burst type
Enable the undefined length INCR burst type and set INCRx.
Different platform may has the different burst size type.
In order to get best performance, we need to tune the burst size to
one special value, instead of the default value.
Signed-off-by: Changming Huang
Signed-off-by: Rajesh Bhagat
---
Changes in v4:
- Modify the codes according to the definition of this property.
Changes in v3:
- add new property for INCR burst in usb node to reset GSBUSCFG0.
Changes in v2:
- split patch
- create one new function to handle soc bus configuration register.
drivers/usb/dwc3/core.c | 83 +++
drivers/usb/dwc3/core.h |7
2 files changed, 90 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 369bab1..446aec3 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -650,6 +650,87 @@ static void dwc3_core_setup_global_control(struct dwc3
*dwc)
dwc3_writel(dwc->regs, DWC3_GCTL, reg);
}
+/* set global soc bus configuration registers */
+static void dwc3_set_soc_bus_cfg(struct dwc3 *dwc)
+{
+ struct device *dev = dwc->dev;
+ u32 *vals;
+ u32 cfg;
+ int ntype;
+ int ret;
+ int i;
+
+ cfg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
+
+ /*
+* Handle property "snps,incr-burst-type-adjustment".
+* Get the number of value from this property:
+* result <= 0, means this property is not supported.
+* result = 1, means INCRx burst mode supported.
+* result > 1, means undefined length burst mode supported.
+*/
+ ntype = device_property_read_u32_array(dev,
+ "snps,incr-burst-type-adjustment", NULL, 0);
+ if (ntype > 0) {
+ vals = kcalloc(ntype, sizeof(u32), GFP_KERNEL);
+ if (!vals) {
+ dev_err(dev, "Error to get memory\n");
+ return;
+ }
+ /* Get INCR burst type, and parse it */
+ ret = device_property_read_u32_array(dev,
+ "snps,incr-burst-type-adjustment", vals, ntype);
+ if (ret) {
+ dev_err(dev, "Error to get property\n");
+ return;
+ }
+ *(dwc->incrx_type + 1) = vals[0];
+ if (ntype > 1) {
+ *dwc->incrx_type = 1;
+ for (i = 1; i < ntype; i++) {
+ if (vals[i] > *(dwc->incrx_type + 1))
+ *(dwc->incrx_type + 1) = vals[i];
+ }
+ } else
+ *dwc->incrx_type = 0;
+
+ /* Enable Undefined Length INCR Burst and Enable INCRx Burst */
+ cfg &= ~DWC3_GSBUSCFG0_INCRBRST_MASK;
+ if (*dwc->incrx_type)
+ cfg |= DWC3_GSBUSCFG0_INCRBRSTENA;
+ switch (*(dwc->incrx_type + 1)) {
+ case 256:
+ cfg |= DWC3_GSBUSCFG0_INCR256BRSTENA;
+ break;
+ case 128:
+ cfg |= DWC3_GSBUSCFG0_INCR128BRSTENA;
+ break;
+ case 64:
+ cfg |= DWC3_GSBUSCFG0_INCR64BRSTENA;
+ break;
+ case 32:
+ cfg |= DWC3_GSBUSCFG0_INCR32BRSTENA;
+ break;
+ case 16:
+ cfg |= DWC3_GSBUSCFG0_INCR16BRSTENA;
+ break;
+ case 8:
+ cfg |= DWC3_GSBUSCFG0_INCR8BRSTENA;
+ break;
+ case 4:
+ cfg |= DWC3_GSBUSCFG0_INCR4BRSTENA;
+ break;
+ case 1:
+ break;
+ default:
+ dev_err(dev, "Invalid property\n");
+ break;
+ }
+ }
+
+ dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, cfg);
+}
+
/**
* dwc3_core_init - Low-level initialization of DWC3 Core
* @dwc: Pointer to our controller context structure
@@ -698,6 +779,8 @@ static int dwc3_core_init(struct dwc3 *dwc)
/* Adjust Frame Length */
dwc3_frame_length_adjustment(dwc);
+ dwc3_set_soc_bus_cfg(dwc);
+
usb_phy_set_suspend(dwc->usb2_phy, 0);
usb_phy_set_suspend(dwc->usb3_phy, 0);
ret = phy_power_on(dwc->usb2_generic_phy);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 065aa6f..9df6304 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -805,6 +805,7 @@ struct dwc3_scratchpad_array {
* @regs: base address for our registers
* @regs_size: address space size
* @fladj: frame length adjustment
+ * @incrx_type: INCR burst type adjustment
* @irq_gadget: peripheral controller's IRQ number
* @nr_scratch: number of scratch buffers
* @u1u2:
Geschäftsvorschlag
Lieber Freund. Erlauben Sie mir, auf diese Weise auf Sie zuzugehen. Ich bin Dr. Arnold Kristofferson, ein US-Auftragnehmer, der mit Nichtkämpfer US Marine in Ba'qubah, Irak arbeitet. Ich habe die Summe von 10,6 Millionen Dollar, die ich aus einem Rohöl-Deal gemacht habe, und ich möchte, dass Sie mir helfen, es zu erhalten. Da ich hier auf offizieller Kapazität arbeite, kann ich diesen Fonds nicht bei mir behalten und dies ist mein einziger Grund für die Kontaktaufnahme mit Ihnen. Wenn Sie interessiert sind und Sie diesen Fonds in Ihrer Kapazität erhalten können, melden Sie sich bitte bei mir, damit ich Ihnen weitere Details geben kann, wie Sie dieses Geld für mich erhalten können. Bitte erreichen Sie mich per E-Mail: arnoldkristoferson...@gmail.com Mit freundlichen Grüßen. -- 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
Darlehen Angebot
Brauchen Sie eine persönliche oder geschäftliche Darlehen ohne Stress und schnelle Genehmigung? Wenn ja, kontaktieren Sie uns heute, wie wir derzeit bieten Darlehen zu hervorragenden Zinssatz. Unser Darlehen ist gesichert und sicher, unsere Kunden Glück ist unsere Stärke. Für weitere Informationen und Anwendung -- 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 v5 4/6] usb: xhci: use bus->sysdev for DMA configuration
On 12.01.2017 10:38, Roger Quadros wrote: Mathias, On 11/01/17 17:08, Alan Stern wrote: On Wed, 11 Jan 2017, Mathias Nyman wrote: On 17.11.2016 13:43, Sriram Dash wrote: From: Arnd Bergmann For xhci-hcd platform device, all the DMA parameters are not configured properly, notably dma ops for dwc3 devices. So, set the dma for xhci from sysdev. sysdev is pointing to device that is known to the system firmware or hardware. Signed-off-by: Arnd Bergmann Signed-off-by: Sriram Dash Tested-by: Baolin Wang --- ... + /* +* sysdev must point to a device that is known to the system firmware +* or PCI hardware. We handle these three cases here: +* 1. xhci_plat comes from firmware +* 2. xhci_plat is child of a device from firmware (dwc3-plat) +* 3. xhci_plat is grandchild of a pci device (dwc3-pci) +*/ + sysdev = &pdev->dev; + if (sysdev->parent && !sysdev->of_node && sysdev->parent->of_node) + sysdev = sysdev->parent; +#ifdef CONFIG_PCI + else if (sysdev->parent && sysdev->parent->parent && +sysdev->parent->parent->bus == &pci_bus_type) + sysdev = sysdev->parent->parent; +#endif + Not maybe the the ideal situation here, and looks really tailored to make PCI dwc3 controllers with xhci support work. Was there some reason child devices can't automatically inherit the dma mask from the parents, forcing us to dig it from grandparents? Anyway, looks like the dwc3 part is already in 4.10-rc, If Greg and Alan want to take this series that's fine by me I have no objections. Alan Stern I haven't tested that it won't break anything on PCI XHCI controllers though -Mathias Are you going to pick all the remaining patches from this series (i.e. 1 to 4)? That should fix the warning that people are seeing on v4.10-rc. Let's check with Greg Greg, 5/6 and 6/6 are in 4.10-rc already, causing additional warnings for people using dwc3 xhci. First 3 patches change usb core, patch 4 xhci. Compiles and boots, doesn't break pci xhci (non-dwc3) functionality Would you like me to send first 4 patches for usb-linus to get a clean final 4.10 without warnings, or to send them for usb-next? Or will you just pick the patches from here directly. for patch 4/6: Acked-by: Mathias Nyman -Mathias -- 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
[RFC PATCH 3/3] usb: phy: fsl: Remove the set_power callback
Since the set_power callback did not do anything for power setting,
then remove it.
Signed-off-by: Baolin Wang
---
drivers/usb/phy/phy-fsl-usb.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 94eb292..392ab42 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -642,17 +642,6 @@ static int fsl_otg_set_peripheral(struct usb_otg *otg,
return 0;
}
-/* Set OTG port power, only for B-device */
-static int fsl_otg_set_power(struct usb_phy *phy, unsigned mA)
-{
- if (!fsl_otg_dev)
- return -ENODEV;
- if (phy->otg->state == OTG_STATE_B_PERIPHERAL)
- pr_info("FSL OTG: Draw %d mA\n", mA);
-
- return 0;
-}
-
/*
* Delayed pin detect interrupt processing.
*
@@ -821,7 +810,6 @@ static int fsl_otg_conf(struct platform_device *pdev)
/* initialize the otg structure */
fsl_otg_tc->phy.label = DRIVER_DESC;
fsl_otg_tc->phy.dev = &pdev->dev;
- fsl_otg_tc->phy.set_power = fsl_otg_set_power;
fsl_otg_tc->phy.otg->usb_phy = &fsl_otg_tc->phy;
fsl_otg_tc->phy.otg->set_host = fsl_otg_set_host;
--
1.7.9.5
--
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
[RFC PATCH 1/3] usb: phy: ab8500: Remove the set_power callback
There are no users will use the vbus_draw variable set by set_power()
callback to set the vbus current. Thus we can remove it.
Signed-off-by: Baolin Wang
---
drivers/usb/phy/phy-ab8500-usb.c | 20
1 file changed, 20 deletions(-)
diff --git a/drivers/usb/phy/phy-ab8500-usb.c b/drivers/usb/phy/phy-ab8500-usb.c
index a03caf4..3dfbb97 100644
--- a/drivers/usb/phy/phy-ab8500-usb.c
+++ b/drivers/usb/phy/phy-ab8500-usb.c
@@ -1036,25 +1036,6 @@ static unsigned ab8500_eyediagram_workaroud(struct
ab8500_usb *ab, unsigned mA)
return mA;
}
-static int ab8500_usb_set_power(struct usb_phy *phy, unsigned mA)
-{
- struct ab8500_usb *ab;
-
- if (!phy)
- return -ENODEV;
-
- ab = phy_to_ab(phy);
-
- mA = ab8500_eyediagram_workaroud(ab, mA);
-
- ab->vbus_draw = mA;
-
- atomic_notifier_call_chain(&ab->phy.notifier,
- UX500_MUSB_VBUS, &ab->vbus_draw);
-
- return 0;
-}
-
static int ab8500_usb_set_suspend(struct usb_phy *x, int suspend)
{
/* TODO */
@@ -1392,7 +1373,6 @@ static int ab8500_usb_probe(struct platform_device *pdev)
ab->phy.otg = otg;
ab->phy.label = "ab8500";
ab->phy.set_suspend = ab8500_usb_set_suspend;
- ab->phy.set_power = ab8500_usb_set_power;
ab->phy.otg->state = OTG_STATE_UNDEFINED;
otg->usb_phy= &ab->phy;
--
1.7.9.5
--
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
[RFC PATCH 2/3] usb: phy: msm: Remove the set_power callback
Since it will not set the PMIC current drawn from USB configuration by
set_power callback, then remove it.
Signed-off-by: Baolin Wang
---
drivers/usb/phy/phy-msm-usb.c | 18 --
1 file changed, 18 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 8a34759..9e52890 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -842,23 +842,6 @@ static void msm_otg_notify_charger(struct msm_otg *motg,
unsigned mA)
motg->cur_power = mA;
}
-static int msm_otg_set_power(struct usb_phy *phy, unsigned mA)
-{
- struct msm_otg *motg = container_of(phy, struct msm_otg, phy);
-
- /*
-* Gadget driver uses set_power method to notify about the
-* available current based on suspend/configured states.
-*
-* IDEV_CHG can be drawn irrespective of suspend/un-configured
-* states when CDP/ACA is connected.
-*/
- if (motg->chg_type == USB_SDP_CHARGER)
- msm_otg_notify_charger(motg, mA);
-
- return 0;
-}
-
static void msm_otg_start_host(struct usb_phy *phy, int on)
{
struct msm_otg *motg = container_of(phy, struct msm_otg, phy);
@@ -1950,7 +1933,6 @@ static int msm_otg_probe(struct platform_device *pdev)
}
phy->init = msm_phy_init;
- phy->set_power = msm_otg_set_power;
phy->notify_disconnect = msm_phy_notify_disconnect;
phy->type = USB_PHY_TYPE_USB2;
--
1.7.9.5
--
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
[RFC PATCH 0/3] Remove some dummy set_power callbacks
In future we plan to introduce USB charger to set PMIC current drawn from USB configuration, instead of using set_power callback in phy driver. Moreover in these 3 phy drivers, the set_power callback did not implement anything to set PMIC current, thus we should remove them. Baolin Wang (3): usb: phy: ab8500: Remove the set_power callback usb: phy: msm: Remove the set_power callback usb: phy: fsl: Remove the set_power callback drivers/usb/phy/phy-ab8500-usb.c | 20 drivers/usb/phy/phy-fsl-usb.c| 12 drivers/usb/phy/phy-msm-usb.c| 18 -- 3 files changed, 50 deletions(-) -- 1.7.9.5 -- 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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On Tue, Jan 17, 2017 at 09:55:24AM -0800, Tony Lindgren wrote:
> Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
> when no cable is connected. But looks like few corner case issues still
> remain.
>
> Looks like just by re-plugging USB cable about ten or so times on BeagleBone
> when configured in USB peripheral mode we can get warnings and eventually
> trigger an oops in cppi41 DMA:
>
> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+
> x28/0x38 [cppi41]
> ...
>
> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
> push_desc_queue+0x94/0x9c [cppi41]
> ...
>
> Unable to handle kernel NULL pointer dereference at virtual
> address 0104
> pgd = c0004000
> [0104] *pgd=
> Internal error: Oops: 805 [#1] SMP ARM
> ...
> [] (cppi41_runtime_resume [cppi41]) from []
> (__rpm_callback+0xc0/0x214)
> [] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
> [] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
> [] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
> [] (pm_runtime_work) from []
> (process_one_work+0x2b4/0x808)
>
> This is because of a race with runtime PM and cppi41_dma_issue_pending()
> as reported by Alexandre Bailon in earlier
> set of patches. Based on mailing list discussions we however came to the
> conclusion that a different fix from Alexandre's fix is needed in order
> to guarantee that DMA is really active when we try to use it.
>
> To fix the issue, we need to add a driver specific flag as we otherwise
> can have -EINPROGRESS state set by runtime PM and can't rely on
> pm_runtime_active() to tell us when we can use the DMA.
>
> And we need to make sure the DMA transfers get triggered in the queued
> order. So let's always queue the transfers, then flush the queue
> from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
> as suggested by Grygorii Strashko in an
> earlier example patch.
>
> For reference, this is also documented in Documentation/power/runtime_pm.txt
> in the example at the end of the file as pointed out by Grygorii Strashko
> .
>
> Based on earlier patches from Alexandre Bailon
> and Grygorii Strashko modified based on
> testing and what was discussed on the mailing lists.
>
> Note that additional patches are still needed depending on how we want
> to handle the cppi41 runtime PM. So far cppi41 is always coupled with
> musb driver, and musb guarantees that cppi41 is always active during
> use. So until we have a better solution, let's just set the cppi41
> autosuspend delay 1 second to avoid pointless EINPROGRESS warnings during
> USB mass storage enumeration. As cppi41 is properly clocked with musb
> being active, we can safely do this.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Cc: Andy Shevchenko
> Cc: Bin Liu
> Cc: Grygorii Strashko
> Cc: Kevin Hilman
> Cc: Patrick Titiano
> Cc: Sergei Shtylyov
> Reported-by: Alexandre Bailon
> Signed-off-by: Tony Lindgren
With this v3, I still get -71 error when a device is plugged to a hub to
musb. It doesn't happen though if the device is already plugged to the hub
before attaching the hub to musb.
[ 177.579300] usb 1-1: reset high-speed USB device number 2 using musb-hdrc
[ 177.719308] usb 1-1: device descriptor read/64, error -71
[ 178.350297] usb 1-1.1: new high-speed USB device number 4 using musb-hdrc
And I still get -115 error flooding with thumb drive.
[ 236.880068] cppi41-dma-engine 4740.dma-controller: cppi41_irq pm runtime
get: -115
I tested with TI AM335x GP EVM. The problems happen on both musb ports.
Regards,
-Bin.
--
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
[BISSECTED]: BUG: usb: dwc3: usb ports not working anymore on odroid-XU4
Hi,
Since commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit paths")
(merged in 4.8), the usb ports on odroid-XU4 don't work anymore.
[ Actually, it's commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all
cores"), cf below ]
Inserting an usb key (USB2.0) on the USB3.0 port result in:
[ 64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 2
msec, port status = 0xc400fe3
[ 74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop
endpoint command.
[ 74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting host.
[ 74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up
[ 74.606276] usb 3-1: USB disconnect, device number 2
[ 74.613565] usb 4-1: USB disconnect, device number 2
[ 74.621208] usb usb3-port1: couldn't allocate usb_device
NB: it's not related to USB2.0 devices, I get the same result with an USB3.0
device (SATA to USB3 for instance).
NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the realtek
RTL8153 chip.
I instrumented what was read/written in the registers before and after this
patch, and I found that the culprit is:
if (dwc->revision > DWC3_REVISION_194A)
reg |= DWC3_GUSB3PIPECTL_SUSPHY;
Before commit c499ff71ff2a2 ("usb: dwc3: core: re-factor init and exit paths"),
dwc3_phy_setup() was
done early in dwc3_probe() and thus, dwc->revision wasn't set yet (==0)
After this commit, dwc3_phy_setup() is done in dwc3_core_init(), after setting
dwc->revision (it's done in dwc3_core_is_valid()).
(The dwc3->revision on odroid-XU4 is 5533200a.)
If I comment out the 2 lines:
if (dwc->revision > DWC3_REVISION_194A)
reg |= DWC3_GUSB3PIPECTL_SUSPHY;
The usb key is recognized right away:
[ 38.008158] usb 3-1.2: new high-speed USB device number 3 using xhci-hcd
[ 38.138924] usb 3-1.2: New USB device found, idVendor=abcd, idProduct=1234
[ 38.144399] usb 3-1.2: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
I took a look at the history behind those 2 lines, and they've been introduced
by:
commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores") in 3.19.
In 3.19, the dwc->revision dwc3_phy_setup() was set in dwc3_core_init().
And, booting a 3.19 kernel on XU4 gives the same error (whereas booting a 3.18
kernel is ok)
[ So, I should have started this email with:
Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"),
the usb ports on odroid-XU4 don't work anymore. ]
The funny thing is that commit 45bb7de213d8("usb: dwc3: setup phys earlier")
(merged in 4.2)
moved dwc3_phy_setup() in dwc3_probe(), way before dwc->revision was set,
acting like a revert
on commit 2164a476205c("usb: dwc3: set SUSPHY bit for all cores")
That's why on 4.2, inserting an USB key on odroid-XU4 worked again,
So, to make a resume:
xxx-> 3.18: usb ok
3.19->4.1: usb error (due to commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit
for all cores"))
4.2->4.7: usb ok (due to commit 45bb7de213d8("usb: dwc3: setup phys earlier"))
4.8->now: usb error (due to commit c499ff71ff2a2 ("usb: dwc3: core: re-factor
init and exit paths"))
any idea on this ?
regards,
Richard
--
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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
Hi,
* Bin Liu [170118 06:26]:
> With this v3, I still get -71 error when a device is plugged to a hub to
> musb. It doesn't happen though if the device is already plugged to the hub
> before attaching the hub to musb.
>
> [ 177.579300] usb 1-1: reset high-speed USB device number 2 using musb-hdrc
> [ 177.719308] usb 1-1: device descriptor read/64, error -71
> [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using musb-hdrc
I think the -71 issue is a different regression. I've verified that v4.8.y
does not have this, but v4.9.y does have it.
So far the easiest way for me to reproduce the -71 problem is by plugging
a USB drive into a connected hub. If I connect the hub with USB drive already
plugged into the hub, it does not happen.
With the hub connected musb is already active when the USB drive is plugged
in. So I'm now suspecting this -71 regression is not related to runtime PM
changes. Bisect and boot and plug devices time I think unless you have
better ideas?
> And I still get -115 error flooding with thumb drive.
>
> [ 236.880068] cppi41-dma-engine 4740.dma-controller: cppi41_irq pm
> runtime get: -115
>
> I tested with TI AM335x GP EVM. The problems happen on both musb ports.
OK. So it's pointless to try to play with the autosuspend timeout.
Let's just do a WARN(!pm_runtime_active(cdd->ddev.dev->parent)) there
until we have musb_cppi41.c dma calls properly paired and don't have
dma requests lingering.
Care to try the updated patch below? It won't help with the -71 issue
but the $subject issue should be fixed. And you should not see the
WARN() trigger with your tests presumably.
Regards,
Tony
8<
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren
Date: Mon, 16 Jan 2017 09:52:10 -0800
Subject: [PATCH] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
when no cable is connected. But looks like few corner case issues still
remain.
Looks like just by re-plugging USB cable about ten or so times on BeagleBone
when configured in USB peripheral mode we can get warnings and eventually
trigger an oops in cppi41 DMA:
WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+
x28/0x38 [cppi41]
...
WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
push_desc_queue+0x94/0x9c [cppi41]
...
Unable to handle kernel NULL pointer dereference at virtual
address 0104
pgd = c0004000
[0104] *pgd=
Internal error: Oops: 805 [#1] SMP ARM
...
[] (cppi41_runtime_resume [cppi41]) from []
(__rpm_callback+0xc0/0x214)
[] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
[] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
[] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
[] (pm_runtime_work) from [] (process_one_work+0x2b4/0x808)
This is because of a race with runtime PM and cppi41_dma_issue_pending()
as reported by Alexandre Bailon in earlier
set of patches. Based on mailing list discussions we however came to the
conclusion that a different fix from Alexandre's fix is needed in order
to guarantee that DMA is really active when we try to use it.
To fix the issue, we need to add a driver specific flag as we otherwise
can have -EINPROGRESS state set by runtime PM and can't rely on
pm_runtime_active() to tell us when we can use the DMA.
And we need to make sure the DMA transfers get triggered in the queued
order. So let's always queue the transfers, then flush the queue
from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
as suggested by Grygorii Strashko in an
earlier example patch.
For reference, this is also documented in Documentation/power/runtime_pm.txt
in the example at the end of the file as pointed out by Grygorii Strashko
.
Based on earlier patches from Alexandre Bailon
and Grygorii Strashko modified based on
testing and what was discussed on the mailing lists.
Note that additional patches are still needed depending on how we want
to handle the cppi41 runtime PM. So far cppi41 is always coupled with
musb driver, and musb guarantees that cppi41 is always active during
use. So until we have a better solution, let's just WARN() if the musb
parent is not active to avoid pointless EINPROGRESS warnings during
USB mass storage enumeration. As cppi41 is properly clocked with musb
being active, we can safely do this.
Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Cc: Andy Shevchenko
Cc: Bin Liu
Cc: Grygorii Strashko
Cc: Kevin Hilman
Cc: Patrick Titiano
Cc: Sergei Shtylyov
Reported-by: Alexandre Bailon
Signed-off-by: Tony Lindgren
---
drivers/dma/cppi41.c | 51 ---
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -153,6 +153,8 @@ struct cppi41_dd {
/* con
Re: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On 01/18/2017 10:53 AM, Tony Lindgren wrote: > Hi, > > * Bin Liu [170118 06:26]: >> With this v3, I still get -71 error when a device is plugged to a hub to >> musb. It doesn't happen though if the device is already plugged to the hub >> before attaching the hub to musb. >> >> [ 177.579300] usb 1-1: reset high-speed USB device number 2 using musb-hdrc >> [ 177.719308] usb 1-1: device descriptor read/64, error -71 >> [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using musb-hdrc > > I think the -71 issue is a different regression. I've verified that v4.8.y > does not have this, but v4.9.y does have it. > > So far the easiest way for me to reproduce the -71 problem is by plugging > a USB drive into a connected hub. If I connect the hub with USB drive already > plugged into the hub, it does not happen. > > With the hub connected musb is already active when the USB drive is plugged This is not exactly try, i think, because cppi can be in inactive state because of autosuspend (all calls are paired). > in. So I'm now suspecting this -71 regression is not related to runtime PM > changes. Bisect and boot and plug devices time I think unless you have > better ideas? > >> And I still get -115 error flooding with thumb drive. >> >> [ 236.880068] cppi41-dma-engine 4740.dma-controller: cppi41_irq pm >> runtime get: -115 >> >> I tested with TI AM335x GP EVM. The problems happen on both musb ports. > > OK. So it's pointless to try to play with the autosuspend timeout. > > Let's just do a WARN(!pm_runtime_active(cdd->ddev.dev->parent)) there > until we have musb_cppi41.c dma calls properly paired and don't have > dma requests lingering. > > Care to try the updated patch below? It won't help with the -71 issue > but the $subject issue should be fixed. And you should not see the > WARN() trigger with your tests presumably. > Sry, but wouldn't be more simple and safe just to drop PM runtime from this driver, as it is part of musb and musb PM state expected to be handled properly by PM runtime now. More over, There is another possibility related to the current PM runtime implementation and autosuspend - it might introduce delay between time when musb request desc push and time when cppi will actually put this desc in HW queue if cppi was not active. For example, there might not be -71 error seen if CPPI autosuspend is disabled (cppi is active all the time) before plug-in hub and then USB drive. -- regards, -grygorii -- 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: Strange behaviour of U2F usb key
Hi Greg, Sorry for the long delay before this message. Real life intruded on my computing time. On 05/01/17 12:51, Greg KH wrote: > Are you sure the device even works properly? It could be the device has > problems returning data correctly. Can you run usbmon to see if the > data is being sent/received correctly? Tests done with three different keys, with the same results. Since the keys come from the same manufacturer, we ordered a (small) batch of UF2 keys from 4 different manufacturers. Should receive them around next week, and we will test if those new keys behave properly or not. -- Pierre-Yves Bonnetain-Nesterenko -- 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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Tony Lindgren [170118 10:15]:
> * Grygorii Strashko [170118 10:05]:
> >
> >
> > On 01/18/2017 10:53 AM, Tony Lindgren wrote:
> > > Hi,
> > >
> > > * Bin Liu [170118 06:26]:
> > >> With this v3, I still get -71 error when a device is plugged to a hub to
> > >> musb. It doesn't happen though if the device is already plugged to the
> > >> hub
> > >> before attaching the hub to musb.
> > >>
> > >> [ 177.579300] usb 1-1: reset high-speed USB device number 2 using
> > >> musb-hdrc
> > >> [ 177.719308] usb 1-1: device descriptor read/64, error -71
> > >> [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using
> > >> musb-hdrc
> > >
> > > I think the -71 issue is a different regression. I've verified that v4.8.y
> > > does not have this, but v4.9.y does have it.
> > >
> > > So far the easiest way for me to reproduce the -71 problem is by plugging
> > > a USB drive into a connected hub. If I connect the hub with USB drive
> > > already
> > > plugged into the hub, it does not happen.
...
> > Sry, but wouldn't be more simple and safe just to drop PM runtime from this
> > driver,
> > as it is part of musb and musb PM state expected to be handled properly by
> > PM runtime now.
>
> Well we still need PM runtime in cppi41 to initialize it when it gets
> probed and idle it when not used even if musb modules are not loaded.
>
> Unfortunately until musb_cppi41.c dma calls are fixed, we can't do
> any sane use counting in cppi41.c without adding timeout handling
> to it.
>
> > More over, There is another possibility related to the current PM runtime
> > implementation and autosuspend
> > - it might introduce delay between time when musb request desc push and
> > time when cppi will actually
> > put this desc in HW queue if cppi was not active.
> > For example, there might not be -71 error seen if CPPI autosuspend is
> > disabled
> > (cppi is active all the time) before plug-in hub and then USB drive.
>
> I already checked that. The -71 error seems to be a separate issue.
OK found it. I can make v4.8.17 produce -71 errors with just commit
467d5c980709 ("usb: musb: Implement session bit based runtime PM for
musb-core") applied on top of it. So looks like we're missing some
musb quirk handling for the devctl session bit.
Regards,
Tony
--
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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Bin Liu [170118 10:43]: > On Wed, Jan 18, 2017 at 08:53:09AM -0800, Tony Lindgren wrote: > > Hi, > > > > * Bin Liu [170118 06:26]: > > > With this v3, I still get -71 error when a device is plugged to a hub to > > > musb. It doesn't happen though if the device is already plugged to the hub > > > before attaching the hub to musb. > > > > > > [ 177.579300] usb 1-1: reset high-speed USB device number 2 using > > > musb-hdrc > > > [ 177.719308] usb 1-1: device descriptor read/64, error -71 > > > [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using > > > musb-hdrc > > > > I think the -71 issue is a different regression. I've verified that v4.8.y > > does not have this, but v4.9.y does have it. > > I will take a look on this one. Found the commit causing, see the mail I just sent. It's 467d5c980709. > > Care to try the updated patch below? It won't help with the -71 issue > > but the $subject issue should be fixed. And you should not see the > > WARN() trigger with your tests presumably. > > Yes, now no WARN() and no -115 any more. Thanks. OK. I'll take a look at what additional quirk handling the devctl session bit needs for the -71 error and then with these we should have things working again. Regards, Tony -- 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 1/5] UDC: Split the driver into amd (pci) and Synopsys core driver
On 01/17/2017 12:05 AM, Raviteja Garimella wrote: > This patch splits the amd5536udc driver into two -- one that does > pci device registration and the other file that does the rest of > the driver tasks like the gadget/ep ops etc for Synopsys UDC. > > This way of splitting helps in exporting core driver symbols which > can be used by any other platform/pci driver that is written for > the same Synopsys USB device controller. > > The current patch also includes a change in the Kconfig and Makefile. > A new config option USB_SNP_CORE will be selected automatically when > any one of the platform or pci driver for the same UDC is selected. > > Signed-off-by: Raviteja Garimella Although the changes you have done make sense and it is most certainly a good idea to split udc core from bus specific glue logic, it is really hard to review the changes per-se because of the file rename, could that happen at a later time? Also, keep in mind that anytime a driver file is renamed, this poses a backport/maintenance issue where backporting fixes from latest upstream to a kernel version that has a different file/directory structure is a major pain. -- Florian -- 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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On Wed, Jan 18, 2017 at 08:53:09AM -0800, Tony Lindgren wrote:
> Hi,
>
> * Bin Liu [170118 06:26]:
> > With this v3, I still get -71 error when a device is plugged to a hub to
> > musb. It doesn't happen though if the device is already plugged to the hub
> > before attaching the hub to musb.
> >
> > [ 177.579300] usb 1-1: reset high-speed USB device number 2 using musb-hdrc
> > [ 177.719308] usb 1-1: device descriptor read/64, error -71
> > [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using musb-hdrc
>
> I think the -71 issue is a different regression. I've verified that v4.8.y
> does not have this, but v4.9.y does have it.
I will take a look on this one.
>
> So far the easiest way for me to reproduce the -71 problem is by plugging
> a USB drive into a connected hub. If I connect the hub with USB drive already
> plugged into the hub, it does not happen.
>
> With the hub connected musb is already active when the USB drive is plugged
> in. So I'm now suspecting this -71 regression is not related to runtime PM
> changes. Bisect and boot and plug devices time I think unless you have
> better ideas?
>
> > And I still get -115 error flooding with thumb drive.
> >
> > [ 236.880068] cppi41-dma-engine 4740.dma-controller: cppi41_irq pm
> > runtime get: -115
> >
> > I tested with TI AM335x GP EVM. The problems happen on both musb ports.
>
> OK. So it's pointless to try to play with the autosuspend timeout.
>
> Let's just do a WARN(!pm_runtime_active(cdd->ddev.dev->parent)) there
> until we have musb_cppi41.c dma calls properly paired and don't have
> dma requests lingering.
>
> Care to try the updated patch below? It won't help with the -71 issue
> but the $subject issue should be fixed. And you should not see the
> WARN() trigger with your tests presumably.
Yes, now no WARN() and no -115 any more. Thanks.
Regards,
-Bin.
>
> Regards,
>
> Tony
>
> 8<
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren
> Date: Mon, 16 Jan 2017 09:52:10 -0800
> Subject: [PATCH] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
>
> Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
> when no cable is connected. But looks like few corner case issues still
> remain.
>
> Looks like just by re-plugging USB cable about ten or so times on BeagleBone
> when configured in USB peripheral mode we can get warnings and eventually
> trigger an oops in cppi41 DMA:
>
> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+
> x28/0x38 [cppi41]
> ...
>
> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
> push_desc_queue+0x94/0x9c [cppi41]
> ...
>
> Unable to handle kernel NULL pointer dereference at virtual
> address 0104
> pgd = c0004000
> [0104] *pgd=
> Internal error: Oops: 805 [#1] SMP ARM
> ...
> [] (cppi41_runtime_resume [cppi41]) from []
> (__rpm_callback+0xc0/0x214)
> [] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
> [] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
> [] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
> [] (pm_runtime_work) from []
> (process_one_work+0x2b4/0x808)
>
> This is because of a race with runtime PM and cppi41_dma_issue_pending()
> as reported by Alexandre Bailon in earlier
> set of patches. Based on mailing list discussions we however came to the
> conclusion that a different fix from Alexandre's fix is needed in order
> to guarantee that DMA is really active when we try to use it.
>
> To fix the issue, we need to add a driver specific flag as we otherwise
> can have -EINPROGRESS state set by runtime PM and can't rely on
> pm_runtime_active() to tell us when we can use the DMA.
>
> And we need to make sure the DMA transfers get triggered in the queued
> order. So let's always queue the transfers, then flush the queue
> from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
> as suggested by Grygorii Strashko in an
> earlier example patch.
>
> For reference, this is also documented in Documentation/power/runtime_pm.txt
> in the example at the end of the file as pointed out by Grygorii Strashko
> .
>
> Based on earlier patches from Alexandre Bailon
> and Grygorii Strashko modified based on
> testing and what was discussed on the mailing lists.
>
> Note that additional patches are still needed depending on how we want
> to handle the cppi41 runtime PM. So far cppi41 is always coupled with
> musb driver, and musb guarantees that cppi41 is always active during
> use. So until we have a better solution, let's just WARN() if the musb
> parent is not active to avoid pointless EINPROGRESS warnings during
> USB mass storage enumeration. As cppi41 is properly clocked with musb
> being active, we can safely do this.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Cc: Andy Shevchenko
> Cc: Bin Liu
> Cc: Grygorii Strashko
> Cc: Kevin Hilman
> Cc
Re: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Grygorii Strashko [170118 10:05]: > > > On 01/18/2017 10:53 AM, Tony Lindgren wrote: > > Hi, > > > > * Bin Liu [170118 06:26]: > >> With this v3, I still get -71 error when a device is plugged to a hub to > >> musb. It doesn't happen though if the device is already plugged to the hub > >> before attaching the hub to musb. > >> > >> [ 177.579300] usb 1-1: reset high-speed USB device number 2 using > >> musb-hdrc > >> [ 177.719308] usb 1-1: device descriptor read/64, error -71 > >> [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using > >> musb-hdrc > > > > I think the -71 issue is a different regression. I've verified that v4.8.y > > does not have this, but v4.9.y does have it. > > > > So far the easiest way for me to reproduce the -71 problem is by plugging > > a USB drive into a connected hub. If I connect the hub with USB drive > > already > > plugged into the hub, it does not happen. > > > > With the hub connected musb is already active when the USB drive is plugged > > This is not exactly try, i think, because cppi can be in inactive state > because of autosuspend (all calls are paired). Musb is active when there's something connected meaning cppi41 is clocked when a hub is connected. The actual runtime PM implementation happens for the interconnect target module for both musb and cppi41 in hwmod.c code. > > in. So I'm now suspecting this -71 regression is not related to runtime PM > > changes. Bisect and boot and plug devices time I think unless you have > > better ideas? > > > >> And I still get -115 error flooding with thumb drive. > >> > >> [ 236.880068] cppi41-dma-engine 4740.dma-controller: cppi41_irq pm > >> runtime get: -115 > >> > >> I tested with TI AM335x GP EVM. The problems happen on both musb ports. > > > > OK. So it's pointless to try to play with the autosuspend timeout. > > > > Let's just do a WARN(!pm_runtime_active(cdd->ddev.dev->parent)) there > > until we have musb_cppi41.c dma calls properly paired and don't have > > dma requests lingering. > > > > Care to try the updated patch below? It won't help with the -71 issue > > but the $subject issue should be fixed. And you should not see the > > WARN() trigger with your tests presumably. > > > > Sry, but wouldn't be more simple and safe just to drop PM runtime from this > driver, > as it is part of musb and musb PM state expected to be handled properly by PM > runtime now. Well we still need PM runtime in cppi41 to initialize it when it gets probed and idle it when not used even if musb modules are not loaded. Unfortunately until musb_cppi41.c dma calls are fixed, we can't do any sane use counting in cppi41.c without adding timeout handling to it. > More over, There is another possibility related to the current PM runtime > implementation and autosuspend > - it might introduce delay between time when musb request desc push and time > when cppi will actually > put this desc in HW queue if cppi was not active. > For example, there might not be -71 error seen if CPPI autosuspend is disabled > (cppi is active all the time) before plug-in hub and then USB drive. I already checked that. The -71 error seems to be a separate issue. Regards, Tony -- 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] usb: dwc2: remove dead function dwc2_pci_quirks
On 1/17/2017 12:51 PM, Heiner Kallweit wrote:
> Commit 9962b62f1be9 "Deprecate g-use-dma binding" removed the only
> property in dwc2_pci_quirks. This function is dead code now.
>
> Maybe it was kept intentionally to be prepared for the case that
> another quirk-related property needs to be added in future.
> If not it can be removed.
>
> Signed-off-by: Heiner Kallweit
> ---
> drivers/usb/dwc2/pci.c | 18 --
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
> index a23329e3..ae419615 100644
> --- a/drivers/usb/dwc2/pci.c
> +++ b/drivers/usb/dwc2/pci.c
> @@ -62,20 +62,6 @@ struct dwc2_pci_glue {
> struct platform_device *phy;
> };
>
> -static int dwc2_pci_quirks(struct pci_dev *pdev, struct platform_device
> *dwc2)
> -{
> - if (pdev->vendor == PCI_VENDOR_ID_SYNOPSYS &&
> - pdev->device == PCI_PRODUCT_ID_HAPS_HSOTG) {
> - struct property_entry properties[] = {
> - { },
> - };
> -
> - return platform_device_add_properties(dwc2, properties);
> - }
> -
> - return 0;
> -}
> -
> static void dwc2_pci_remove(struct pci_dev *pci)
> {
> struct dwc2_pci_glue *glue = pci_get_drvdata(pci);
> @@ -136,10 +122,6 @@ static int dwc2_pci_probe(struct pci_dev *pci,
> return PTR_ERR(phy);
> }
>
> - ret = dwc2_pci_quirks(pci, dwc2);
> - if (ret)
> - goto err;
> -
> ret = platform_device_add(dwc2);
> if (ret) {
> dev_err(dev, "failed to register dwc2 device\n");
>
I'd like to leave this in place for now.
We still have stuff in here internally and we have some enhancements
in the works to support IP validation, which will need to set
properties from here.
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
Re: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Grygorii Strashko [170118 11:18]: > Just thinking, may be cppi41 should not be platform device at all > and it might be reasonable to have it as lib with > cppi41_init()/cppi41_remove(), > so musb SoC glue layer will initialize it, because it provides services to > other HW blocks withing musb only. Nah, we already have things almost working :) And if some TI SoC with general purpose cppi41 pops up like a DSP running Linux, we're in big trouble. And musb is already a mess as is.. Tony -- 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 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller
On Wed, Jan 11, 2017 at 08:02:29PM -0200, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C2410 and
> compatible USB device controller.
>
> Signed-off-by: Sergio Prado
> ---
> .../devicetree/bindings/usb/s3c2410-usb.txt| 28
> ++
> 1 file changed, 28 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> index e45b38ce2986..28353eea31fd 100644
> --- a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
> @@ -20,3 +20,31 @@ usb0: ohci@4900 {
> clocks = <&clocks UCLK>, <&clocks HCLK_USBH>;
> clock-names = "usb-bus-host", "usb-host";
> };
> +
> +Samsung S3C2410 and compatible USB device controller
> +
> +Required properties:
> + - compatible: Should be one of the following
> +"samsung,s3c2410-udc"
> +"samsung,s3c2440-udc"
> + - reg: address and length of the controller memory mapped region
> + - interrupts: interrupt number for the USB device controller
> + - clocks: Should reference the bus and host clocks
> + - clock-names: Should contain two strings
> + "uclk" for the USB bus clock
> + "usb-device" for the USB device clock
Perhaps just "bus" and "device".
> +
> +Optional properties:
> + - samsung,vbus-gpio: specifies a gpio that allows to detect whether
> + vbus is present - USB is connected (active high, input).
> + - samsung,pullup-gpio: If present, specifies a gpio to control the
> + USB D+ pullup (active high, output).
"-gpios", not "-gpio" is preferred.
These should be common property names if we're going to have them. The
problem with just "vbus-gpios" is does that mean an enable control or
status as you have. I guess in the former case, that should always be
modeled as a regulator.
Also, these should all be part of a connector node as these controls are
more related to the USB connector than the controller. And I don't mean
extcon here because those bindings are a mess.
Rob
--
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 1/5] UDC: Split the driver into amd (pci) and Synopsys core driver
On Wed, Jan 18, 2017 at 10:45:39AM -0800, Florian Fainelli wrote: > On 01/17/2017 12:05 AM, Raviteja Garimella wrote: > > This patch splits the amd5536udc driver into two -- one that does > > pci device registration and the other file that does the rest of > > the driver tasks like the gadget/ep ops etc for Synopsys UDC. > > > > This way of splitting helps in exporting core driver symbols which > > can be used by any other platform/pci driver that is written for > > the same Synopsys USB device controller. > > > > The current patch also includes a change in the Kconfig and Makefile. > > A new config option USB_SNP_CORE will be selected automatically when > > any one of the platform or pci driver for the same UDC is selected. > > > > Signed-off-by: Raviteja Garimella > > Although the changes you have done make sense and it is most certainly a > good idea to split udc core from bus specific glue logic, it is really > hard to review the changes per-se because of the file rename, could that > happen at a later time? > > Also, keep in mind that anytime a driver file is renamed, this poses a > backport/maintenance issue where backporting fixes from latest upstream > to a kernel version that has a different file/directory structure is a > major pain. Don't ever let stable tree work prevent you from doing the right thing. If this needs to be split, wonderful, stable kernel work will just have to deal with it. Hint, we handle it all the time just fine :) thanks, greg k-h -- 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] USB: qcserial: add Dell DW5570 QDL
The Dell DW5570 is a re-branded Sierra Wireless MC8805 which will by
default boot with vid 0x413c and pid 0x81a3. When triggered QDL download
mode, the device switches to pid 0x81a6 and provides the standard TTY
used for firmware upgrade.
Cc:
Signed-off-by: Aleksander Morgado
---
drivers/usb/serial/qcserial.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
index 1bc6089b9008..696458db7e3c 100644
--- a/drivers/usb/serial/qcserial.c
+++ b/drivers/usb/serial/qcserial.c
@@ -124,6 +124,7 @@ static const struct usb_device_id id_table[] = {
{USB_DEVICE(0x1410, 0xa021)}, /* Novatel Gobi 3000 Composite */
{USB_DEVICE(0x413c, 0x8193)}, /* Dell Gobi 3000 QDL */
{USB_DEVICE(0x413c, 0x8194)}, /* Dell Gobi 3000 Composite */
+ {USB_DEVICE(0x413c, 0x81a6)}, /* Dell DW5570 QDL (MC8805) */
{USB_DEVICE(0x1199, 0x68a4)}, /* Sierra Wireless QDL */
{USB_DEVICE(0x1199, 0x68a5)}, /* Sierra Wireless Modem */
{USB_DEVICE(0x1199, 0x68a8)}, /* Sierra Wireless QDL */
--
2.11.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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Bin Liu [170118 10:49]:
> On Wed, Jan 18, 2017 at 10:44:32AM -0800, Tony Lindgren wrote:
> > * Tony Lindgren [170118 10:15]:
> > > * Grygorii Strashko [170118 10:05]:
> > > >
> > > >
> > > > On 01/18/2017 10:53 AM, Tony Lindgren wrote:
> > > > > Hi,
> > > > >
> > > > > * Bin Liu [170118 06:26]:
> > > > >> With this v3, I still get -71 error when a device is plugged to a
> > > > >> hub to
> > > > >> musb. It doesn't happen though if the device is already plugged to
> > > > >> the hub
> > > > >> before attaching the hub to musb.
> > > > >>
> > > > >> [ 177.579300] usb 1-1: reset high-speed USB device number 2 using
> > > > >> musb-hdrc
> > > > >> [ 177.719308] usb 1-1: device descriptor read/64, error -71
> > > > >> [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using
> > > > >> musb-hdrc
> > > > >
> > > > > I think the -71 issue is a different regression. I've verified that
> > > > > v4.8.y
> > > > > does not have this, but v4.9.y does have it.
> > > > >
> > > > > So far the easiest way for me to reproduce the -71 problem is by
> > > > > plugging
> > > > > a USB drive into a connected hub. If I connect the hub with USB drive
> > > > > already
> > > > > plugged into the hub, it does not happen.
> > ...
> >
> > > > Sry, but wouldn't be more simple and safe just to drop PM runtime from
> > > > this driver,
> > > > as it is part of musb and musb PM state expected to be handled properly
> > > > by PM runtime now.
> > >
> > > Well we still need PM runtime in cppi41 to initialize it when it gets
> > > probed and idle it when not used even if musb modules are not loaded.
> > >
> > > Unfortunately until musb_cppi41.c dma calls are fixed, we can't do
> > > any sane use counting in cppi41.c without adding timeout handling
> > > to it.
> > >
> > > > More over, There is another possibility related to the current PM
> > > > runtime implementation and autosuspend
> > > > - it might introduce delay between time when musb request desc push and
> > > > time when cppi will actually
> > > > put this desc in HW queue if cppi was not active.
> > > > For example, there might not be -71 error seen if CPPI autosuspend is
> > > > disabled
> > > > (cppi is active all the time) before plug-in hub and then USB drive.
> > >
> > > I already checked that. The -71 error seems to be a separate issue.
> >
> > OK found it. I can make v4.8.17 produce -71 errors with just commit
> > 467d5c980709 ("usb: musb: Implement session bit based runtime PM for
> > musb-core") applied on top of it. So looks like we're missing some
> > musb quirk handling for the devctl session bit.
>
> Nice!
And here's a fix for the error -71 regression.
Bin, can you review and test your earlier case mentioned in
commit 9298b4aad37e ("usb: musb: fix device hotplug behind hub") with
the patch below to make sure this is safe to do now starting with v4.9?
Regards,
Tony
8< ---
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren
Date: Wed, 18 Jan 2017 12:31:25 -0800
Subject: [PATCH] usb: musb: Fix host mode error -71 regression
Commit 467d5c980709 ("usb: musb: Implement session bit based runtime PM for
musb-core") started implementing musb generic runtime PM support by
introducing devctl register session bit based state control.
This caused a regression where if a USB mass storage device is connected
to a USB hub, we can get:
usb 1-1: reset high-speed USB device number 2 using musb-hdrc
usb 1-1: device descriptor read/64, error -71
usb 1-1.1: new high-speed USB device number 4 using musb-hdrc
This is because before the USB storage device is connected, musb is
in OTG_STATE_A_SUSPEND. And we currently only set need_finish_resume
in musb_stage0_irq() and the related code calling finish_resume_work
in musb_resume() and musb_runtime_resume() never gets called.
To fix the issue, we can call schedule_delayed_work() directly in
musb_stage0_irq() to have finish_resume_work run.
And we should no longer never get interrupts when when suspended.
We have changed musb to no longer need pm_runtime_irqsafe().
The need_finish_resume flag was added in commit 9298b4aad37e ("usb:
musb: fix device hotplug behind hub") and no longer applies as far
as I can tell. So let's just remove the earlier code that no longer
is needed.
Fixes: 467d5c980709 ("usb: musb: Implement session bit based
runtime PM for musb-core")
Reported-by: Bin Liu
Signed-off-by: Tony Lindgren
---
drivers/usb/musb/musb_core.c | 15 ++-
drivers/usb/musb/musb_core.h | 1 -
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -594,11 +594,11 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
int_usb,
| MUSB_PORT_STAT_RESUME;
musb->rh_timer = jiffies
+ msecs_to_jiffies(
Re: [PATCH v6 23/25] usb: chipidea: Pullup D+ in device mode via phy APIs
On Wed, Jan 18, 2017 at 2:54 PM, Stephen Boyd wrote: > Quoting Peter Chen (2017-01-17 23:34:32) >> On Tue, Jan 17, 2017 at 09:58:33AM -0800, Stephen Boyd wrote: >> > Quoting Peter Chen (2017-01-15 19:45:51) >> > > >> > > At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need >> > > to port it to generic phy framework. >> > > >> > >> > Ok. I'll look into that. Can the other patches in this series be picked >> > up? Otherwise I can resend them all again once I fix the phy_set_mode() >> > call location and introduce a new phy op. >> >> I can pick up chipidea patches after you test the patch I supplied at: >> >> [PATCH v6 11/25] usb: chipidea: vbus event may exist before starting >> gadget > > Ok. I've confirmed that this updated patch works fine for me. You can > have my Tested-by and Reviewed-by there. > >> >> You may ping other maintainers to pick up other patches. >> > > I was hoping you could pick the beginning of the series up until the PHY > drivers, which can go via Kishon's tree. That would mean applying the > drivers/of/ part. Rob is that ok? Peter, If there's a dependency then please take the patches I've acked. That's why I acked them. Rob -- 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 v6 23/25] usb: chipidea: Pullup D+ in device mode via phy APIs
Quoting Peter Chen (2017-01-17 23:34:32) > On Tue, Jan 17, 2017 at 09:58:33AM -0800, Stephen Boyd wrote: > > Quoting Peter Chen (2017-01-15 19:45:51) > > > > > > At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need > > > to port it to generic phy framework. > > > > > > > Ok. I'll look into that. Can the other patches in this series be picked > > up? Otherwise I can resend them all again once I fix the phy_set_mode() > > call location and introduce a new phy op. > > I can pick up chipidea patches after you test the patch I supplied at: > > [PATCH v6 11/25] usb: chipidea: vbus event may exist before starting > gadget Ok. I've confirmed that this updated patch works fine for me. You can have my Tested-by and Reviewed-by there. > > You may ping other maintainers to pick up other patches. > I was hoping you could pick the beginning of the series up until the PHY drivers, which can go via Kishon's tree. That would mean applying the drivers/of/ part. Rob is that ok? -- 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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On Wed, Jan 18, 2017 at 10:44:32AM -0800, Tony Lindgren wrote:
> * Tony Lindgren [170118 10:15]:
> > * Grygorii Strashko [170118 10:05]:
> > >
> > >
> > > On 01/18/2017 10:53 AM, Tony Lindgren wrote:
> > > > Hi,
> > > >
> > > > * Bin Liu [170118 06:26]:
> > > >> With this v3, I still get -71 error when a device is plugged to a hub
> > > >> to
> > > >> musb. It doesn't happen though if the device is already plugged to the
> > > >> hub
> > > >> before attaching the hub to musb.
> > > >>
> > > >> [ 177.579300] usb 1-1: reset high-speed USB device number 2 using
> > > >> musb-hdrc
> > > >> [ 177.719308] usb 1-1: device descriptor read/64, error -71
> > > >> [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using
> > > >> musb-hdrc
> > > >
> > > > I think the -71 issue is a different regression. I've verified that
> > > > v4.8.y
> > > > does not have this, but v4.9.y does have it.
> > > >
> > > > So far the easiest way for me to reproduce the -71 problem is by
> > > > plugging
> > > > a USB drive into a connected hub. If I connect the hub with USB drive
> > > > already
> > > > plugged into the hub, it does not happen.
> ...
>
> > > Sry, but wouldn't be more simple and safe just to drop PM runtime from
> > > this driver,
> > > as it is part of musb and musb PM state expected to be handled properly
> > > by PM runtime now.
> >
> > Well we still need PM runtime in cppi41 to initialize it when it gets
> > probed and idle it when not used even if musb modules are not loaded.
> >
> > Unfortunately until musb_cppi41.c dma calls are fixed, we can't do
> > any sane use counting in cppi41.c without adding timeout handling
> > to it.
> >
> > > More over, There is another possibility related to the current PM runtime
> > > implementation and autosuspend
> > > - it might introduce delay between time when musb request desc push and
> > > time when cppi will actually
> > > put this desc in HW queue if cppi was not active.
> > > For example, there might not be -71 error seen if CPPI autosuspend is
> > > disabled
> > > (cppi is active all the time) before plug-in hub and then USB drive.
> >
> > I already checked that. The -71 error seems to be a separate issue.
>
> OK found it. I can make v4.8.17 produce -71 errors with just commit
> 467d5c980709 ("usb: musb: Implement session bit based runtime PM for
> musb-core") applied on top of it. So looks like we're missing some
> musb quirk handling for the devctl session bit.
Nice!
--
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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On 01/18/2017 12:15 PM, Tony Lindgren wrote: > * Grygorii Strashko [170118 10:05]: >> >> >> On 01/18/2017 10:53 AM, Tony Lindgren wrote: >>> Hi, >>> >>> * Bin Liu [170118 06:26]: With this v3, I still get -71 error when a device is plugged to a hub to musb. It doesn't happen though if the device is already plugged to the hub before attaching the hub to musb. [ 177.579300] usb 1-1: reset high-speed USB device number 2 using musb-hdrc [ 177.719308] usb 1-1: device descriptor read/64, error -71 [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using musb-hdrc >>> >>> I think the -71 issue is a different regression. I've verified that v4.8.y >>> does not have this, but v4.9.y does have it. >>> >>> So far the easiest way for me to reproduce the -71 problem is by plugging >>> a USB drive into a connected hub. If I connect the hub with USB drive >>> already >>> plugged into the hub, it does not happen. >>> >>> With the hub connected musb is already active when the USB drive is plugged >> >> This is not exactly try, i think, because cppi can be in inactive state >> because of autosuspend (all calls are paired). > > Musb is active when there's something connected meaning cppi41 is clocked > when a hub is connected. The actual runtime PM implementation happens for > the interconnect target module for both musb and cppi41 in hwmod.c code. > >>> in. So I'm now suspecting this -71 regression is not related to runtime PM >>> changes. Bisect and boot and plug devices time I think unless you have >>> better ideas? >>> And I still get -115 error flooding with thumb drive. [ 236.880068] cppi41-dma-engine 4740.dma-controller: cppi41_irq pm runtime get: -115 I tested with TI AM335x GP EVM. The problems happen on both musb ports. >>> >>> OK. So it's pointless to try to play with the autosuspend timeout. >>> >>> Let's just do a WARN(!pm_runtime_active(cdd->ddev.dev->parent)) there >>> until we have musb_cppi41.c dma calls properly paired and don't have >>> dma requests lingering. >>> >>> Care to try the updated patch below? It won't help with the -71 issue >>> but the $subject issue should be fixed. And you should not see the >>> WARN() trigger with your tests presumably. >>> >> >> Sry, but wouldn't be more simple and safe just to drop PM runtime from this >> driver, >> as it is part of musb and musb PM state expected to be handled properly by >> PM runtime now. > > Well we still need PM runtime in cppi41 to initialize it when it gets > probed and idle it when not used even if musb modules are not loaded. > > Unfortunately until musb_cppi41.c dma calls are fixed, we can't do > any sane use counting in cppi41.c without adding timeout handling > to it. > Just thinking, may be cppi41 should not be platform device at all and it might be reasonable to have it as lib with cppi41_init()/cppi41_remove(), so musb SoC glue layer will initialize it, because it provides services to other HW blocks withing musb only. -- regards, -grygorii -- 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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On Wed, Jan 18, 2017 at 12:48:59PM -0800, Tony Lindgren wrote:
> * Bin Liu [170118 10:49]:
> > On Wed, Jan 18, 2017 at 10:44:32AM -0800, Tony Lindgren wrote:
> > > * Tony Lindgren [170118 10:15]:
> > > > * Grygorii Strashko [170118 10:05]:
> > > > >
> > > > >
> > > > > On 01/18/2017 10:53 AM, Tony Lindgren wrote:
> > > > > > Hi,
> > > > > >
> > > > > > * Bin Liu [170118 06:26]:
> > > > > >> With this v3, I still get -71 error when a device is plugged to a
> > > > > >> hub to
> > > > > >> musb. It doesn't happen though if the device is already plugged to
> > > > > >> the hub
> > > > > >> before attaching the hub to musb.
> > > > > >>
> > > > > >> [ 177.579300] usb 1-1: reset high-speed USB device number 2 using
> > > > > >> musb-hdrc
> > > > > >> [ 177.719308] usb 1-1: device descriptor read/64, error -71
> > > > > >> [ 178.350297] usb 1-1.1: new high-speed USB device number 4 using
> > > > > >> musb-hdrc
> > > > > >
> > > > > > I think the -71 issue is a different regression. I've verified that
> > > > > > v4.8.y
> > > > > > does not have this, but v4.9.y does have it.
> > > > > >
> > > > > > So far the easiest way for me to reproduce the -71 problem is by
> > > > > > plugging
> > > > > > a USB drive into a connected hub. If I connect the hub with USB
> > > > > > drive already
> > > > > > plugged into the hub, it does not happen.
> > > ...
> > >
> > > > > Sry, but wouldn't be more simple and safe just to drop PM runtime
> > > > > from this driver,
> > > > > as it is part of musb and musb PM state expected to be handled
> > > > > properly by PM runtime now.
> > > >
> > > > Well we still need PM runtime in cppi41 to initialize it when it gets
> > > > probed and idle it when not used even if musb modules are not loaded.
> > > >
> > > > Unfortunately until musb_cppi41.c dma calls are fixed, we can't do
> > > > any sane use counting in cppi41.c without adding timeout handling
> > > > to it.
> > > >
> > > > > More over, There is another possibility related to the current PM
> > > > > runtime implementation and autosuspend
> > > > > - it might introduce delay between time when musb request desc push
> > > > > and time when cppi will actually
> > > > > put this desc in HW queue if cppi was not active.
> > > > > For example, there might not be -71 error seen if CPPI autosuspend is
> > > > > disabled
> > > > > (cppi is active all the time) before plug-in hub and then USB drive.
> > > >
> > > > I already checked that. The -71 error seems to be a separate issue.
> > >
> > > OK found it. I can make v4.8.17 produce -71 errors with just commit
> > > 467d5c980709 ("usb: musb: Implement session bit based runtime PM for
> > > musb-core") applied on top of it. So looks like we're missing some
> > > musb quirk handling for the devctl session bit.
> >
> > Nice!
>
> And here's a fix for the error -71 regression.
>
> Bin, can you review and test your earlier case mentioned in
> commit 9298b4aad37e ("usb: musb: fix device hotplug behind hub") with
> the patch below to make sure this is safe to do now starting with v4.9?
>
This solves the issue, and the patch looks good to me. Thanks for fixing
it up.
Regards,
-Bin.
> Regards,
>
> Tony
>
> 8< ---
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren
> Date: Wed, 18 Jan 2017 12:31:25 -0800
> Subject: [PATCH] usb: musb: Fix host mode error -71 regression
>
> Commit 467d5c980709 ("usb: musb: Implement session bit based runtime PM for
> musb-core") started implementing musb generic runtime PM support by
> introducing devctl register session bit based state control.
>
> This caused a regression where if a USB mass storage device is connected
> to a USB hub, we can get:
>
> usb 1-1: reset high-speed USB device number 2 using musb-hdrc
> usb 1-1: device descriptor read/64, error -71
> usb 1-1.1: new high-speed USB device number 4 using musb-hdrc
>
> This is because before the USB storage device is connected, musb is
> in OTG_STATE_A_SUSPEND. And we currently only set need_finish_resume
> in musb_stage0_irq() and the related code calling finish_resume_work
> in musb_resume() and musb_runtime_resume() never gets called.
>
> To fix the issue, we can call schedule_delayed_work() directly in
> musb_stage0_irq() to have finish_resume_work run.
>
> And we should no longer never get interrupts when when suspended.
> We have changed musb to no longer need pm_runtime_irqsafe().
> The need_finish_resume flag was added in commit 9298b4aad37e ("usb:
> musb: fix device hotplug behind hub") and no longer applies as far
> as I can tell. So let's just remove the earlier code that no longer
> is needed.
>
> Fixes: 467d5c980709 ("usb: musb: Implement session bit based
> runtime PM for musb-core")
> Reported-by: Bin Liu
> Signed-off-by: Tony Lindgren
> ---
> drivers/usb/musb/musb_core.c | 15 ++-
> drivers/usb/musb/musb_core.h | 1 -
> 2 files changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/musb/m
[PATCH 0/2] Two musb fixes for v4.10-rc cycle
Hi all, Here are two fixes for v4.10-rc cycle to deal with error -75 and -115 issues when plugging in USB mass storage device to a hub. Note that I will also post two cppi41 dma related patches, but those can be applied separately. Regards, Tony Tony Lindgren (2): usb: musb: Fix host mode error -71 regression usb: musb: Tiny dma in transfers won't complete with cpp41 drivers/usb/musb/musb_core.c | 15 ++- drivers/usb/musb/musb_core.h | 1 - drivers/usb/musb/musb_host.c | 9 - 3 files changed, 10 insertions(+), 15 deletions(-) -- 2.11.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 1/2] usb: musb: Fix host mode error -71 regression
Commit 467d5c980709 ("usb: musb: Implement session bit based runtime PM for
musb-core") started implementing musb generic runtime PM support by
introducing devctl register session bit based state control.
This caused a regression where if a USB mass storage device is connected
to a USB hub, we can get:
usb 1-1: reset high-speed USB device number 2 using musb-hdrc
usb 1-1: device descriptor read/64, error -71
usb 1-1.1: new high-speed USB device number 4 using musb-hdrc
This is because before the USB storage device is connected, musb is
in OTG_STATE_A_SUSPEND. And we currently only set need_finish_resume
in musb_stage0_irq() and the related code calling finish_resume_work
in musb_resume() and musb_runtime_resume() never gets called.
To fix the issue, we can call schedule_delayed_work() directly in
musb_stage0_irq() to have finish_resume_work run.
And we should no longer never get interrupts when when suspended.
We have changed musb to no longer need pm_runtime_irqsafe().
The need_finish_resume flag was added in commit 9298b4aad37e ("usb:
musb: fix device hotplug behind hub") and no longer applies as far
as I can tell. So let's just remove the earlier code that no longer
is needed.
Fixes: 467d5c980709 ("usb: musb: Implement session bit based
runtime PM for musb-core")
Reported-by: Bin Liu
Signed-off-by: Tony Lindgren
---
drivers/usb/musb/musb_core.c | 15 ++-
drivers/usb/musb/musb_core.h | 1 -
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -594,11 +594,11 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
int_usb,
| MUSB_PORT_STAT_RESUME;
musb->rh_timer = jiffies
+ msecs_to_jiffies(USB_RESUME_TIMEOUT);
- musb->need_finish_resume = 1;
-
musb->xceiv->otg->state = OTG_STATE_A_HOST;
musb->is_active = 1;
musb_host_resume_root_hub(musb);
+ schedule_delayed_work(&musb->finish_resume_work,
+ msecs_to_jiffies(USB_RESUME_TIMEOUT));
break;
case OTG_STATE_B_WAIT_ACON:
musb->xceiv->otg->state =
OTG_STATE_B_PERIPHERAL;
@@ -2710,11 +2710,6 @@ static int musb_resume(struct device *dev)
mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
if ((devctl & mask) != (musb->context.devctl & mask))
musb->port1_status = 0;
- if (musb->need_finish_resume) {
- musb->need_finish_resume = 0;
- schedule_delayed_work(&musb->finish_resume_work,
- msecs_to_jiffies(USB_RESUME_TIMEOUT));
- }
/*
* The USB HUB code expects the device to be in RPM_ACTIVE once it came
@@ -2766,12 +2761,6 @@ static int musb_runtime_resume(struct device *dev)
musb_restore_context(musb);
- if (musb->need_finish_resume) {
- musb->need_finish_resume = 0;
- schedule_delayed_work(&musb->finish_resume_work,
- msecs_to_jiffies(USB_RESUME_TIMEOUT));
- }
-
spin_lock_irqsave(&musb->lock, flags);
error = musb_run_resume_work(musb);
if (error)
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -410,7 +410,6 @@ struct musb {
/* is_suspended means USB B_PERIPHERAL suspend */
unsignedis_suspended:1;
- unsignedneed_finish_resume :1;
/* may_wakeup means remote wakeup is enabled */
unsignedmay_wakeup:1;
--
2.11.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 2/2] usb: musb: Tiny dma in transfers won't complete with cpp41
At least with the cppi41 dma, size 1 in dma transfers will just wait
until the device is disconnected. And it also seems that enumerating
a USB stick with a hub can take a USB reset with smallish size in
transfers.
This causes timeouts in cppi41 dma runtime PM.
Fix the issue by adding a minimum size of 16 which is based on my
obeservations on BeagleBone to make enumerating more reliable.
Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Signed-off-by: Tony Lindgren
---
drivers/usb/musb/musb_host.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -743,7 +743,14 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
musb_ep_select(mbase, epnum);
- if (is_out && !len) {
+ /*
+* Skip dma for zero sized out and small in transfers.
+* At least cppi41 in dma will just hang with size of 1 until the
+* device is connected. For sizes 8 and less it seems to take a
+* long time to complete. Let's use minimum size of 16 to avoid
+* tiny in DMA transfers.
+*/
+ if ((is_out && !len) || (!is_out && (len < 16))) {
use_dma = 0;
csr = musb_readw(epio, MUSB_TXCSR);
csr &= ~MUSB_TXCSR_DMAENAB;
--
2.11.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: [PATCHv3] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Bin Liu [170118 13:14]:
> On Wed, Jan 18, 2017 at 12:48:59PM -0800, Tony Lindgren wrote:
> > And here's a fix for the error -71 regression.
> >
> > Bin, can you review and test your earlier case mentioned in
> > commit 9298b4aad37e ("usb: musb: fix device hotplug behind hub") with
> > the patch below to make sure this is safe to do now starting with v4.9?
> >
>
> This solves the issue, and the patch looks good to me. Thanks for fixing
> it up.
OK will post a two patch musb fixes series with this and the
short in dma fix as that's what's keeping cpp41 channels hanging.
Then after that I'll post a series of two cpp41 fixes.
Regards,
Tony
--
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 2/2] usb: musb: Tiny dma in transfers won't complete with cpp41
* Tony Lindgren [170118 16:50]:
> At least with the cppi41 dma, size 1 in dma transfers will just wait
> until the device is disconnected. And it also seems that enumerating
> a USB stick with a hub can take a USB reset with smallish size in
> transfers.
>
> This causes timeouts in cppi41 dma runtime PM.
>
> Fix the issue by adding a minimum size of 16 which is based on my
> obeservations on BeagleBone to make enumerating more reliable.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Signed-off-by: Tony Lindgren
> ---
> drivers/usb/musb/musb_host.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -743,7 +743,14 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
>
> musb_ep_select(mbase, epnum);
>
> - if (is_out && !len) {
> + /*
> + * Skip dma for zero sized out and small in transfers.
> + * At least cppi41 in dma will just hang with size of 1 until the
> + * device is connected. For sizes 8 and less it seems to take a
> + * long time to complete. Let's use minimum size of 16 to avoid
> + * tiny in DMA transfers.
> + */
> + if ((is_out && !len) || (!is_out && (len < 16))) {
> use_dma = 0;
> csr = musb_readw(epio, MUSB_TXCSR);
> csr &= ~MUSB_TXCSR_DMAENAB;
Uh I think this needs a separate check to not mess with the TXCSR
for in transfers.
Regards,
Tony
--
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] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
On 1/16/2017 2:38 AM, Felipe Balbi wrote: > > Hi, > > John Youn writes: >>> Baolin Wang writes: > Baolin Wang writes: >> When dwc3 controller acts as host role with attaching slow speed device >> (like mouse or keypad). Then if we plugged out the slow speed device, >> it will timeout to run the deconfiguration endpoint command to drop the >> endpoint's resources. Some xHCI command timeout log as below when >> disconnecting one slow device: >> >> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1 >> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub >> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port >> polling. >> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status >> = 0x202a0 >> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100 >> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual >> port 0 status = 0x2a0 >> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffc01ed6cd00, dev 1, >> ep 0x81, starting at offset 0xc406d210 >> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong! >> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB >> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at >> 0xc406d210 (dma). >> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context >> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1 >> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment = >> ffc1112f0880 (virtual) >> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA) >> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg = >> ffc1112f0880 (0xc406d000 dma), >> new deq ptr = ff8002175220 >> (0xc406d220 dma), new cycle = 1 >> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong! >> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd, >> deq = @c406d220 >> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port >> polling. >> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev >> ffc01ae08800 >> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop >> flags = 0x8, new add flags = 0x0 >> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev >> ffc01ae08800 >> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context: >> >> .. >> >> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong! >> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout >> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring >> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not >> match >> command >> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure >> endpoint command >> >> The reason is it will suspend USB phy to disable phy clock when >> disconnecting the slow USB decice, that will hang on the xHCI commands >> executing which depends on the phy clock. >> >> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host >> role. >> >> Signed-off-by: Baolin Wang >> --- >> drivers/usb/dwc3/core.c | 14 ++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 9a4a5e4..0b646cf 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc) >> if (dwc->revision > DWC3_REVISION_194A) >> reg |= DWC3_GUSB2PHYCFG_SUSPHY; >> >> + /* >> + * When dwc3 controller acts as host role with attaching one slow >> speed >> + * device (like mouse or keypad). Then if we plugged out the slow >> speed >> + * device, it will timeout to run the deconfiguration endpoint >> command. >> + * The reason is it will suspend USB phy to disable phy clock when >> + * disconnecting slow speed decice, which will affect the xHCI >> commands >> + * executing. >> + * >> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 >> acts as >> + * host role. >> + */ >> + if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->dr_mode == >> USB_DR_MODE_OTG) >> + reg &=
[PATCHv2 0/2] Two musb fixes for v4.10-rc cycle
Hi all, Here are two fixes for v4.10-rc cycle to deal with error -75 and -115 issues when plugging in USB mass storage device to a hub. Note that I will also post two cppi41 dma related patches, but those can be applied separately. Regards, Tony Changes since v1: - Handle in transfer quirk separately with a cpp41 flag and don't trash the TX registers in RX dma quirk - Limit quirk to cpp41 in dma sizes 0 - 1 only - Update description accordingly Tony Lindgren (2): usb: musb: Fix host mode error -71 regression usb: musb: Size 1 dma in transfers won't complete with cpp41 drivers/usb/musb/musb_core.c | 15 ++- drivers/usb/musb/musb_core.h | 1 - drivers/usb/musb/musb_cppi41.c | 1 + drivers/usb/musb/musb_dma.h| 3 +++ drivers/usb/musb/musb_host.c | 16 +++- 5 files changed, 21 insertions(+), 15 deletions(-) -- 2.11.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 2/4] usb: dwc2: host: Correct snpsid checking for GDFIFOCFG
From: Sevak Arakelyan
GDFIFOCFG is available from IP version 2.91a. Fix the code to reflect
this.
Signed-off-by: Sevak Arakelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/hcd.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index a473853ca39c..5370e6429f28 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -940,6 +940,7 @@ struct dwc2_hsotg {
/* DWC OTG HW Release versions */
#define DWC2_CORE_REV_2_71a0x4f54271a
#define DWC2_CORE_REV_2_90a0x4f54290a
+#define DWC2_CORE_REV_2_91a0x4f54291a
#define DWC2_CORE_REV_2_92a0x4f54292a
#define DWC2_CORE_REV_2_94a0x4f54294a
#define DWC2_CORE_REV_3_00a0x4f54300a
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 6d399485501f..aca05ac1514c 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -494,8 +494,9 @@ static void dwc2_config_fifos(struct dwc2_hsotg *hsotg)
dwc2_readl(hsotg->regs + HPTXFSIZ));
if (hsotg->params.en_multiple_tx_fifo &&
- hsotg->hw_params.snpsid <= DWC2_CORE_REV_2_94a) {
+ hsotg->hw_params.snpsid >= DWC2_CORE_REV_2_91a) {
/*
+* This feature was implemented in 2.91a version
* Global DFIFOCFG calculation for Host mode -
* include RxFIFO, NPTXFIFO and HPTXFIFO
*/
--
2.11.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 1/4] usb: dwc2: gadget: Set GDFIFOCFG
From: Sevak Arakelyan Add programming of GDFIFOCFG register in device mode. It must contain start address for EP Info block and total FIFO depth. Signed-off-by: Sevak Arakelyan Signed-off-by: John Youn --- drivers/usb/dwc2/gadget.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 288402d44fce..bfda1cfd4fa6 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -241,6 +241,9 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg) val = dwc2_readl(hsotg->regs + DPTXFSIZN(ep)); } + dwc2_writel(hsotg->hw_params.total_fifo_size | + addr << GDFIFOCFG_EPINFOBASE_SHIFT, + hsotg->regs + GDFIFOCFG); /* * according to p428 of the design guide, we need to ensure that * all fifos are flushed before continuing -- 2.11.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 0/4] usb: dwc2: FIFO configuration
This series addresses a couple of FIFO issues in dwc2. The first it properly configures the GDIFIFOCFG registers in host/gadget mode. Next it configures the TX FIFOs to default values in gadget mode. These values should work for all cores. The old constant values were incorrect in some cases depending on the core configuration. These value may still be overridden on a per-platform basis and we add checks to make sure that overridden values are consistent with the hardware capabilities and fall-back to defaults if so. Test on Synopsys HAPS core version 3.30a. Appreciate any Tested-by's on other platforms. This series depends on the latest dwc2 patches for next and 4.10-rc5 [1] which you can find applied to my git tree for convenience [2]. Regards, John [1] https://www.spinics.net/lists/linux-usb/msg152258.html [2] https://github.com/synopsys-usb/linux.git Sevak Arakelyan (4): usb: dwc2: gadget: Set GDFIFOCFG usb: dwc2: host: Correct snpsid checking for GDFIFOCFG usb: dwc2: gadget: Set TX FIFO depths to calculated defaults usb: dwc2: gadget: Add checking for g-tx-fifo-size parameter drivers/usb/dwc2/core.h | 17 + drivers/usb/dwc2/gadget.c | 96 +++ drivers/usb/dwc2/hcd.c| 3 +- drivers/usb/dwc2/params.c | 47 +-- 4 files changed, 151 insertions(+), 12 deletions(-) -- 2.11.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 3/4] usb: dwc2: gadget: Set TX FIFO depths to calculated defaults
From: Sevak Arakelyan
Remove legacy DWC2_G_P_LEGACY_TX_FIFO_SIZE array for TX FIFOs.
Update dwc2_set_param_tx_fifo_sizes function to calculate
and assign default average FIFO depth to each member of
g_tx_fifo_size array. Total FIFO size, EP Info block's size,
FIFO operation mode and device operation mode are taken into
consideration during the calculation.
Cc: Stefan Wahren
Signed-off-by: Sevak Arakelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/core.h | 16
drivers/usb/dwc2/gadget.c | 93 +++
drivers/usb/dwc2/params.c | 12 --
3 files changed, 110 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 5370e6429f28..f10eca91d2be 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -274,13 +274,6 @@ enum dwc2_lx_state {
DWC2_L3,/* Off state */
};
-/*
- * Gadget periodic tx fifo sizes as used by legacy driver
- * EP0 is not included
- */
-#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, \
- 768, 0, 0, 0, 0, 0, 0, 0}
-
/* Gadget ep0 states */
enum dwc2_ep0_state {
DWC2_EP0_SETUP,
@@ -1180,6 +1173,9 @@ int dwc2_hsotg_set_test_mode(struct dwc2_hsotg *hsotg,
int testmode);
#define dwc2_is_device_connected(hsotg) (hsotg->connected)
int dwc2_backup_device_registers(struct dwc2_hsotg *hsotg);
int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg);
+int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg);
+int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg);
+int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg);
#else
static inline int dwc2_hsotg_remove(struct dwc2_hsotg *dwc2)
{ return 0; }
@@ -1201,6 +1197,12 @@ static inline int dwc2_backup_device_registers(struct
dwc2_hsotg *hsotg)
{ return 0; }
static inline int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg)
{ return 0; }
+static inline int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg)
+{ return 0; }
#endif
#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index bfda1cfd4fa6..bc3b3fda5000 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -191,6 +191,99 @@ static void dwc2_hsotg_ctrl_epint(struct dwc2_hsotg *hsotg,
local_irq_restore(flags);
}
+/**
+ * dwc2_hsotg_tx_fifo_count - return count of TX FIFOs in device mode
+ */
+int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg)
+{
+ if (hsotg->hw_params.en_multiple_tx_fifo)
+ /* In dedicated FIFO mode we need count of IN EPs */
+ return (dwc2_readl(hsotg->regs + GHWCFG4) &
+ GHWCFG4_NUM_IN_EPS_MASK) >> GHWCFG4_NUM_IN_EPS_SHIFT;
+ else
+ /* In shared FIFO mode we need count of Periodic IN EPs */
+ return hsotg->hw_params.num_dev_perio_in_ep;
+}
+
+/**
+ * dwc2_hsotg_ep_info_size - return Endpoint Info Control block size in DWORDs
+ */
+static int dwc2_hsotg_ep_info_size(struct dwc2_hsotg *hsotg)
+{
+ int val = 0;
+ int i;
+ u32 ep_dirs;
+
+ /*
+* Don't need additional space for ep info control registers in
+* slave mode.
+*/
+ if (!using_dma(hsotg)) {
+ dev_dbg(hsotg->dev, "Buffer DMA ep info size 0\n");
+ return 0;
+ }
+
+ /*
+* Buffer DMA mode - 1 location per endpoit
+* Descriptor DMA mode - 4 locations per endpoint
+*/
+ ep_dirs = hsotg->hw_params.dev_ep_dirs;
+
+ for (i = 0; i <= hsotg->hw_params.num_dev_ep; i++) {
+ val += ep_dirs & 3 ? 1 : 2;
+ ep_dirs >>= 2;
+ }
+
+ if (using_desc_dma(hsotg))
+ val = val * 4;
+
+ return val;
+}
+
+/**
+ * dwc2_hsotg_tx_fifo_total_depth - return total FIFO depth available for
+ * device mode TX FIFOs
+ */
+int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
+{
+ int ep_info_size;
+ int addr;
+ int tx_addr_max;
+ u32 np_tx_fifo_size;
+
+ np_tx_fifo_size = min_t(u32, hsotg->hw_params.dev_nperio_tx_fifo_size,
+ hsotg->params.g_np_tx_fifo_size);
+
+ /* Get Endpoint Info Control block size in DWORDs. */
+ ep_info_size = dwc2_hsotg_ep_info_size(hsotg);
+ tx_addr_max = hsotg->hw_params.total_fifo_size - ep_info_size;
+
+ addr = hsotg->params.g_rx_fifo_size + np_tx_fifo_size;
+ if (tx_addr_max <= addr)
+ return 0;
+
+ return tx_addr_max - addr;
+}
+
+/**
+ * dwc2_hsotg_tx_fifo_average_depth - returns average depth of device mode
+ * TX FIFOs
+ */
+int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg
[PATCH 4/4] usb: dwc2: gadget: Add checking for g-tx-fifo-size parameter
From: Sevak Arakelyan
Add dwc2_check_param_tx_fifo_sizes function which validates
the members of g_tx_fifo_size array and sets to average or
default values if it is needed.
Cc: Stefan Wahren
Signed-off-by: Sevak Arakelyan
Signed-off-by: John Youn
---
drivers/usb/dwc2/params.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 016fff0cb887..2990c347289f 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -427,6 +427,40 @@ static void dwc2_check_param_phy_utmi_width(struct
dwc2_hsotg *hsotg)
dwc2_set_param_phy_utmi_width(hsotg);
}
+static void dwc2_check_param_tx_fifo_sizes(struct dwc2_hsotg *hsotg)
+{
+ int fifo_count;
+ int fifo;
+ int min;
+ u32 total = 0;
+ u32 dptxfszn;
+
+ fifo_count = dwc2_hsotg_tx_fifo_count(hsotg);
+ min = hsotg->hw_params.en_multiple_tx_fifo ? 16 : 4;
+
+ for (fifo = 1; fifo <= fifo_count; fifo++)
+ total += hsotg->params.g_tx_fifo_size[fifo];
+
+ if (total > dwc2_hsotg_tx_fifo_total_depth(hsotg) || !total) {
+ dev_warn(hsotg->dev, "%s: Invalid parameter g-tx-fifo-size,
setting to default average\n",
+__func__);
+ dwc2_set_param_tx_fifo_sizes(hsotg);
+ }
+
+ for (fifo = 1; fifo <= fifo_count; fifo++) {
+ dptxfszn = (dwc2_readl(hsotg->regs + DPTXFSIZN(fifo)) &
+ FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
+
+ if (hsotg->params.g_tx_fifo_size[fifo] < min ||
+ hsotg->params.g_tx_fifo_size[fifo] > dptxfszn) {
+ dev_warn(hsotg->dev, "%s: Invalid parameter
g_tx_fifo_size[%d]=%d\n",
+__func__, fifo,
+hsotg->params.g_tx_fifo_size[fifo]);
+ hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
+ }
+ }
+}
+
#define CHECK_RANGE(_param, _min, _max, _def) do { \
if ((hsotg->params._param) < (_min) || \
(hsotg->params._param) > (_max)) { \
@@ -496,6 +530,7 @@ static void dwc2_check_params(struct dwc2_hsotg *hsotg)
CHECK_RANGE(g_np_tx_fifo_size,
16, hw->dev_nperio_tx_fifo_size,
hw->dev_nperio_tx_fifo_size);
+ dwc2_check_param_tx_fifo_sizes(hsotg);
}
}
--
2.11.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 2/2] usb: musb: Size 1 dma in transfers won't complete with cpp41
At least with the cppi41 dma, size 1 in dma transfers will just wait
until the device is disconnected. This causes timeouts in cppi41 dma
runtime PM.
Also the initial size 8 transfers take about 200ms to complete when
plugging a USB mass storage device to a hub. But we probably want to
keep those to avoid using PIO.
Fix the issue by adding a quirk for cppi41 and skip size 1 in dma if
set.
Note that additional cpp41 patches are needed to avoid error -115
messages, but that can be applied separately.
Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Signed-off-by: Tony Lindgren
---
drivers/usb/musb/musb_cppi41.c | 1 +
drivers/usb/musb/musb_dma.h| 3 +++
drivers/usb/musb/musb_host.c | 16 +++-
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -695,6 +695,7 @@ cppi41_dma_controller_create(struct musb *musb, void
__iomem *base)
controller->controller.channel_program = cppi41_dma_channel_program;
controller->controller.channel_abort = cppi41_dma_channel_abort;
controller->controller.is_compatible = cppi41_is_compatible;
+ controller->controller.quirks = MUSB_DMA_QUIRK_CPPI41_IN;
ret = cppi41_dma_controller_start(controller);
if (ret)
diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
--- a/drivers/usb/musb/musb_dma.h
+++ b/drivers/usb/musb/musb_dma.h
@@ -171,6 +171,8 @@ dma_channel_status(struct dma_channel *c)
return (is_dma_capable() && c) ? c->status : MUSB_DMA_STATUS_UNKNOWN;
}
+#define MUSB_DMA_QUIRK_CPPI41_IN BIT(0)
+
/**
* struct dma_controller - A DMA Controller.
* @start: call this to start a DMA controller;
@@ -196,6 +198,7 @@ struct dma_controller {
int (*is_compatible)(struct dma_channel *channel,
u16 maxpacket,
void *buf, u32 length);
+ unsigned int quirks;
};
/* called after channel_program(), may indicate a fault */
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -743,6 +743,8 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
musb_ep_select(mbase, epnum);
+ dma_controller = musb->dma_controller;
+
if (is_out && !len) {
use_dma = 0;
csr = musb_readw(epio, MUSB_TXCSR);
@@ -751,8 +753,20 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
hw_ep->tx_channel = NULL;
}
+ /*
+* At least cppi41 in dma will just hang with size of 1 until the
+* device is disconnected.
+*/
+ if (!is_out && dma_controller &&
+ (dma_controller->quirks & MUSB_DMA_QUIRK_CPPI41_IN)) {
+ use_dma = 0;
+ csr = musb_readw(epio, MUSB_RXCSR);
+ csr &= ~MUSB_RXCSR_DMAENAB;
+ musb_writew(epio, MUSB_RXCSR, csr);
+ hw_ep->rx_channel = NULL;
+ }
+
/* candidate for DMA? */
- dma_controller = musb->dma_controller;
if (use_dma && is_dma_capable() && epnum && dma_controller) {
dma_channel = is_out ? hw_ep->tx_channel : hw_ep->rx_channel;
if (!dma_channel) {
--
2.11.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 1/2] usb: musb: Fix host mode error -71 regression
Commit 467d5c980709 ("usb: musb: Implement session bit based runtime PM for
musb-core") started implementing musb generic runtime PM support by
introducing devctl register session bit based state control.
This caused a regression where if a USB mass storage device is connected
to a USB hub, we can get:
usb 1-1: reset high-speed USB device number 2 using musb-hdrc
usb 1-1: device descriptor read/64, error -71
usb 1-1.1: new high-speed USB device number 4 using musb-hdrc
This is because before the USB storage device is connected, musb is
in OTG_STATE_A_SUSPEND. And we currently only set need_finish_resume
in musb_stage0_irq() and the related code calling finish_resume_work
in musb_resume() and musb_runtime_resume() never gets called.
To fix the issue, we can call schedule_delayed_work() directly in
musb_stage0_irq() to have finish_resume_work run.
And we should no longer never get interrupts when when suspended.
We have changed musb to no longer need pm_runtime_irqsafe().
The need_finish_resume flag was added in commit 9298b4aad37e ("usb:
musb: fix device hotplug behind hub") and no longer applies as far
as I can tell. So let's just remove the earlier code that no longer
is needed.
Fixes: 467d5c980709 ("usb: musb: Implement session bit based
runtime PM for musb-core")
Reported-by: Bin Liu
Signed-off-by: Tony Lindgren
---
drivers/usb/musb/musb_core.c | 15 ++-
drivers/usb/musb/musb_core.h | 1 -
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -594,11 +594,11 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8
int_usb,
| MUSB_PORT_STAT_RESUME;
musb->rh_timer = jiffies
+ msecs_to_jiffies(USB_RESUME_TIMEOUT);
- musb->need_finish_resume = 1;
-
musb->xceiv->otg->state = OTG_STATE_A_HOST;
musb->is_active = 1;
musb_host_resume_root_hub(musb);
+ schedule_delayed_work(&musb->finish_resume_work,
+ msecs_to_jiffies(USB_RESUME_TIMEOUT));
break;
case OTG_STATE_B_WAIT_ACON:
musb->xceiv->otg->state =
OTG_STATE_B_PERIPHERAL;
@@ -2710,11 +2710,6 @@ static int musb_resume(struct device *dev)
mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
if ((devctl & mask) != (musb->context.devctl & mask))
musb->port1_status = 0;
- if (musb->need_finish_resume) {
- musb->need_finish_resume = 0;
- schedule_delayed_work(&musb->finish_resume_work,
- msecs_to_jiffies(USB_RESUME_TIMEOUT));
- }
/*
* The USB HUB code expects the device to be in RPM_ACTIVE once it came
@@ -2766,12 +2761,6 @@ static int musb_runtime_resume(struct device *dev)
musb_restore_context(musb);
- if (musb->need_finish_resume) {
- musb->need_finish_resume = 0;
- schedule_delayed_work(&musb->finish_resume_work,
- msecs_to_jiffies(USB_RESUME_TIMEOUT));
- }
-
spin_lock_irqsave(&musb->lock, flags);
error = musb_run_resume_work(musb);
if (error)
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -410,7 +410,6 @@ struct musb {
/* is_suspended means USB B_PERIPHERAL suspend */
unsignedis_suspended:1;
- unsignedneed_finish_resume :1;
/* may_wakeup means remote wakeup is enabled */
unsignedmay_wakeup:1;
--
2.11.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] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
On 1/18/2017 7:12 PM, Baolin Wang wrote: > Hi John, > > On 19 January 2017 at 09:33, John Youn wrote: >> On 1/16/2017 2:38 AM, Felipe Balbi wrote: >>> >>> Hi, >>> >>> John Youn writes: > Baolin Wang writes: >>> Baolin Wang writes: When dwc3 controller acts as host role with attaching slow speed device (like mouse or keypad). Then if we plugged out the slow speed device, it will timeout to run the deconfiguration endpoint command to drop the endpoint's resources. Some xHCI command timeout log as below when disconnecting one slow device: [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1 [ 99.814699] c0 xhci-hcd.0.auto: resume root hub [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port polling. [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status = 0x202a0 [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100 [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual port 0 status = 0x2a0 [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffc01ed6cd00, dev 1, ep 0x81, starting at offset 0xc406d210 [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong! [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at 0xc406d210 (dma). [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1 [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment = ffc1112f0880 (virtual) [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 (DMA) [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg = ffc1112f0880 (0xc406d000 dma), new deq ptr = ff8002175220 (0xc406d220 dma), new cycle = 1 [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong! [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd, deq = @c406d220 [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port polling. [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev ffc01ae08800 [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop flags = 0x8, new add flags = 0x0 [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev ffc01ae08800 [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context: .. [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong! [ 105.430728] c0 xhci-hcd.0.auto: Command timeout [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not match command [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure endpoint command The reason is it will suspend USB phy to disable phy clock when disconnecting the slow USB decice, that will hang on the xHCI commands executing which depends on the phy clock. Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host role. Signed-off-by: Baolin Wang --- drivers/usb/dwc3/core.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 9a4a5e4..0b646cf 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc) if (dwc->revision > DWC3_REVISION_194A) reg |= DWC3_GUSB2PHYCFG_SUSPHY; + /* + * When dwc3 controller acts as host role with attaching one slow speed + * device (like mouse or keypad). Then if we plugged out the slow speed + * device, it will timeout to run the deconfiguration endpoint command. + * The reason is it will suspend USB phy to disable phy clock when + * disconnecting slow speed decice, which will affect the
Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
Hi John, On 19 January 2017 at 09:33, John Youn wrote: > On 1/16/2017 2:38 AM, Felipe Balbi wrote: >> >> Hi, >> >> John Youn writes: Baolin Wang writes: >> Baolin Wang writes: >>> When dwc3 controller acts as host role with attaching slow speed device >>> (like mouse or keypad). Then if we plugged out the slow speed device, >>> it will timeout to run the deconfiguration endpoint command to drop the >>> endpoint's resources. Some xHCI command timeout log as below when >>> disconnecting one slow device: >>> >>> [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1 >>> [ 99.814699] c0 xhci-hcd.0.auto: resume root hub >>> [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port >>> polling. >>> [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 status >>> = 0x202a0 >>> [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100 >>> [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual >>> port 0 status = 0x2a0 >>> [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffc01ed6cd00, dev 1, >>> ep 0x81, starting at offset 0xc406d210 >>> [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong! >>> [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB >>> [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at >>> 0xc406d210 (dma). >>> [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context >>> [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1 >>> [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment = >>> ffc1112f0880 (virtual) >>> [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 >>> (DMA) >>> [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg = >>> ffc1112f0880 (0xc406d000 dma), >>> new deq ptr = ff8002175220 >>> (0xc406d220 dma), new cycle = 1 >>> [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong! >>> [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd, >>> deq = @c406d220 >>> [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port >>> polling. >>> [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev >>> ffc01ae08800 >>> [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop >>> flags = 0x8, new add flags = 0x0 >>> [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for udev >>> ffc01ae08800 >>> [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context: >>> >>> .. >>> >>> [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong! >>> [ 105.430728] c0 xhci-hcd.0.auto: Command timeout >>> [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring >>> [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not >>> match >>> command >>> [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure >>> endpoint command >>> >>> The reason is it will suspend USB phy to disable phy clock when >>> disconnecting the slow USB decice, that will hang on the xHCI commands >>> executing which depends on the phy clock. >>> >>> Thus we should disable USB2.0 phy suspend feature when dwc3 acts as host >>> role. >>> >>> Signed-off-by: Baolin Wang >>> --- >>> drivers/usb/dwc3/core.c | 14 ++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 9a4a5e4..0b646cf 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc) >>> if (dwc->revision > DWC3_REVISION_194A) >>> reg |= DWC3_GUSB2PHYCFG_SUSPHY; >>> >>> + /* >>> + * When dwc3 controller acts as host role with attaching one slow >>> speed >>> + * device (like mouse or keypad). Then if we plugged out the slow >>> speed >>> + * device, it will timeout to run the deconfiguration endpoint >>> command. >>> + * The reason is it will suspend USB phy to disable phy clock when >>> + * disconnecting slow speed decice, which will affect the xHCI >>> commands >>> + * executing. >>> + * >>> + * Thus we should disable USB 2.0 phy suspend feature when dwc3 >>> acts as >>>
Re: [PATCH] usb: dwc3: core: Disable USB2.0 phy suspend when dwc3 acts as host role
Hi John, On 19 January 2017 at 11:31, John Youn wrote: > On 1/18/2017 7:12 PM, Baolin Wang wrote: >> Hi John, >> >> On 19 January 2017 at 09:33, John Youn wrote: >>> On 1/16/2017 2:38 AM, Felipe Balbi wrote: Hi, John Youn writes: >> Baolin Wang writes: Baolin Wang writes: > When dwc3 controller acts as host role with attaching slow speed > device > (like mouse or keypad). Then if we plugged out the slow speed device, > it will timeout to run the deconfiguration endpoint command to drop > the > endpoint's resources. Some xHCI command timeout log as below when > disconnecting one slow device: > > [ 99.807739] c0 xhci-hcd.0.auto: Port Status Change Event for port 1 > [ 99.814699] c0 xhci-hcd.0.auto: resume root hub > [ 99.819992] c0 xhci-hcd.0.auto: handle_port_status: starting port > polling. > [ 99.827808] c0 xhci-hcd.0.auto: get port status, actual port 0 > status > = 0x202a0 > [ 99.835903] c0 xhci-hcd.0.auto: Get port status returned 0x10100 > [ 99.850052] c0 xhci-hcd.0.auto: clear port connect change, actual > port 0 status = 0x2a0 > [ 99.859313] c0 xhci-hcd.0.auto: Cancel URB ffc01ed6cd00, dev 1, > ep 0x81, starting at offset > 0xc406d210 > [ 99.869645] c0 xhci-hcd.0.auto: // Ding dong! > [ 99.874776] c0 xhci-hcd.0.auto: Stopped on Transfer TRB > [ 99.880713] c0 xhci-hcd.0.auto: Removing canceled TD starting at > 0xc406d210 (dma). > [ 99.889012] c0 xhci-hcd.0.auto: Finding endpoint context > [ 99.895069] c0 xhci-hcd.0.auto: Cycle state = 0x1 > [ 99.900519] c0 xhci-hcd.0.auto: New dequeue segment = > ffc1112f0880 (virtual) > [ 99.908655] c0 xhci-hcd.0.auto: New dequeue pointer = 0xc406d220 > (DMA) > [ 99.915927] c0 xhci-hcd.0.auto: Set TR Deq Ptr cmd, new deq seg = > ffc1112f0880 (0xc406d000 dma), > new deq ptr = ff8002175220 > (0xc406d220 dma), new cycle = 1 > [ 99.931242] c0 xhci-hcd.0.auto: // Ding dong! > [ 99.936360] c0 xhci-hcd.0.auto: Successful Set TR Deq Ptr cmd, > deq = @c406d220 > [ 99.944458] c0 xhci-hcd.0.auto: xhci_hub_status_data: stopping port > polling. > [ 100.047619] c0 xhci-hcd.0.auto: xhci_drop_endpoint called for udev > ffc01ae08800 > [ 100.057002] c0 xhci-hcd.0.auto: drop ep 0x81, slot id 1, new drop > flags = 0x8, new add flags = 0x0 > [ 100.067878] c0 xhci-hcd.0.auto: xhci_check_bandwidth called for > udev > ffc01ae08800 > [ 100.076868] c0 xhci-hcd.0.auto: New Input Control Context: > > .. > > [ 100.427252] c0 xhci-hcd.0.auto: // Ding dong! > [ 105.430728] c0 xhci-hcd.0.auto: Command timeout > [ 105.436029] c0 xhci-hcd.0.auto: Abort command ring > [ 113.558223] c0 xhci-hcd.0.auto: Command completion event does not > match > command > [ 113.569778] c0 xhci-hcd.0.auto: Timeout while waiting for configure > endpoint command > > The reason is it will suspend USB phy to disable phy clock when > disconnecting the slow USB decice, that will hang on the xHCI commands > executing which depends on the phy clock. > > Thus we should disable USB2.0 phy suspend feature when dwc3 acts as > host > role. > > Signed-off-by: Baolin Wang > --- > drivers/usb/dwc3/core.c | 14 ++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 9a4a5e4..0b646cf 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -565,6 +565,20 @@ static int dwc3_phy_setup(struct dwc3 *dwc) > if (dwc->revision > DWC3_REVISION_194A) > reg |= DWC3_GUSB2PHYCFG_SUSPHY; > > + /* > + * When dwc3 controller acts as host role with attaching one > slow speed > + * device (like mouse or keypad). Then if we plugged out the > slow speed > + * device, it will timeout to run the deconfigurat
Re: [PATCH 2/2] usb: musb: Size 1 dma in transfers won't complete with cpp41
On Wed, Jan 18, 2017 at 06:29:59PM -0800, Tony Lindgren wrote:
> At least with the cppi41 dma, size 1 in dma transfers will just wait
In which case do you see the size 1 transfer? using testusb?
> until the device is disconnected. This causes timeouts in cppi41 dma
> runtime PM.
>
> Also the initial size 8 transfers take about 200ms to complete when
> plugging a USB mass storage device to a hub. But we probably want to
> keep those to avoid using PIO.
>
> Fix the issue by adding a quirk for cppi41 and skip size 1 in dma if
> set.
It is fine to bypass dma for size 1 transfers, due to the dma setup
overhead. But I'd like to know the test case to understand why it hangs.
>
> Note that additional cpp41 patches are needed to avoid error -115
> messages, but that can be applied separately.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Signed-off-by: Tony Lindgren
> ---
> drivers/usb/musb/musb_cppi41.c | 1 +
> drivers/usb/musb/musb_dma.h| 3 +++
> drivers/usb/musb/musb_host.c | 16 +++-
> 3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> --- a/drivers/usb/musb/musb_cppi41.c
> +++ b/drivers/usb/musb/musb_cppi41.c
> @@ -695,6 +695,7 @@ cppi41_dma_controller_create(struct musb *musb, void
> __iomem *base)
> controller->controller.channel_program = cppi41_dma_channel_program;
> controller->controller.channel_abort = cppi41_dma_channel_abort;
> controller->controller.is_compatible = cppi41_is_compatible;
> + controller->controller.quirks = MUSB_DMA_QUIRK_CPPI41_IN;
>
> ret = cppi41_dma_controller_start(controller);
> if (ret)
> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
> --- a/drivers/usb/musb/musb_dma.h
> +++ b/drivers/usb/musb/musb_dma.h
> @@ -171,6 +171,8 @@ dma_channel_status(struct dma_channel *c)
> return (is_dma_capable() && c) ? c->status : MUSB_DMA_STATUS_UNKNOWN;
> }
>
> +#define MUSB_DMA_QUIRK_CPPI41_IN BIT(0)
> +
> /**
> * struct dma_controller - A DMA Controller.
> * @start: call this to start a DMA controller;
> @@ -196,6 +198,7 @@ struct dma_controller {
> int (*is_compatible)(struct dma_channel *channel,
> u16 maxpacket,
> void *buf, u32 length);
> + unsigned int quirks;
> };
>
> /* called after channel_program(), may indicate a fault */
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -743,6 +743,8 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
>
> musb_ep_select(mbase, epnum);
>
> + dma_controller = musb->dma_controller;
> +
> if (is_out && !len) {
> use_dma = 0;
> csr = musb_readw(epio, MUSB_TXCSR);
> @@ -751,8 +753,20 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
> hw_ep->tx_channel = NULL;
> }
>
> + /*
> + * At least cppi41 in dma will just hang with size of 1 until the
> + * device is disconnected.
> + */
> + if (!is_out && dma_controller &&
> + (dma_controller->quirks & MUSB_DMA_QUIRK_CPPI41_IN)) {
Forget to add size constraint here?
Regards,
-Bin.
--
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 v6 23/25] usb: chipidea: Pullup D+ in device mode via phy APIs
On Wed, Jan 18, 2017 at 02:57:27PM -0600, Rob Herring wrote: > On Wed, Jan 18, 2017 at 2:54 PM, Stephen Boyd wrote: > > Quoting Peter Chen (2017-01-17 23:34:32) > >> On Tue, Jan 17, 2017 at 09:58:33AM -0800, Stephen Boyd wrote: > >> > Quoting Peter Chen (2017-01-15 19:45:51) > >> > > > >> > > At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need > >> > > to port it to generic phy framework. > >> > > > >> > > >> > Ok. I'll look into that. Can the other patches in this series be picked > >> > up? Otherwise I can resend them all again once I fix the phy_set_mode() > >> > call location and introduce a new phy op. > >> > >> I can pick up chipidea patches after you test the patch I supplied at: > >> > >> [PATCH v6 11/25] usb: chipidea: vbus event may exist before > >> starting > >> gadget > > > > Ok. I've confirmed that this updated patch works fine for me. You can > > have my Tested-by and Reviewed-by there. > > > >> > >> You may ping other maintainers to pick up other patches. > >> > > > > I was hoping you could pick the beginning of the series up until the PHY > > drivers, which can go via Kishon's tree. That would mean applying the > > drivers/of/ part. Rob is that ok? > > Peter, If there's a dependency then please take the patches I've > acked. That's why I acked them. > Ok, I will do it. -- Best Regards, Peter Chen -- 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 v6 03/25] usb: ulpi: Support device discovery via DT
On Wed, Dec 28, 2016 at 02:56:49PM -0800, Stephen Boyd wrote:
> The qcom HSIC ULPI phy doesn't have any bits set in the vendor or
> product ID registers. This makes it impossible to make a ULPI
> driver match against the ID registers. Add support to discover
> the ULPI phys via DT help alleviate this problem. In the DT case,
> we'll look for a ULPI bus node underneath the device registering
> the ULPI viewport (or the parent of that device to support
> chipidea's device layout) and then match up the phy node
> underneath that with the ULPI device that's created.
>
> The side benefit of this is that we can use standard properties
> in the phy node like clks, regulators, gpios, etc. because we
> don't have firmware like ACPI to turn these things on for us. And
> we can use the DT phy binding to point our phy consumer to the
> phy provider.
>
> The ULPI bus code supports native enumeration by reading the
> vendor ID and product ID registers at device creation time, but
> we can't be certain that those register reads will succeed if the
> phy is not powered up. To avoid any problems with reading the ID
> registers before the phy is powered we fallback to DT matching
> when the ID reads fail.
>
> If the ULPI spec had some generic power sequencing for these
> registers we could put that into the ULPI bus layer and power up
> the device before reading the ID registers. Unfortunately this
> doesn't exist and the power sequence is usually device specific.
> By having the device matched up with DT we can avoid this
> problem.
>
> Cc: Greg Kroah-Hartman
> Acked-by: Heikki Krogerus
> Cc:
> Acked-by: Rob Herring
> Signed-off-by: Stephen Boyd
Greg, is it ok I pick up this patch, and send it with chipidea
changes together for 4.11-rc1 later?
Peter
> ---
> Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++
> drivers/usb/common/ulpi.c | 79
> --
> 2 files changed, 93 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/ulpi.txt
> b/Documentation/devicetree/bindings/usb/ulpi.txt
> new file mode 100644
> index ..ca179dc4bd50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ulpi.txt
> @@ -0,0 +1,20 @@
> +ULPI bus binding
> +
> +
> +Phys that are behind a ULPI connection can be described with the following
> +binding. The host controller shall have a "ulpi" named node as a child, and
> +that node shall have one enabled node underneath it representing the ulpi
> +device on the bus.
> +
> +EXAMPLE
> +---
> +
> +usb {
> + compatible = "vendor,usb-controller";
> +
> + ulpi {
> + phy {
> + compatible = "vendor,phy";
> + };
> + };
> +};
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 8b317702d761..c9480d77810c 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -16,6 +16,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> /*
> -- */
>
> @@ -39,6 +42,10 @@ static int ulpi_match(struct device *dev, struct
> device_driver *driver)
> struct ulpi *ulpi = to_ulpi_dev(dev);
> const struct ulpi_device_id *id;
>
> + /* Some ULPI devices don't have a vendor id so rely on OF match */
> + if (ulpi->id.vendor == 0)
> + return of_driver_match_device(dev, driver);
> +
> for (id = drv->id_table; id->vendor; id++)
> if (id->vendor == ulpi->id.vendor &&
> id->product == ulpi->id.product)
> @@ -50,6 +57,11 @@ static int ulpi_match(struct device *dev, struct
> device_driver *driver)
> static int ulpi_uevent(struct device *dev, struct kobj_uevent_env *env)
> {
> struct ulpi *ulpi = to_ulpi_dev(dev);
> + int ret;
> +
> + ret = of_device_uevent_modalias(dev, env);
> + if (ret != -ENODEV)
> + return ret;
>
> if (add_uevent_var(env, "MODALIAS=ulpi:v%04xp%04x",
> ulpi->id.vendor, ulpi->id.product))
> @@ -60,6 +72,11 @@ static int ulpi_uevent(struct device *dev, struct
> kobj_uevent_env *env)
> static int ulpi_probe(struct device *dev)
> {
> struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
> + int ret;
> +
> + ret = of_clk_set_defaults(dev->of_node, false);
> + if (ret < 0)
> + return ret;
>
> return drv->probe(to_ulpi_dev(dev));
> }
> @@ -87,8 +104,13 @@ static struct bus_type ulpi_bus = {
> static ssize_t modalias_show(struct device *dev, struct device_attribute
> *attr,
>char *buf)
> {
> + int len;
> struct ulpi *ulpi = to_ulpi_dev(dev);
>
> + len = of_device_get_modalias(dev, buf, PAGE_SIZE - 1);
> + if (len != -ENODEV)
> + return len;
> +
> return sprintf(buf, "ulpi:v%
Re: [PATCH v6 03/25] usb: ulpi: Support device discovery via DT
On Thu, Jan 19, 2017 at 02:33:49PM +0800, Peter Chen wrote: > On Wed, Dec 28, 2016 at 02:56:49PM -0800, Stephen Boyd wrote: > > The qcom HSIC ULPI phy doesn't have any bits set in the vendor or > > product ID registers. This makes it impossible to make a ULPI > > driver match against the ID registers. Add support to discover > > the ULPI phys via DT help alleviate this problem. In the DT case, > > we'll look for a ULPI bus node underneath the device registering > > the ULPI viewport (or the parent of that device to support > > chipidea's device layout) and then match up the phy node > > underneath that with the ULPI device that's created. > > > > The side benefit of this is that we can use standard properties > > in the phy node like clks, regulators, gpios, etc. because we > > don't have firmware like ACPI to turn these things on for us. And > > we can use the DT phy binding to point our phy consumer to the > > phy provider. > > > > The ULPI bus code supports native enumeration by reading the > > vendor ID and product ID registers at device creation time, but > > we can't be certain that those register reads will succeed if the > > phy is not powered up. To avoid any problems with reading the ID > > registers before the phy is powered we fallback to DT matching > > when the ID reads fail. > > > > If the ULPI spec had some generic power sequencing for these > > registers we could put that into the ULPI bus layer and power up > > the device before reading the ID registers. Unfortunately this > > doesn't exist and the power sequence is usually device specific. > > By having the device matched up with DT we can avoid this > > problem. > > > > Cc: Greg Kroah-Hartman > > Acked-by: Heikki Krogerus > > Cc: > > Acked-by: Rob Herring > > Signed-off-by: Stephen Boyd > > Greg, is it ok I pick up this patch, and send it with chipidea > changes together for 4.11-rc1 later? No objection from me. thanks, greg k-h -- 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
