* [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch @ 2024-01-19 21:31 Frank Li 2024-01-19 21:31 ` [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot Frank Li 2024-01-24 17:36 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch Conor Dooley 0 siblings, 2 replies; 19+ messages in thread From: Frank Li @ 2024-01-19 21:31 UTC (permalink / raw) To: ran.wang_1, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list Cc: devicetree, linux-kernel, linux-usb, mark.rutland, pku.leo, sergei.shtylyov From: Ran Wang <ran.wang_1@nxp.com> When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS (or its control signal) will turn on immediately on related Root Hub ports. Then the VBUS will be de-asserted for a little while during xhci reset (conducted by xhci driver) for a little while and back to normal. This VBUS glitch might cause some USB devices emuration fail if kernel boot with them connected. One SW workaround which can fix this is to program all PORTSC[PP] to 0 to turn off VBUS immediately after setting host mode in DWC3 driver(per signal measurement result, it will be too late to do it in xhci-plat.c or xhci.c). Signed-off-by: Ran Wang <ran.wang_1@nxp.com> Reviewed-by: Peter Chen <peter.chen@nxp.com> Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml index 203a1eb66691f..dbf272b76e0b5 100644 --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml @@ -273,6 +273,13 @@ properties: with an external supply. type: boolean + snps,host-vbus-glitches: + description: + When set, power off all Root Hub ports immediately after + setting host mode to avoid vbus (negative) glitch happen in later + xhci reset. And the vbus will back to 5V automatically when reset done. + type: boolean + snps,is-utmi-l1-suspend: description: True when DWC3 asserts output signal utmi_l1_suspend_n, false when -- 2.34.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot 2024-01-19 21:31 [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch Frank Li @ 2024-01-19 21:31 ` Frank Li 2024-01-20 0:51 ` Thinh Nguyen 2024-01-24 17:36 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch Conor Dooley 1 sibling, 1 reply; 19+ messages in thread From: Frank Li @ 2024-01-19 21:31 UTC (permalink / raw) To: ran.wang_1, Thinh Nguyen, Greg Kroah-Hartman, open list:DESIGNWARE USB3 DRD IP DRIVER, open list Cc: balbi, devicetree, linux-kernel, linux-usb, mark.rutland, pku.leo, robh+dt, sergei.shtylyov From: Ran Wang <ran.wang_1@nxp.com> When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS (or its control signal) will be turned on immediately on related Root Hub ports. Then, the VBUS is turned off for a little while(15us) when do xhci reset (conducted by xhci driver) and back to normal finally, we can observe a negative glitch of related signal happen. This VBUS glitch might cause some USB devices enumeration fail if kernel boot with them connected. Such as LS1012AFWRY/LS1043ARDB/LX2160AQDS /LS1088ARDB with Kingston 16GB USB2.0/Kingston USB3.0/JetFlash Transcend 4GB USB2.0 drives. The fail cases include enumerated as full-speed device or report wrong device descriptor, etc. One SW workaround which can fix this is by programing all xhci PORTSC[PP] to 0 to turn off VBUS immediately after setting host mode in DWC3 driver (per signal measurement result, it will be too late to do it in xhci-plat.c or xhci.c). Then, after xhci reset complete in xhci driver, PORTSC[PP]s' value will back to 1 automatically and VBUS on at that time, no glitch happen and normal enumeration process has no impact. Signed-off-by: Ran Wang <ran.wang_1@nxp.com> Reviewed-by: Peter Chen <peter.chen@nxp.com> Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Notes: Last review at June 06, 2019. Fixed all review comments and sent again. https://lore.kernel.org/linux-kernel/AM5PR0402MB2865979E26017BC5937DBBA5F1170@AM5PR0402MB2865.eurprd04.prod.outlook.com/ drivers/usb/dwc3/core.c | 2 ++ drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/host.c | 46 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 3e55838c00014..a57adf0c11dd1 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1626,6 +1626,8 @@ static void dwc3_get_properties(struct dwc3 *dwc) dwc->dis_split_quirk = device_property_read_bool(dev, "snps,dis-split-quirk"); + dwc->host_vbus_glitches = device_property_read_bool(dev, "snps,host-vbus-glitches"); + dwc->lpm_nyet_threshold = lpm_nyet_threshold; dwc->tx_de_emphasis = tx_de_emphasis; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index e3eea965e57bf..0269bacbbf6bd 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1135,6 +1135,7 @@ struct dwc3_scratchpad_array { * @dis_split_quirk: set to disable split boundary. * @wakeup_configured: set if the device is configured for remote wakeup. * @suspended: set to track suspend event due to U3/L2. + * @host_vbus_glitches: set to avoid vbus glitch during xhci reset. * @imod_interval: set the interrupt moderation interval in 250ns * increments or 0 to disable. * @max_cfg_eps: current max number of IN eps used across all USB configs. @@ -1353,6 +1354,7 @@ struct dwc3 { unsigned tx_de_emphasis:2; unsigned dis_metastability_quirk:1; + unsigned host_vbus_glitches:1; unsigned dis_split_quirk:1; unsigned async_callbacks:1; diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index 61f57fe5bb783..af8903ee37c20 100644 --- a/drivers/usb/dwc3/host.c +++ b/drivers/usb/dwc3/host.c @@ -11,6 +11,7 @@ #include <linux/of.h> #include <linux/platform_device.h> +#include "../host/xhci.h" #include "core.h" static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc, @@ -28,6 +29,44 @@ static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc, dwc->xhci_resources[1].name = name; } +#define XHCI_HCSPARAMS1 0x4 +#define XHCI_PORTSC_BASE 0x400 + +/* + * dwc3_power_off_all_roothub_ports - Power off all Root hub ports + * @dwc3: Pointer to our controller context structure + */ +static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc) +{ + int i, port_num; + u32 reg, op_regs_base, offset; + void __iomem *xhci_regs; + + /* xhci regs is not mapped yet, do it temperary here */ + if (dwc->xhci_resources[0].start) { + xhci_regs = ioremap(dwc->xhci_resources[0].start, + DWC3_XHCI_REGS_END); + if (IS_ERR(xhci_regs)) { + dev_err(dwc->dev, "Failed to ioremap xhci_regs\n"); + return; + } + + op_regs_base = HC_LENGTH(readl(xhci_regs)); + reg = readl(xhci_regs + XHCI_HCSPARAMS1); + port_num = HCS_MAX_PORTS(reg); + + for (i = 1; i <= port_num; i++) { + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); + reg = readl(xhci_regs + offset); + reg &= ~PORT_POWER; + writel(reg, xhci_regs + offset); + } + + iounmap(xhci_regs); + } else + dev_err(dwc->dev, "xhci base reg invalid\n"); +} + static int dwc3_host_get_irq(struct dwc3 *dwc) { struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); @@ -66,6 +105,13 @@ int dwc3_host_init(struct dwc3 *dwc) int ret, irq; int prop_idx = 0; + /* + * We have to power off all Root hub ports immediately after DWC3 set + * to host mode to avoid VBUS glitch happen when xhci get reset later. + */ + if (dwc->host_vbus_glitches) + dwc3_power_off_all_roothub_ports(dwc); + irq = dwc3_host_get_irq(dwc); if (irq < 0) return irq; -- 2.34.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot 2024-01-19 21:31 ` [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot Frank Li @ 2024-01-20 0:51 ` Thinh Nguyen 2024-01-20 0:55 ` Thinh Nguyen 2024-01-20 2:00 ` Frank Li 0 siblings, 2 replies; 19+ messages in thread From: Thinh Nguyen @ 2024-01-20 0:51 UTC (permalink / raw) To: Frank Li Cc: ran.wang_1, Thinh Nguyen, Greg Kroah-Hartman, open list:DESIGNWARE USB3 DRD IP DRIVER, open list, balbi, devicetree, mark.rutland, pku.leo, robh+dt, sergei.shtylyov On Fri, Jan 19, 2024, Frank Li wrote: > From: Ran Wang <ran.wang_1@nxp.com> > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > (or its control signal) will be turned on immediately on related Root Hub > ports. Then, the VBUS is turned off for a little while(15us) when do xhci > reset (conducted by xhci driver) and back to normal finally, we can > observe a negative glitch of related signal happen. > > This VBUS glitch might cause some USB devices enumeration fail if kernel > boot with them connected. Such as LS1012AFWRY/LS1043ARDB/LX2160AQDS > /LS1088ARDB with Kingston 16GB USB2.0/Kingston USB3.0/JetFlash Transcend > 4GB USB2.0 drives. The fail cases include enumerated as full-speed device > or report wrong device descriptor, etc. > > One SW workaround which can fix this is by programing all xhci PORTSC[PP] > to 0 to turn off VBUS immediately after setting host mode in DWC3 driver > (per signal measurement result, it will be too late to do it in > xhci-plat.c or xhci.c). Then, after xhci reset complete in xhci driver, > PORTSC[PP]s' value will back to 1 automatically and VBUS on at that time, > no glitch happen and normal enumeration process has no impact. > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > Reviewed-by: Peter Chen <peter.chen@nxp.com> > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > > Notes: > Last review at June 06, 2019. Fixed all review comments and sent again. > > https://urldefense.com/v3/__https://lore.kernel.org/linux-kernel/AM5PR0402MB2865979E26017BC5937DBBA5F1170@AM5PR0402MB2865.eurprd04.prod.outlook.com/__;!!A4F2R9G_pg!bdutJWi1Tcz8SYscB7Mr96SWYMKIo8ElUKgEILFJfK3_60EbECQHXPBmJYAMMhNwQ4YrjxqMZWHdokXhHB6a$ > > drivers/usb/dwc3/core.c | 2 ++ > drivers/usb/dwc3/core.h | 2 ++ > drivers/usb/dwc3/host.c | 46 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 50 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 3e55838c00014..a57adf0c11dd1 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -1626,6 +1626,8 @@ static void dwc3_get_properties(struct dwc3 *dwc) > dwc->dis_split_quirk = device_property_read_bool(dev, > "snps,dis-split-quirk"); > > + dwc->host_vbus_glitches = device_property_read_bool(dev, "snps,host-vbus-glitches"); This is a quirk. The property should be named with "quirk" subfix. But do we need a new quirk? How many different platforms are affected? If it's just 1 or 2, then just use compatible string. > + > dwc->lpm_nyet_threshold = lpm_nyet_threshold; > dwc->tx_de_emphasis = tx_de_emphasis; > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index e3eea965e57bf..0269bacbbf6bd 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1135,6 +1135,7 @@ struct dwc3_scratchpad_array { > * @dis_split_quirk: set to disable split boundary. > * @wakeup_configured: set if the device is configured for remote wakeup. > * @suspended: set to track suspend event due to U3/L2. > + * @host_vbus_glitches: set to avoid vbus glitch during xhci reset. This is only applicable to the first xhci reset in its initialization and not every xhci reset right? If so, please reword. Also, please place it correspond to the order of the field. > * @imod_interval: set the interrupt moderation interval in 250ns > * increments or 0 to disable. > * @max_cfg_eps: current max number of IN eps used across all USB configs. > @@ -1353,6 +1354,7 @@ struct dwc3 { > unsigned tx_de_emphasis:2; > > unsigned dis_metastability_quirk:1; > + unsigned host_vbus_glitches:1; > > unsigned dis_split_quirk:1; > unsigned async_callbacks:1; > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c > index 61f57fe5bb783..af8903ee37c20 100644 > --- a/drivers/usb/dwc3/host.c > +++ b/drivers/usb/dwc3/host.c > @@ -11,6 +11,7 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > > +#include "../host/xhci.h" Let's not import the entire xhci.h. If we're going to share some macros from xhci.h, please split them from xhci.h and perhaps create xhci-ports.h for dwc3 to share. > #include "core.h" > > static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc, > @@ -28,6 +29,44 @@ static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc, > dwc->xhci_resources[1].name = name; > } > > +#define XHCI_HCSPARAMS1 0x4 > +#define XHCI_PORTSC_BASE 0x400 > + > +/* > + * dwc3_power_off_all_roothub_ports - Power off all Root hub ports > + * @dwc3: Pointer to our controller context structure > + */ > +static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc) > +{ > + int i, port_num; > + u32 reg, op_regs_base, offset; > + void __iomem *xhci_regs; > + > + /* xhci regs is not mapped yet, do it temperary here */ > + if (dwc->xhci_resources[0].start) { > + xhci_regs = ioremap(dwc->xhci_resources[0].start, > + DWC3_XHCI_REGS_END); > + if (IS_ERR(xhci_regs)) { > + dev_err(dwc->dev, "Failed to ioremap xhci_regs\n"); > + return; > + } > + > + op_regs_base = HC_LENGTH(readl(xhci_regs)); > + reg = readl(xhci_regs + XHCI_HCSPARAMS1); > + port_num = HCS_MAX_PORTS(reg); > + > + for (i = 1; i <= port_num; i++) { > + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); > + reg = readl(xhci_regs + offset); > + reg &= ~PORT_POWER; > + writel(reg, xhci_regs + offset); > + } > + > + iounmap(xhci_regs); > + } else > + dev_err(dwc->dev, "xhci base reg invalid\n"); > +} > + > static int dwc3_host_get_irq(struct dwc3 *dwc) > { > struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); > @@ -66,6 +105,13 @@ int dwc3_host_init(struct dwc3 *dwc) > int ret, irq; > int prop_idx = 0; > > + /* > + * We have to power off all Root hub ports immediately after DWC3 set > + * to host mode to avoid VBUS glitch happen when xhci get reset later. > + */ > + if (dwc->host_vbus_glitches) > + dwc3_power_off_all_roothub_ports(dwc); > + It's part of the dwc3_host_init(), but don't do this in dwc3_host_get_irq(). Place it where it makes sense. > irq = dwc3_host_get_irq(dwc); > if (irq < 0) > return irq; > -- > 2.34.1 > Please run checkpatch.pl and fix them next time. Regarding PARENTHESIS_ALIGNMENT or line continuation, it can be two indentations or parenthesis aligned, whichever one makes it easier to read. From checkpatch: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #119: FILE: drivers/usb/dwc3/host.c:48: + xhci_regs = ioremap(dwc->xhci_resources[0].start, + DWC3_XHCI_REGS_END); CHECK:SPACING: spaces preferred around that '*' (ctx:VxV) #130: FILE: drivers/usb/dwc3/host.c:59: + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); ^ CHECK:SPACING: spaces preferred around that '-' (ctx:VxV) #130: FILE: drivers/usb/dwc3/host.c:59: + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); ^ CHECK:BRACES: Unbalanced braces around else statement #137: FILE: drivers/usb/dwc3/host.c:66: + } else total: 0 errors, 0 warnings, 4 checks, 86 lines checked Thanks, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot 2024-01-20 0:51 ` Thinh Nguyen @ 2024-01-20 0:55 ` Thinh Nguyen 2024-01-20 2:00 ` Frank Li 1 sibling, 0 replies; 19+ messages in thread From: Thinh Nguyen @ 2024-01-20 0:55 UTC (permalink / raw) To: Frank Li Cc: ran.wang_1, Greg Kroah-Hartman, open list:DESIGNWARE USB3 DRD IP DRIVER, open list, balbi, devicetree, mark.rutland, pku.leo, robh+dt, sergei.shtylyov On Sat, Jan 20, 2024, Thinh Nguyen wrote: > > + > > static int dwc3_host_get_irq(struct dwc3 *dwc) > > { > > struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); > > @@ -66,6 +105,13 @@ int dwc3_host_init(struct dwc3 *dwc) > > int ret, irq; > > int prop_idx = 0; > > > > + /* > > + * We have to power off all Root hub ports immediately after DWC3 set > > + * to host mode to avoid VBUS glitch happen when xhci get reset later. > > + */ > > + if (dwc->host_vbus_glitches) > > + dwc3_power_off_all_roothub_ports(dwc); > > + > > It's part of the dwc3_host_init(), but don't do this in > dwc3_host_get_irq(). Place it where it makes sense. Ignore this comment, I thought it's called from dwc3_host_get_irq(), but it's not. BR, Thinh > > > irq = dwc3_host_get_irq(dwc); > > if (irq < 0) > > return irq; > > -- > > 2.34.1 > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot 2024-01-20 0:51 ` Thinh Nguyen 2024-01-20 0:55 ` Thinh Nguyen @ 2024-01-20 2:00 ` Frank Li 1 sibling, 0 replies; 19+ messages in thread From: Frank Li @ 2024-01-20 2:00 UTC (permalink / raw) To: Thinh Nguyen Cc: ran.wang_1, Greg Kroah-Hartman, open list:DESIGNWARE USB3 DRD IP DRIVER, open list, balbi, devicetree, mark.rutland, pku.leo, robh+dt, sergei.shtylyov On Sat, Jan 20, 2024 at 12:51:07AM +0000, Thinh Nguyen wrote: > On Fri, Jan 19, 2024, Frank Li wrote: > > From: Ran Wang <ran.wang_1@nxp.com> > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > (or its control signal) will be turned on immediately on related Root Hub > > ports. Then, the VBUS is turned off for a little while(15us) when do xhci > > reset (conducted by xhci driver) and back to normal finally, we can > > observe a negative glitch of related signal happen. > > > > This VBUS glitch might cause some USB devices enumeration fail if kernel > > boot with them connected. Such as LS1012AFWRY/LS1043ARDB/LX2160AQDS > > /LS1088ARDB with Kingston 16GB USB2.0/Kingston USB3.0/JetFlash Transcend > > 4GB USB2.0 drives. The fail cases include enumerated as full-speed device > > or report wrong device descriptor, etc. > > > > One SW workaround which can fix this is by programing all xhci PORTSC[PP] > > to 0 to turn off VBUS immediately after setting host mode in DWC3 driver > > (per signal measurement result, it will be too late to do it in > > xhci-plat.c or xhci.c). Then, after xhci reset complete in xhci driver, > > PORTSC[PP]s' value will back to 1 automatically and VBUS on at that time, > > no glitch happen and normal enumeration process has no impact. > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > > > Notes: > > Last review at June 06, 2019. Fixed all review comments and sent again. > > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-kernel/AM5PR0402MB2865979E26017BC5937DBBA5F1170@AM5PR0402MB2865.eurprd04.prod.outlook.com/__;!!A4F2R9G_pg!bdutJWi1Tcz8SYscB7Mr96SWYMKIo8ElUKgEILFJfK3_60EbECQHXPBmJYAMMhNwQ4YrjxqMZWHdokXhHB6a$ > > > > drivers/usb/dwc3/core.c | 2 ++ > > drivers/usb/dwc3/core.h | 2 ++ > > drivers/usb/dwc3/host.c | 46 +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 50 insertions(+) > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index 3e55838c00014..a57adf0c11dd1 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -1626,6 +1626,8 @@ static void dwc3_get_properties(struct dwc3 *dwc) > > dwc->dis_split_quirk = device_property_read_bool(dev, > > "snps,dis-split-quirk"); > > > > + dwc->host_vbus_glitches = device_property_read_bool(dev, "snps,host-vbus-glitches"); > > This is a quirk. The property should be named with "quirk" subfix. > But do we need a new quirk? How many different platforms are affected? > If it's just 1 or 2, then just use compatible string. more than 2 platform. I think quirk is more flexible. Frank > > > + > > dwc->lpm_nyet_threshold = lpm_nyet_threshold; > > dwc->tx_de_emphasis = tx_de_emphasis; > > > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > > index e3eea965e57bf..0269bacbbf6bd 100644 > > --- a/drivers/usb/dwc3/core.h > > +++ b/drivers/usb/dwc3/core.h > > @@ -1135,6 +1135,7 @@ struct dwc3_scratchpad_array { > > * @dis_split_quirk: set to disable split boundary. > > * @wakeup_configured: set if the device is configured for remote wakeup. > > * @suspended: set to track suspend event due to U3/L2. > > + * @host_vbus_glitches: set to avoid vbus glitch during xhci reset. > > This is only applicable to the first xhci reset in its initialization > and not every xhci reset right? If so, please reword. > > Also, please place it correspond to the order of the field. > > > * @imod_interval: set the interrupt moderation interval in 250ns > > * increments or 0 to disable. > > * @max_cfg_eps: current max number of IN eps used across all USB configs. > > @@ -1353,6 +1354,7 @@ struct dwc3 { > > unsigned tx_de_emphasis:2; > > > > unsigned dis_metastability_quirk:1; > > + unsigned host_vbus_glitches:1; > > > > unsigned dis_split_quirk:1; > > unsigned async_callbacks:1; > > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c > > index 61f57fe5bb783..af8903ee37c20 100644 > > --- a/drivers/usb/dwc3/host.c > > +++ b/drivers/usb/dwc3/host.c > > @@ -11,6 +11,7 @@ > > #include <linux/of.h> > > #include <linux/platform_device.h> > > > > +#include "../host/xhci.h" > > Let's not import the entire xhci.h. If we're going to share some macros > from xhci.h, please split them from xhci.h and perhaps create > xhci-ports.h for dwc3 to share. > > > #include "core.h" > > > > static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc, > > @@ -28,6 +29,44 @@ static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc, > > dwc->xhci_resources[1].name = name; > > } > > > > +#define XHCI_HCSPARAMS1 0x4 > > +#define XHCI_PORTSC_BASE 0x400 > > + > > +/* > > + * dwc3_power_off_all_roothub_ports - Power off all Root hub ports > > + * @dwc3: Pointer to our controller context structure > > + */ > > +static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc) > > +{ > > + int i, port_num; > > + u32 reg, op_regs_base, offset; > > + void __iomem *xhci_regs; > > + > > + /* xhci regs is not mapped yet, do it temperary here */ > > + if (dwc->xhci_resources[0].start) { > > + xhci_regs = ioremap(dwc->xhci_resources[0].start, > > + DWC3_XHCI_REGS_END); > > + if (IS_ERR(xhci_regs)) { > > + dev_err(dwc->dev, "Failed to ioremap xhci_regs\n"); > > + return; > > + } > > + > > + op_regs_base = HC_LENGTH(readl(xhci_regs)); > > + reg = readl(xhci_regs + XHCI_HCSPARAMS1); > > + port_num = HCS_MAX_PORTS(reg); > > + > > + for (i = 1; i <= port_num; i++) { > > + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); > > + reg = readl(xhci_regs + offset); > > + reg &= ~PORT_POWER; > > + writel(reg, xhci_regs + offset); > > + } > > + > > + iounmap(xhci_regs); > > + } else > > + dev_err(dwc->dev, "xhci base reg invalid\n"); > > +} > > + > > static int dwc3_host_get_irq(struct dwc3 *dwc) > > { > > struct platform_device *dwc3_pdev = to_platform_device(dwc->dev); > > @@ -66,6 +105,13 @@ int dwc3_host_init(struct dwc3 *dwc) > > int ret, irq; > > int prop_idx = 0; > > > > + /* > > + * We have to power off all Root hub ports immediately after DWC3 set > > + * to host mode to avoid VBUS glitch happen when xhci get reset later. > > + */ > > + if (dwc->host_vbus_glitches) > > + dwc3_power_off_all_roothub_ports(dwc); > > + > > It's part of the dwc3_host_init(), but don't do this in > dwc3_host_get_irq(). Place it where it makes sense. > > > irq = dwc3_host_get_irq(dwc); > > if (irq < 0) > > return irq; > > -- > > 2.34.1 > > > > Please run checkpatch.pl and fix them next time. Regarding > PARENTHESIS_ALIGNMENT or line continuation, it can be two indentations > or parenthesis aligned, whichever one makes it easier to read. > > > From checkpatch: > > CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis > #119: FILE: drivers/usb/dwc3/host.c:48: > + xhci_regs = ioremap(dwc->xhci_resources[0].start, > + DWC3_XHCI_REGS_END); > > CHECK:SPACING: spaces preferred around that '*' (ctx:VxV) > #130: FILE: drivers/usb/dwc3/host.c:59: > + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); > ^ > > CHECK:SPACING: spaces preferred around that '-' (ctx:VxV) > #130: FILE: drivers/usb/dwc3/host.c:59: > + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); > ^ > > CHECK:BRACES: Unbalanced braces around else statement > #137: FILE: drivers/usb/dwc3/host.c:66: > + } else > Sorry, I too trust original patch author. Will fix next version Frank > total: 0 errors, 0 warnings, 4 checks, 86 lines checked > > > Thanks, > Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-19 21:31 [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch Frank Li 2024-01-19 21:31 ` [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot Frank Li @ 2024-01-24 17:36 ` Conor Dooley 2024-01-24 17:47 ` Frank Li 1 sibling, 1 reply; 19+ messages in thread From: Conor Dooley @ 2024-01-24 17:36 UTC (permalink / raw) To: Frank Li Cc: ran.wang_1, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov [-- Attachment #1: Type: text/plain, Size: 2146 bytes --] On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > From: Ran Wang <ran.wang_1@nxp.com> > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > (or its control signal) will turn on immediately on related Root Hub > ports. Then the VBUS will be de-asserted for a little while during xhci > reset (conducted by xhci driver) for a little while and back to normal. > > This VBUS glitch might cause some USB devices emuration fail if kernel > boot with them connected. One SW workaround which can fix this is to > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > host mode in DWC3 driver(per signal measurement result, it will be too > late to do it in xhci-plat.c or xhci.c). > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > Reviewed-by: Peter Chen <peter.chen@nxp.com> > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > index 203a1eb66691f..dbf272b76e0b5 100644 > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > @@ -273,6 +273,13 @@ properties: > with an external supply. > type: boolean > > + snps,host-vbus-glitches: > + description: > + When set, power off all Root Hub ports immediately after > + setting host mode to avoid vbus (negative) glitch happen in later > + xhci reset. And the vbus will back to 5V automatically when reset done. > + type: boolean Why do we want to have a property for this at all? The commit message seems to describe a problem that's limited to specific configurations and appears to be somethng the driver should do unconditionally. Could you explain why this cannot be done unconditionally please? Thanks, Conor. > + > snps,is-utmi-l1-suspend: > description: > True when DWC3 asserts output signal utmi_l1_suspend_n, false when > -- > 2.34.1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-24 17:36 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch Conor Dooley @ 2024-01-24 17:47 ` Frank Li 2024-01-24 17:59 ` Conor Dooley 0 siblings, 1 reply; 19+ messages in thread From: Frank Li @ 2024-01-24 17:47 UTC (permalink / raw) To: Conor Dooley Cc: ran.wang_1, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov On Wed, Jan 24, 2024 at 05:36:42PM +0000, Conor Dooley wrote: > On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > > From: Ran Wang <ran.wang_1@nxp.com> > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > (or its control signal) will turn on immediately on related Root Hub > > ports. Then the VBUS will be de-asserted for a little while during xhci > > reset (conducted by xhci driver) for a little while and back to normal. > > > > This VBUS glitch might cause some USB devices emuration fail if kernel > > boot with them connected. One SW workaround which can fix this is to > > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > > host mode in DWC3 driver(per signal measurement result, it will be too > > late to do it in xhci-plat.c or xhci.c). > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > index 203a1eb66691f..dbf272b76e0b5 100644 > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > @@ -273,6 +273,13 @@ properties: > > with an external supply. > > type: boolean > > > > + snps,host-vbus-glitches: > > + description: > > + When set, power off all Root Hub ports immediately after > > + setting host mode to avoid vbus (negative) glitch happen in later > > + xhci reset. And the vbus will back to 5V automatically when reset done. > > + type: boolean > > Why do we want to have a property for this at all? The commit message > seems to describe a problem that's limited to specific configurations > and appears to be somethng the driver should do unconditionally. > > Could you explain why this cannot be done unconditionally please? It depends on board design, not all system vbus can be controller by root hub port. If it is always on, it will not trigger this issue. Frank > > Thanks, > Conor. > > > + > > snps,is-utmi-l1-suspend: > > description: > > True when DWC3 asserts output signal utmi_l1_suspend_n, false when > > -- > > 2.34.1 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-24 17:47 ` Frank Li @ 2024-01-24 17:59 ` Conor Dooley 2024-01-24 19:17 ` Frank Li 2024-01-30 18:13 ` Rob Herring 0 siblings, 2 replies; 19+ messages in thread From: Conor Dooley @ 2024-01-24 17:59 UTC (permalink / raw) To: Frank Li Cc: ran.wang_1, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov [-- Attachment #1: Type: text/plain, Size: 2910 bytes --] On Wed, Jan 24, 2024 at 12:47:14PM -0500, Frank Li wrote: > On Wed, Jan 24, 2024 at 05:36:42PM +0000, Conor Dooley wrote: > > On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > > > From: Ran Wang <ran.wang_1@nxp.com> > > > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > > (or its control signal) will turn on immediately on related Root Hub > > > ports. Then the VBUS will be de-asserted for a little while during xhci > > > reset (conducted by xhci driver) for a little while and back to normal. > > > > > > This VBUS glitch might cause some USB devices emuration fail if kernel > > > boot with them connected. One SW workaround which can fix this is to > > > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > > > host mode in DWC3 driver(per signal measurement result, it will be too > > > late to do it in xhci-plat.c or xhci.c). > > > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > index 203a1eb66691f..dbf272b76e0b5 100644 > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > @@ -273,6 +273,13 @@ properties: > > > with an external supply. > > > type: boolean > > > > > > + snps,host-vbus-glitches: > > > + description: > > > + When set, power off all Root Hub ports immediately after > > > + setting host mode to avoid vbus (negative) glitch happen in later > > > + xhci reset. And the vbus will back to 5V automatically when reset done. nit: "will return to" > > > + type: boolean > > > > Why do we want to have a property for this at all? The commit message > > seems to describe a problem that's limited to specific configurations > > and appears to be somethng the driver should do unconditionally. > > > > Could you explain why this cannot be done unconditionally please? > > It depends on board design, not all system vbus can be controller by root > hub port. If it is always on, it will not trigger this issue. Okay, that seems reasonable to have a property for. Can you add that info to the commit message please? On another note, I like it when the property name explains why you would add it, rather than the thing it is trying to solve. Named after the disease, rather than the symptoms, if you get me. I tried to come up with a name here, but could not really suggest something good. If you can think of something, that'd be good, but don't stress it. Thanks, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-24 17:59 ` Conor Dooley @ 2024-01-24 19:17 ` Frank Li 2024-01-25 17:43 ` Conor Dooley 2024-01-30 18:13 ` Rob Herring 1 sibling, 1 reply; 19+ messages in thread From: Frank Li @ 2024-01-24 19:17 UTC (permalink / raw) To: Conor Dooley Cc: ran.wang_1, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov On Wed, Jan 24, 2024 at 05:59:00PM +0000, Conor Dooley wrote: > On Wed, Jan 24, 2024 at 12:47:14PM -0500, Frank Li wrote: > > On Wed, Jan 24, 2024 at 05:36:42PM +0000, Conor Dooley wrote: > > > On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > > > > From: Ran Wang <ran.wang_1@nxp.com> > > > > > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > > > (or its control signal) will turn on immediately on related Root Hub > > > > ports. Then the VBUS will be de-asserted for a little while during xhci > > > > reset (conducted by xhci driver) for a little while and back to normal. > > > > > > > > This VBUS glitch might cause some USB devices emuration fail if kernel > > > > boot with them connected. One SW workaround which can fix this is to > > > > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > > > > host mode in DWC3 driver(per signal measurement result, it will be too > > > > late to do it in xhci-plat.c or xhci.c). > > > > > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > --- > > > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > index 203a1eb66691f..dbf272b76e0b5 100644 > > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > @@ -273,6 +273,13 @@ properties: > > > > with an external supply. > > > > type: boolean > > > > > > > > + snps,host-vbus-glitches: > > > > + description: > > > > + When set, power off all Root Hub ports immediately after > > > > + setting host mode to avoid vbus (negative) glitch happen in later > > > > + xhci reset. And the vbus will back to 5V automatically when reset done. > > nit: "will return to" > > > > > + type: boolean > > > > > > Why do we want to have a property for this at all? The commit message > > > seems to describe a problem that's limited to specific configurations > > > and appears to be somethng the driver should do unconditionally. > > > > > > Could you explain why this cannot be done unconditionally please? > > > > It depends on board design, not all system vbus can be controller by root > > hub port. If it is always on, it will not trigger this issue. > > Okay, that seems reasonable to have a property for. Can you add that > info to the commit message please? By the way, I sent v4 at https://lore.kernel.org/imx/20240124152525.3910311-1-Frank.Li@nxp.com/T/#t How about add below sentence? This was only happen when PORTSC[PP} can control vbus. Needn't set it if vbus is always on. > > On another note, I like it when the property name explains why you would > add it, rather than the thing it is trying to solve. > Named after the disease, rather than the symptoms, if you get me. I > tried to come up with a name here, but could not really suggest > something good. If you can think of something, that'd be good, but don't > stress it. snps,host-vbus-glitches change to snps,host-vbus-glitches-quirk. How about use below description: When set, power off all Root Hub ports immediately after setting host mode to avoid vbus (negative) glitch happen in later xhci reset. That may cause some USB devices emuration fail when kernel boot with device connected and PORTSC[PP] control vbus in board desgin. Frank > > Thanks, > Conor. > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-24 19:17 ` Frank Li @ 2024-01-25 17:43 ` Conor Dooley 2024-01-26 4:06 ` Frank Li 0 siblings, 1 reply; 19+ messages in thread From: Conor Dooley @ 2024-01-25 17:43 UTC (permalink / raw) To: Frank Li Cc: ran.wang_1, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov [-- Attachment #1: Type: text/plain, Size: 4395 bytes --] On Wed, Jan 24, 2024 at 02:17:22PM -0500, Frank Li wrote: > On Wed, Jan 24, 2024 at 05:59:00PM +0000, Conor Dooley wrote: > > On Wed, Jan 24, 2024 at 12:47:14PM -0500, Frank Li wrote: > > > On Wed, Jan 24, 2024 at 05:36:42PM +0000, Conor Dooley wrote: > > > > On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > > > > > From: Ran Wang <ran.wang_1@nxp.com> > > > > > > > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > > > > (or its control signal) will turn on immediately on related Root Hub > > > > > ports. Then the VBUS will be de-asserted for a little while during xhci > > > > > reset (conducted by xhci driver) for a little while and back to normal. > > > > > > > > > > This VBUS glitch might cause some USB devices emuration fail if kernel > > > > > boot with them connected. One SW workaround which can fix this is to > > > > > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > > > > > host mode in DWC3 driver(per signal measurement result, it will be too > > > > > late to do it in xhci-plat.c or xhci.c). > > > > > > > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > > > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > > --- > > > > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > index 203a1eb66691f..dbf272b76e0b5 100644 > > > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > @@ -273,6 +273,13 @@ properties: > > > > > with an external supply. > > > > > type: boolean > > > > > > > > > > + snps,host-vbus-glitches: > > > > > + description: > > > > > + When set, power off all Root Hub ports immediately after > > > > > + setting host mode to avoid vbus (negative) glitch happen in later > > > > > + xhci reset. And the vbus will back to 5V automatically when reset done. > > > > nit: "will return to" > > > > > > > + type: boolean > > > > > > > > Why do we want to have a property for this at all? The commit message > > > > seems to describe a problem that's limited to specific configurations > > > > and appears to be somethng the driver should do unconditionally. > > > > > > > > Could you explain why this cannot be done unconditionally please? > > > > > > It depends on board design, not all system vbus can be controller by root > > > hub port. If it is always on, it will not trigger this issue. > > > > Okay, that seems reasonable to have a property for. Can you add that > > info to the commit message please? > > By the way, I sent v4 at > https://lore.kernel.org/imx/20240124152525.3910311-1-Frank.Li@nxp.com/T/#t I see. > How about add below sentence? > > This was only happen when PORTSC[PP} can control vbus. Needn't set it if > vbus is always on. "This can only happen when ... controls vbus, if vbus is always on, omit this property". Just a wee grammatical nitpicking. > > On another note, I like it when the property name explains why you would > > add it, rather than the thing it is trying to solve. > > Named after the disease, rather than the symptoms, if you get me. I > > tried to come up with a name here, but could not really suggest > > something good. If you can think of something, that'd be good, but don't > > stress it. > > snps,host-vbus-glitches change to snps,host-vbus-glitches-quirk. I don't think adding "quirk" moves the needle. > How about use below description: > > When set, power off all Root Hub ports immediately after > setting host mode to avoid vbus (negative) glitch happen in later > xhci reset. That may cause some USB devices emuration fail when kernel boot > with device connected and PORTSC[PP] control vbus in board desgin. "When set, all root hub ports should be powered off immediately after enabling host mode, to avoid Vbus (negative) glitches that may happen during xHCI reset. These glitches can cause enumeration of some USB devices to fail when PORTSC[PP] controls Vbus. If Vbus is always on, omit this property." How's that? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-25 17:43 ` Conor Dooley @ 2024-01-26 4:06 ` Frank Li 2024-01-26 16:34 ` Conor Dooley 0 siblings, 1 reply; 19+ messages in thread From: Frank Li @ 2024-01-26 4:06 UTC (permalink / raw) To: Conor Dooley Cc: ran.wang_1, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov On Thu, Jan 25, 2024 at 05:43:22PM +0000, Conor Dooley wrote: > On Wed, Jan 24, 2024 at 02:17:22PM -0500, Frank Li wrote: > > On Wed, Jan 24, 2024 at 05:59:00PM +0000, Conor Dooley wrote: > > > On Wed, Jan 24, 2024 at 12:47:14PM -0500, Frank Li wrote: > > > > On Wed, Jan 24, 2024 at 05:36:42PM +0000, Conor Dooley wrote: > > > > > On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > > > > > > From: Ran Wang <ran.wang_1@nxp.com> > > > > > > > > > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > > > > > (or its control signal) will turn on immediately on related Root Hub > > > > > > ports. Then the VBUS will be de-asserted for a little while during xhci > > > > > > reset (conducted by xhci driver) for a little while and back to normal. > > > > > > > > > > > > This VBUS glitch might cause some USB devices emuration fail if kernel > > > > > > boot with them connected. One SW workaround which can fix this is to > > > > > > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > > > > > > host mode in DWC3 driver(per signal measurement result, it will be too > > > > > > late to do it in xhci-plat.c or xhci.c). > > > > > > > > > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > > > > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > > > --- > > > > > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > > index 203a1eb66691f..dbf272b76e0b5 100644 > > > > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > > @@ -273,6 +273,13 @@ properties: > > > > > > with an external supply. > > > > > > type: boolean > > > > > > > > > > > > + snps,host-vbus-glitches: > > > > > > + description: > > > > > > + When set, power off all Root Hub ports immediately after > > > > > > + setting host mode to avoid vbus (negative) glitch happen in later > > > > > > + xhci reset. And the vbus will back to 5V automatically when reset done. > > > > > > nit: "will return to" > > > > > > > > > + type: boolean > > > > > > > > > > Why do we want to have a property for this at all? The commit message > > > > > seems to describe a problem that's limited to specific configurations > > > > > and appears to be somethng the driver should do unconditionally. > > > > > > > > > > Could you explain why this cannot be done unconditionally please? > > > > > > > > It depends on board design, not all system vbus can be controller by root > > > > hub port. If it is always on, it will not trigger this issue. > > > > > > Okay, that seems reasonable to have a property for. Can you add that > > > info to the commit message please? > > > > By the way, I sent v4 at > > https://lore.kernel.org/imx/20240124152525.3910311-1-Frank.Li@nxp.com/T/#t > > I see. > > > How about add below sentence? > > > > This was only happen when PORTSC[PP} can control vbus. Needn't set it if > > vbus is always on. > > "This can only happen when ... controls vbus, if vbus is always on, omit > this property". > > Just a wee grammatical nitpicking. > > > > On another note, I like it when the property name explains why you would > > > add it, rather than the thing it is trying to solve. > > > Named after the disease, rather than the symptoms, if you get me. I > > > tried to come up with a name here, but could not really suggest > > > something good. If you can think of something, that'd be good, but don't > > > stress it. > > > > snps,host-vbus-glitches change to snps,host-vbus-glitches-quirk. > > I don't think adding "quirk" moves the needle. I think "quirk" is reasonable because it is workaround. Frank > > > How about use below description: > > > > When set, power off all Root Hub ports immediately after > > setting host mode to avoid vbus (negative) glitch happen in later > > xhci reset. That may cause some USB devices emuration fail when kernel boot > > with device connected and PORTSC[PP] control vbus in board desgin. > > "When set, all root hub ports should be powered off immediately after > enabling host mode, to avoid Vbus (negative) glitches that may happen > during xHCI reset. These glitches can cause enumeration of some USB > devices to fail when PORTSC[PP] controls Vbus. If Vbus is always on, > omit this property." > > How's that? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-26 4:06 ` Frank Li @ 2024-01-26 16:34 ` Conor Dooley 0 siblings, 0 replies; 19+ messages in thread From: Conor Dooley @ 2024-01-26 16:34 UTC (permalink / raw) To: Frank Li Cc: ran.wang_1, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov [-- Attachment #1: Type: text/plain, Size: 756 bytes --] On Thu, Jan 25, 2024 at 11:06:09PM -0500, Frank Li wrote: > > > > On another note, I like it when the property name explains why you would > > > > add it, rather than the thing it is trying to solve. > > > > Named after the disease, rather than the symptoms, if you get me. I > > > > tried to come up with a name here, but could not really suggest > > > > something good. If you can think of something, that'd be good, but don't > > > > stress it. > > > > > > snps,host-vbus-glitches change to snps,host-vbus-glitches-quirk. > > > > I don't think adding "quirk" moves the needle. > > I think "quirk" is reasonable because it is workaround. Workaround is what the software will do. In the binding we just describe what the problem is. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-24 17:59 ` Conor Dooley 2024-01-24 19:17 ` Frank Li @ 2024-01-30 18:13 ` Rob Herring 2024-01-30 19:29 ` Frank Li 1 sibling, 1 reply; 19+ messages in thread From: Rob Herring @ 2024-01-30 18:13 UTC (permalink / raw) To: Conor Dooley, Frank Li Cc: ran.wang_1, Greg Kroah-Hartman, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov On Wed, Jan 24, 2024 at 05:59:00PM +0000, Conor Dooley wrote: > On Wed, Jan 24, 2024 at 12:47:14PM -0500, Frank Li wrote: > > On Wed, Jan 24, 2024 at 05:36:42PM +0000, Conor Dooley wrote: > > > On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > > > > From: Ran Wang <ran.wang_1@nxp.com> > > > > > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > > > (or its control signal) will turn on immediately on related Root Hub > > > > ports. Then the VBUS will be de-asserted for a little while during xhci > > > > reset (conducted by xhci driver) for a little while and back to normal. > > > > > > > > This VBUS glitch might cause some USB devices emuration fail if kernel > > > > boot with them connected. One SW workaround which can fix this is to > > > > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > > > > host mode in DWC3 driver(per signal measurement result, it will be too > > > > late to do it in xhci-plat.c or xhci.c). > > > > > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > --- > > > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > index 203a1eb66691f..dbf272b76e0b5 100644 > > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > @@ -273,6 +273,13 @@ properties: > > > > with an external supply. > > > > type: boolean > > > > > > > > + snps,host-vbus-glitches: > > > > + description: > > > > + When set, power off all Root Hub ports immediately after > > > > + setting host mode to avoid vbus (negative) glitch happen in later > > > > + xhci reset. And the vbus will back to 5V automatically when reset done. > > nit: "will return to" > > > > > + type: boolean > > > > > > Why do we want to have a property for this at all? The commit message > > > seems to describe a problem that's limited to specific configurations > > > and appears to be somethng the driver should do unconditionally. > > > > > > Could you explain why this cannot be done unconditionally please? > > > > It depends on board design, not all system vbus can be controller by root > > hub port. If it is always on, it will not trigger this issue. > > Okay, that seems reasonable to have a property for. Can you add that > info to the commit message please? But if vbus is always on, then applying the work-around would be a NOP, right? So you could just apply this unconditionally. Rob ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-30 18:13 ` Rob Herring @ 2024-01-30 19:29 ` Frank Li 2024-02-01 1:31 ` Thinh Nguyen 0 siblings, 1 reply; 19+ messages in thread From: Frank Li @ 2024-01-30 19:29 UTC (permalink / raw) To: Rob Herring Cc: Conor Dooley, ran.wang_1, Greg Kroah-Hartman, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov On Tue, Jan 30, 2024 at 12:13:22PM -0600, Rob Herring wrote: > On Wed, Jan 24, 2024 at 05:59:00PM +0000, Conor Dooley wrote: > > On Wed, Jan 24, 2024 at 12:47:14PM -0500, Frank Li wrote: > > > On Wed, Jan 24, 2024 at 05:36:42PM +0000, Conor Dooley wrote: > > > > On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > > > > > From: Ran Wang <ran.wang_1@nxp.com> > > > > > > > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > > > > (or its control signal) will turn on immediately on related Root Hub > > > > > ports. Then the VBUS will be de-asserted for a little while during xhci > > > > > reset (conducted by xhci driver) for a little while and back to normal. > > > > > > > > > > This VBUS glitch might cause some USB devices emuration fail if kernel > > > > > boot with them connected. One SW workaround which can fix this is to > > > > > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > > > > > host mode in DWC3 driver(per signal measurement result, it will be too > > > > > late to do it in xhci-plat.c or xhci.c). > > > > > > > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > > > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > > --- > > > > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > index 203a1eb66691f..dbf272b76e0b5 100644 > > > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > @@ -273,6 +273,13 @@ properties: > > > > > with an external supply. > > > > > type: boolean > > > > > > > > > > + snps,host-vbus-glitches: > > > > > + description: > > > > > + When set, power off all Root Hub ports immediately after > > > > > + setting host mode to avoid vbus (negative) glitch happen in later > > > > > + xhci reset. And the vbus will back to 5V automatically when reset done. > > > > nit: "will return to" > > > > > > > + type: boolean > > > > > > > > Why do we want to have a property for this at all? The commit message > > > > seems to describe a problem that's limited to specific configurations > > > > and appears to be somethng the driver should do unconditionally. > > > > > > > > Could you explain why this cannot be done unconditionally please? > > > > > > It depends on board design, not all system vbus can be controller by root > > > hub port. If it is always on, it will not trigger this issue. > > > > Okay, that seems reasonable to have a property for. Can you add that > > info to the commit message please? > > But if vbus is always on, then applying the work-around would be a NOP, > right? So you could just apply this unconditionally. Supposed yes. But I have not confidence to apply this unconditionaly. There are too much difference SOC and dwc3 version. Not sure if it brokes something. I think it should apply workround as less as possible. Frank > > Rob ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch 2024-01-30 19:29 ` Frank Li @ 2024-02-01 1:31 ` Thinh Nguyen 0 siblings, 0 replies; 19+ messages in thread From: Thinh Nguyen @ 2024-02-01 1:31 UTC (permalink / raw) To: Frank Li Cc: Rob Herring, Conor Dooley, ran.wang_1, Greg Kroah-Hartman, Krzysztof Kozlowski, Conor Dooley, Felipe Balbi, open list:USB SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, open list, mark.rutland, pku.leo, sergei.shtylyov On Tue, Jan 30, 2024, Frank Li wrote: > On Tue, Jan 30, 2024 at 12:13:22PM -0600, Rob Herring wrote: > > On Wed, Jan 24, 2024 at 05:59:00PM +0000, Conor Dooley wrote: > > > On Wed, Jan 24, 2024 at 12:47:14PM -0500, Frank Li wrote: > > > > On Wed, Jan 24, 2024 at 05:36:42PM +0000, Conor Dooley wrote: > > > > > On Fri, Jan 19, 2024 at 04:31:28PM -0500, Frank Li wrote: > > > > > > From: Ran Wang <ran.wang_1@nxp.com> > > > > > > > > > > > > When DWC3 is set to host mode by programming register DWC3_GCTL, VBUS > > > > > > (or its control signal) will turn on immediately on related Root Hub > > > > > > ports. Then the VBUS will be de-asserted for a little while during xhci > > > > > > reset (conducted by xhci driver) for a little while and back to normal. > > > > > > > > > > > > This VBUS glitch might cause some USB devices emuration fail if kernel > > > > > > boot with them connected. One SW workaround which can fix this is to > > > > > > program all PORTSC[PP] to 0 to turn off VBUS immediately after setting > > > > > > host mode in DWC3 driver(per signal measurement result, it will be too > > > > > > late to do it in xhci-plat.c or xhci.c). > > > > > > > > > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com> > > > > > > Reviewed-by: Peter Chen <peter.chen@nxp.com> > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > > > --- > > > > > > Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 7 +++++++ > > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > > index 203a1eb66691f..dbf272b76e0b5 100644 > > > > > > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > > @@ -273,6 +273,13 @@ properties: > > > > > > with an external supply. > > > > > > type: boolean > > > > > > > > > > > > + snps,host-vbus-glitches: > > > > > > + description: > > > > > > + When set, power off all Root Hub ports immediately after > > > > > > + setting host mode to avoid vbus (negative) glitch happen in later > > > > > > + xhci reset. And the vbus will back to 5V automatically when reset done. > > > > > > nit: "will return to" > > > > > > > > > + type: boolean > > > > > > > > > > Why do we want to have a property for this at all? The commit message > > > > > seems to describe a problem that's limited to specific configurations > > > > > and appears to be somethng the driver should do unconditionally. > > > > > > > > > > Could you explain why this cannot be done unconditionally please? > > > > > > > > It depends on board design, not all system vbus can be controller by root > > > > hub port. If it is always on, it will not trigger this issue. > > > > > > Okay, that seems reasonable to have a property for. Can you add that > > > info to the commit message please? > > > > But if vbus is always on, then applying the work-around would be a NOP, > > right? So you could just apply this unconditionally. > > Supposed yes. But I have not confidence to apply this unconditionaly. > There are too much difference SOC and dwc3 version. Not sure if it brokes > something. I think it should apply workround as less as possible. > I can't confirm if there will be negative effect to any platform in the market. But from review, IMHO functionally it should be fine applying this unconditionally. BR, Thinh ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 0/2] usb: dwc3: Add avoiding vbus glitch happen during xhci reset @ 2019-01-16 6:48 Ran Wang 2019-01-16 6:48 ` [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot Ran Wang 0 siblings, 1 reply; 19+ messages in thread From: Ran Wang @ 2019-01-16 6:48 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi Cc: linux-usb, devicetree, linux-kernel, Ran Wang This to fix USB enumeration compatibility issue found on DWC3 (host mode) IP only. Some pre-discussion mails can be referred from: https://lkml.org/lkml/2018/11/23/387 https://lkml.org/lkml/2018/11/22/683 As to the workaround, I know programming xhci register in DWC3 dirver (probe function) is not good from perspective of SW stack, but it seems to be the only place to fix this real existing problem (test result show that doing this in xhci-plat.c or xhci.c would not hlep on this kind of failure). If who have better idea, please let me know, thanks in advanced. Ran Wang (2): dt-bindings: Add workaround for host mode VBUS glitch when boot dwc3 core driver: Add avoiding vbus glitch happen during xhci reset Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ drivers/usb/dwc3/core.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 10 +++++++++- 3 files changed, 60 insertions(+), 0 deletions(-) -- 1.7.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot 2019-01-16 6:48 [PATCH 0/2] usb: dwc3: Add avoiding vbus glitch happen during xhci reset Ran Wang @ 2019-01-16 6:48 ` Ran Wang 2019-01-16 8:21 ` Felipe Balbi 2019-01-16 13:11 ` kbuild test robot 0 siblings, 2 replies; 19+ messages in thread From: Ran Wang @ 2019-01-16 6:48 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi Cc: linux-usb, devicetree, linux-kernel, Ran Wang When DWC3 is set to host mode by programming register DWC3_GCTL, VUBS (or its control signal) will be turned on immediately on related Root Hub ports. Then, the VUBS is turned off for a little while(15us) when do xhci reset (conducted by xhci driver) and back to normal finally, we can observed a negative glitch of related signal from scope. This VBUS glitch might cause some USB devices enumeration fail if kernel boot with them connected. Such as LS1012AFWRY/LS1043ARDB/LX2160AQDS /LS1088ARDB with Kingston 16GB USB2.0/Kingston USB3.0/JetFlash Transcend 4GB USB2.0 drives. The fail cases include enumerated as full-speed device or report wrong device descriptor, etc. One SW workaround which can fix this is to program all xhci PORTSC[PP] to 0 to turn off VBUS immediately after setting host mode in DWC3 driver (per signal measurement result, it will be too late to do it in xhci-plat.c or xhci.c). Then, after xhci reset complete in xhci driver, PORTSC[PP]s' value will back to 1 automatically and VBUS on at that time, no glitch happen and normal enumeration process has no impact. Signed-off-by: Ran Wang <ran.wang_1@nxp.com> --- drivers/usb/dwc3/core.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 10 +++++++++- 2 files changed, 56 insertions(+), 1 deletions(-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index a1b126f..1508397 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -100,6 +100,41 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc) return 0; } +/* + * dwc3_power_of_all_roothub_ports - Power off all Root hub ports + * @dwc3: Pointer to our controller context structure + */ +static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc) +{ + int i, port_num; + u32 reg, op_regs_base, offset; + void __iomem *xhci_regs; + + /* xhci regs is not mapped yet, do it temperary here */ + if (dwc->xhci_resources[0].start) { + xhci_regs = ioremap(dwc->xhci_resources[0].start, + DWC3_XHCI_REGS_END); + if (IS_ERR(xhci_regs)) { + dev_err(dwc->dev, "Failed to ioremap xhci_regs\n"); + return; + } + + op_regs_base = HC_LENGTH(readl(xhci_regs)); + reg = readl(xhci_regs + XHCI_HCSPARAMS1); + port_num = HCS_MAX_PORTS(reg); + + for (i = 1; i <= port_num; i++) { + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); + reg = readl(xhci_regs + offset); + reg &= ~PORT_POWER; + writel(reg, xhci_regs + offset); + } + + iounmap(xhci_regs); + } else + dev_err(dwc->dev, "xhci base reg invalid\n"); +} + void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) { u32 reg; @@ -109,6 +144,15 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) reg |= DWC3_GCTL_PRTCAPDIR(mode); dwc3_writel(dwc->regs, DWC3_GCTL, reg); + /* + * We have to power off all Root hub ports immediately after DWC3 set + * to host mode to avoid VBUS glitch happen when xhci get reset later. + */ + if (dwc->avoid_vbus_glitch_when_set_host) { + if (mode == DWC3_GCTL_PRTCAP_HOST) + dwc3_power_off_all_roothub_ports(dwc); + } + dwc->current_dr_role = mode; } @@ -1306,6 +1350,9 @@ static void dwc3_get_properties(struct dwc3 *dwc) dwc->dis_metastability_quirk = device_property_read_bool(dev, "snps,dis_metastability_quirk"); + dwc->avoid_vbus_glitch_when_set_host = device_property_read_bool(dev, + "snps,avoid-vbus-glitch-when-set-host"); + dwc->lpm_nyet_threshold = lpm_nyet_threshold; dwc->tx_de_emphasis = tx_de_emphasis; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index df87641..691093b 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -606,6 +606,14 @@ #define DWC3_OSTS_VBUSVLD BIT(1) #define DWC3_OSTS_CONIDSTS BIT(0) +/* Partial XHCI Register and Bit fields for quirk */ +#define XHCI_HCSPARAMS1 0x4 +#define XHCI_PORTSC_BASE 0x400 +#define PORT_POWER (1 << 9) +#define HCS_MAX_PORTS(p) (((p) >> 24) & 0x7f) +#define XHCI_HC_LENGTH(p) (((p)>>00)&0x00ff) +#define HC_LENGTH(p) XHCI_HC_LENGTH(p) + /* Structures */ struct dwc3_trb; @@ -1209,6 +1217,7 @@ struct dwc3 { unsigned tx_de_emphasis:2; unsigned dis_metastability_quirk:1; + unsigned avoid_vbus_glitch_when_set_host:1; u16 imod_interval; }; @@ -1217,7 +1226,6 @@ struct dwc3 { #define INCRX_UNDEF_LENGTH_BURST_MODE 1 #define work_to_dwc(w) (container_of((w), struct dwc3, drd_work)) - /* -------------------------------------------------------------------------- */ struct dwc3_event_type { -- 1.7.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot 2019-01-16 6:48 ` [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot Ran Wang @ 2019-01-16 8:21 ` Felipe Balbi 2019-01-16 13:11 ` kbuild test robot 1 sibling, 0 replies; 19+ messages in thread From: Felipe Balbi @ 2019-01-16 8:21 UTC (permalink / raw) To: Ran Wang, Greg Kroah-Hartman, Rob Herring, Mark Rutland Cc: linux-usb, devicetree, linux-kernel, Ran Wang [-- Attachment #1: Type: text/plain, Size: 973 bytes --] Hi, Ran Wang <ran.wang_1@nxp.com> writes: > +static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc) > +{ > + int i, port_num; > + u32 reg, op_regs_base, offset; > + void __iomem *xhci_regs; > + > + /* xhci regs is not mapped yet, do it temperary here */ > + if (dwc->xhci_resources[0].start) { > + xhci_regs = ioremap(dwc->xhci_resources[0].start, > + DWC3_XHCI_REGS_END); > + if (IS_ERR(xhci_regs)) { > + dev_err(dwc->dev, "Failed to ioremap xhci_regs\n"); > + return; > + } > + > + op_regs_base = HC_LENGTH(readl(xhci_regs)); > + reg = readl(xhci_regs + XHCI_HCSPARAMS1); > + port_num = HCS_MAX_PORTS(reg); > + > + for (i = 1; i <= port_num; i++) { > + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); > + reg = readl(xhci_regs + offset); > + reg &= ~PORT_POWER; > + writel(reg, xhci_regs + offset); > + } > + > + iounmap(xhci_regs); why can't this be done during xhci_gen_setup()? -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot 2019-01-16 6:48 ` [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot Ran Wang 2019-01-16 8:21 ` Felipe Balbi @ 2019-01-16 13:11 ` kbuild test robot 2019-02-15 8:39 ` Ran Wang 1 sibling, 1 reply; 19+ messages in thread From: kbuild test robot @ 2019-01-16 13:11 UTC (permalink / raw) To: Ran Wang Cc: kbuild-all, Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi, linux-usb, devicetree, linux-kernel, Ran Wang [-- Attachment #1: Type: text/plain, Size: 23498 bytes --] Hi Ran, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on usb/usb-testing] [also build test WARNING on v5.0-rc2 next-20190116] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Ran-Wang/usb-dwc3-Add-avoiding-vbus-glitch-happen-during-xhci-reset/20190116-153950 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/linux/interrupt.h:268: warning: Function parameter or member 'is_managed' not described in 'irq_affinity_desc' include/linux/rcupdate_wait.h:1: warning: no structured comments found include/linux/rcutree.h:1: warning: no structured comments found kernel/rcu/tree.c:710: warning: Excess function parameter 'irq' description in 'rcu_nmi_exit' include/linux/gfp.h:1: warning: no structured comments found include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev' include/net/cfg80211.h:4687: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev' include/net/mac80211.h:2393: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw' include/net/mac80211.h:2393: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw' include/net/mac80211.h:1004: warning: Function parameter or member 'control.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'status.is_valid_ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'status.status_driver_data' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info' include/net/mac80211.h:1004: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:590: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info' kernel/rcu/tree.c:711: warning: Excess function parameter 'irq' description in 'rcu_nmi_exit' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf' include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array' include/linux/firmware/intel/stratix10-svc-client.h:1: warning: no structured comments found include/linux/gpio/driver.h:371: warning: Function parameter or member 'init_valid_mask' not described in 'gpio_chip' include/linux/iio/hw-consumer.h:1: warning: no structured comments found include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry' drivers/mtd/nand/raw/nand_base.c:420: warning: Function parameter or member 'chip' not described in 'nand_fill_oob' drivers/mtd/nand/raw/nand_bbt.c:173: warning: Function parameter or member 'this' not described in 'read_bbt' drivers/mtd/nand/raw/nand_bbt.c:173: warning: Excess function parameter 'chip' description in 'read_bbt' include/linux/regulator/machine.h:199: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints' include/linux/regulator/driver.h:228: warning: Function parameter or member 'resume' not described in 'regulator_ops' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb' drivers/slimbus/stream.c:1: warning: no structured comments found include/linux/spi/spi.h:180: warning: Function parameter or member 'driver_override' not described in 'spi_device' drivers/target/target_core_device.c:1: warning: no structured comments found >> drivers/usb/dwc3/core.h:1224: warning: Function parameter or member 'avoid_vbus_glitch_when_set_host' not described in 'dwc3' drivers/usb/typec/bus.c:1: warning: no structured comments found drivers/usb/typec/class.c:1: warning: no structured comments found include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family' fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete' fs/file_table.c:1: warning: no structured comments found fs/libfs.c:477: warning: Excess function parameter 'available' description in 'simple_write_end' fs/posix_acl.c:646: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode' fs/posix_acl.c:646: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode' fs/posix_acl.c:646: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:183: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:295: warning: Function parameter or member 'range' not described in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:295: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:295: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:295: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Function parameter or member 'range' not described in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:382: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor ' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:383: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor ' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'end' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:845: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1350: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1350: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1350: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1350: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1350: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1350: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1350: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1517: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1517: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1517: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1517: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1517: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1517: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1517: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1517: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3093: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_make_compute' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:128: warning: Incorrect use of kernel-doc format: Documentation Makefile include scripts source @atomic_obj drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'atomic_obj' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'atomic_obj_lock' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'backlight_link' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'backlight_caps' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'freesync_module' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'fw_dmcu' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:203: warning: Function parameter or member 'dmcu_fw_version' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:1: warning: no structured comments found include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver' include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver' include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver' include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver' include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver' include/drm/drm_drv.h:618: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver' include/drm/drm_atomic_state_helper.h:1: warning: no structured comments found drivers/gpu/drm/drm_dp_helper.c:1364: warning: Function parameter or member 'dsc_dpcd' not described in 'drm_dp_dsc_sink_max_slice_count' drivers/gpu/drm/drm_dp_helper.c:1364: warning: Function parameter or member 'is_edp' not described in 'drm_dp_dsc_sink_max_slice_count' drivers/gpu/drm/i915/i915_vma.h:49: warning: cannot understand function prototype: 'struct i915_vma ' drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found drivers/gpu/drm/i915/intel_guc_fwif.h:536: warning: cannot understand function prototype: 'struct guc_log_buffer_state ' drivers/gpu/drm/i915/i915_trace.h:1: warning: no structured comments found include/linux/skbuff.h:876: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'list' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'head_frag' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'encapsulation' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'csum_valid' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'vlan_present' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'csum_level' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff' include/linux/skbuff.h:876: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff' include/net/sock.h:238: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_portpair' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common' vim +1224 drivers/usb/dwc3/core.h 72246da4 Felipe Balbi 2011-08-19 @1224 :::::: The code at line 1224 was first introduced by commit :::::: 72246da40f3719af3bfd104a2365b32537c27d83 usb: Introduce DesignWare USB3 DRD Driver :::::: TO: Felipe Balbi <balbi@ti.com> :::::: CC: Greg Kroah-Hartman <gregkh@suse.de> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 6630 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot 2019-01-16 13:11 ` kbuild test robot @ 2019-02-15 8:39 ` Ran Wang 0 siblings, 0 replies; 19+ messages in thread From: Ran Wang @ 2019-02-15 8:39 UTC (permalink / raw) To: Felipe Balbi Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, linux-usb, devicetree, linux-kernel Hi Felipe, Sorry for the late response, I didn't receive your mail. Felipe Balbi <balbi@kernel.org> wrotes: >Hi, > >Ran Wang <ran.wang_1@nxp.com> writes: >> +static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc) >> +{ >> + int i, port_num; >> + u32 reg, op_regs_base, offset; >> + void __iomem *xhci_regs; >> + >> + /* xhci regs is not mapped yet, do it temperary here */ >> + if (dwc->xhci_resources[0].start) { >> + xhci_regs = ioremap(dwc->xhci_resources[0].start, >> + DWC3_XHCI_REGS_END); >> + if (IS_ERR(xhci_regs)) { >> + dev_err(dwc->dev, "Failed to ioremap xhci_regs\n"); >> + return; >> + } >> + >> + op_regs_base = HC_LENGTH(readl(xhci_regs)); >> + reg = readl(xhci_regs + XHCI_HCSPARAMS1); >> + port_num = HCS_MAX_PORTS(reg); >> + >> + for (i = 1; i <= port_num; i++) { >> + offset = op_regs_base + XHCI_PORTSC_BASE + 0x10*(i-1); >> + reg = readl(xhci_regs + offset); >> + reg &= ~PORT_POWER; >> + writel(reg, xhci_regs + offset); >> + } >> + >> + iounmap(xhci_regs); > >why can't this be done during xhci_gen_setup()? Actually I have done experiment like what you suggested (in xhci-plat.c), but the timing seems too late--making VBUS waveform look like a square wave as below: Here DWC3 enable host mode, VBUS on | +5V /---------\ 40ms /---------------------------.... 0V ________/ 90ms \______/ | | | Here do xhci reset, VBUS back to +5V again Here set all PORTSC[PP] to 0 in xhci_gen_setup() So I am afraid the solution might have to be added in DWC3 core driver where just after host mode enabling code if want fix this :( Regards, Ran ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-02-01 1:31 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-19 21:31 [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch Frank Li 2024-01-19 21:31 ` [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot Frank Li 2024-01-20 0:51 ` Thinh Nguyen 2024-01-20 0:55 ` Thinh Nguyen 2024-01-20 2:00 ` Frank Li 2024-01-24 17:36 ` [PATCH 1/2] dt-bindings: usb: dwc3: Add snps,host-vbus-glitches avoiding vbus glitch Conor Dooley 2024-01-24 17:47 ` Frank Li 2024-01-24 17:59 ` Conor Dooley 2024-01-24 19:17 ` Frank Li 2024-01-25 17:43 ` Conor Dooley 2024-01-26 4:06 ` Frank Li 2024-01-26 16:34 ` Conor Dooley 2024-01-30 18:13 ` Rob Herring 2024-01-30 19:29 ` Frank Li 2024-02-01 1:31 ` Thinh Nguyen -- strict thread matches above, loose matches on Subject: below -- 2019-01-16 6:48 [PATCH 0/2] usb: dwc3: Add avoiding vbus glitch happen during xhci reset Ran Wang 2019-01-16 6:48 ` [PATCH 2/2] usb: dwc3: Add workaround for host mode VBUS glitch when boot Ran Wang 2019-01-16 8:21 ` Felipe Balbi 2019-01-16 13:11 ` kbuild test robot 2019-02-15 8:39 ` Ran Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).