| Message ID | 20190420064019.57522-5-chenyu56@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for usb on Hikey960 | expand |
Hi Felipe/Greg, What's the status of this patch? I tested here, together with the Hikey 970 phy RFC patches I sent last week. Without this patch, the USB HID driver receives -EPROTO from submitted URBs, causing it to enter into an endless reset cycle on every 500 ms, at the hid_io_error() logic. Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> If you prefer, I can re-submit this one with my SOB. Thanks, Mauro Em Sat, 20 Apr 2019 14:40:10 +0800 Yu Chen <chenyu56@huawei.com> escreveu: > SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core > of Hisilicon Kirin Soc when dwc3 core act as host. > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Felipe Balbi <balbi@kernel.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: John Stultz <john.stultz@linaro.org> > Cc: Binghui Wang <wangbinghui@hisilicon.com> > Signed-off-by: Yu Chen <chenyu56@huawei.com> > --- > v4: > * Add dwc3_complete definition while CONFIG_PM_SLEEP does not defined. > * Add description for 'dis_split_quirk'. > --- > --- > drivers/usb/dwc3/core.c | 26 ++++++++++++++++++++++++++ > drivers/usb/dwc3/core.h | 7 +++++++ > 2 files changed, 33 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index a1b126f90261..c3ef6bd2b0d4 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -117,6 +117,7 @@ static void __dwc3_set_mode(struct work_struct *work) > struct dwc3 *dwc = work_to_dwc(work); > unsigned long flags; > int ret; > + u32 reg; > > if (dwc->dr_mode != USB_DR_MODE_OTG) > return; > @@ -169,6 +170,11 @@ static void __dwc3_set_mode(struct work_struct *work) > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST); > phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST); > phy_calibrate(dwc->usb2_generic_phy); > + if (dwc->dis_split_quirk) { > + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); > + reg |= DWC3_GUCTL3_SPLITDISABLE; > + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); > + } > } > break; > case DWC3_GCTL_PRTCAP_DEVICE: > @@ -1306,6 +1312,9 @@ static void dwc3_get_properties(struct dwc3 *dwc) > dwc->dis_metastability_quirk = device_property_read_bool(dev, > "snps,dis_metastability_quirk"); > > + dwc->dis_split_quirk = device_property_read_bool(dev, > + "snps,dis-split-quirk"); > + > dwc->lpm_nyet_threshold = lpm_nyet_threshold; > dwc->tx_de_emphasis = tx_de_emphasis; > > @@ -1825,10 +1834,27 @@ static int dwc3_resume(struct device *dev) > > return 0; > } > + > +static void dwc3_complete(struct device *dev) > +{ > + struct dwc3 *dwc = dev_get_drvdata(dev); > + u32 reg; > + > + if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST && > + dwc->dis_split_quirk) { > + dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n"); > + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); > + reg |= DWC3_GUCTL3_SPLITDISABLE; > + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); > + } > +} > +#else > +#define dwc3_complete NULL > #endif /* CONFIG_PM_SLEEP */ > > static const struct dev_pm_ops dwc3_dev_pm_ops = { > SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume) > + .complete = dwc3_complete, > SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume, > dwc3_runtime_idle) > }; > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 1528d395b156..28475e301ad9 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -136,6 +136,7 @@ > #define DWC3_GEVNTCOUNT(n) (0xc40c + ((n) * 0x10)) > > #define DWC3_GHWPARAMS8 0xc600 > +#define DWC3_GUCTL3 0xc60c > #define DWC3_GFLADJ 0xc630 > > /* Device Registers */ > @@ -370,6 +371,9 @@ > /* Global User Control Register 2 */ > #define DWC3_GUCTL2_RST_ACTBITLATER BIT(14) > > +/* Global User Control Register 3 */ > +#define DWC3_GUCTL3_SPLITDISABLE BIT(14) > + > /* Device Configuration Register */ > #define DWC3_DCFG_DEVADDR(addr) ((addr) << 3) > #define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f) > @@ -1030,6 +1034,7 @@ struct dwc3_scratchpad_array { > * 2 - No de-emphasis > * 3 - Reserved > * @dis_metastability_quirk: set to disable metastability quirk. > + * @dis_split_quirk: set to disable split boundary. > * @imod_interval: set the interrupt moderation interval in 250ns > * increments or 0 to disable. > */ > @@ -1216,6 +1221,8 @@ struct dwc3 { > > unsigned dis_metastability_quirk:1; > > + unsigned dis_split_quirk:1; > + > u16 imod_interval; > }; > Thanks, Mauro
Hi Mauro, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: > Hi Felipe/Greg, > > What's the status of this patch? to be frank, I don't think I have this in my inbox anymore. > I tested here, together with the Hikey 970 phy RFC patches I sent > last week. > > Without this patch, the USB HID driver receives -EPROTO from > submitted URBs, causing it to enter into an endless reset cycle > on every 500 ms, at the hid_io_error() logic. > Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > If you prefer, I can re-submit this one with my SOB. Please do, but since you're changing device tree, I need Rob's acked-by. > Thanks, > Mauro > > Em Sat, 20 Apr 2019 14:40:10 +0800 > Yu Chen <chenyu56@huawei.com> escreveu: > >> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core >> of Hisilicon Kirin Soc when dwc3 core act as host. is this Kirin-specific or is this something that we should do a revision check? Why does it affect only Hikey kirin? What's the dwc3 revision on that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)? >> @@ -1825,10 +1834,27 @@ static int dwc3_resume(struct device *dev) >> >> return 0; >> } >> + >> +static void dwc3_complete(struct device *dev) >> +{ >> + struct dwc3 *dwc = dev_get_drvdata(dev); >> + u32 reg; >> + >> + if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST && >> + dwc->dis_split_quirk) { >> + dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n"); no more dev_dbg() should be added. This driver relies exclusively on tracepoints for debugging. >> + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); >> + reg |= DWC3_GUCTL3_SPLITDISABLE; >> + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); >> + } >> +} >> +#else >> +#define dwc3_complete NULL >> #endif /* CONFIG_PM_SLEEP */ >> >> static const struct dev_pm_ops dwc3_dev_pm_ops = { >> SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume) >> + .complete = dwc3_complete, why is this done on complete? Why can't it be done at the end of dwc3_resume()?
Em Mon, 07 Sep 2020 17:04:31 +0300 Felipe Balbi <balbi@kernel.org> escreveu: > Hi Mauro, > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: > > > Hi Felipe/Greg, > > > > What's the status of this patch? > > to be frank, I don't think I have this in my inbox anymore. > > > I tested here, together with the Hikey 970 phy RFC patches I sent > > last week. > > > > Without this patch, the USB HID driver receives -EPROTO from > > submitted URBs, causing it to enter into an endless reset cycle > > on every 500 ms, at the hid_io_error() logic. > > > Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > > If you prefer, I can re-submit this one with my SOB. > > Please do, but since you're changing device tree, I need Rob's acked-by. Ok, I'll do that. > > Thanks, > > Mauro > > > > Em Sat, 20 Apr 2019 14:40:10 +0800 > > Yu Chen <chenyu56@huawei.com> escreveu: > > > >> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core > >> of Hisilicon Kirin Soc when dwc3 core act as host. > > is this Kirin-specific or is this something that we should do a revision > check? I've no idea. I don't have any datasheets from this device. > Why does it affect only Hikey kirin? As John Stultz didn't re-submit this one (and looking at the DT between Kirin 960 and 970 from the original Kernel 4.9 official drivers), I suspect that only Kirin 970 requires this quirk. It could well be due to some Dwc3 revision, but it could also be due to some differences at the USB part of the SoC, as there are a few other things different between hikey 960 and 970: it has a different PHY driver, and there are also some differences at the USB HUB which is connected into it. On both devices, the USB physical ports are actually connected into a HUB. In the case of Hikey 970, the hub seems to be a TI TUSB8041 4-Port Hub: $ lsusb Bus 002 Device 002: ID 0451:8140 Texas Instruments, Inc. TUSB8041 4-Port Hub Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub Bus 001 Device 004: ID 090c:1000 Silicon Motion, Inc. - Taiwan (formerly Feiya Technology Corp.) Flash Drive Bus 001 Device 003: ID 413c:301a Dell Computer Corp. Dell MS116 Optical Mouse Bus 001 Device 002: ID 0451:8142 Texas Instruments, Inc. TUSB8041 4-Port Hub Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > What's the dwc3 revision on > that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)? GSNPSID = 0x33313130 > > >> @@ -1825,10 +1834,27 @@ static int dwc3_resume(struct device *dev) > >> > >> return 0; > >> } > >> + > >> +static void dwc3_complete(struct device *dev) > >> +{ > >> + struct dwc3 *dwc = dev_get_drvdata(dev); > >> + u32 reg; > >> + > >> + if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST && > >> + dwc->dis_split_quirk) { > >> + dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n"); > > no more dev_dbg() should be added. This driver relies exclusively on > tracepoints for debugging. Ok. > > >> + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); > >> + reg |= DWC3_GUCTL3_SPLITDISABLE; > >> + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); > >> + } > >> +} > >> +#else > >> +#define dwc3_complete NULL > >> #endif /* CONFIG_PM_SLEEP */ > >> > >> static const struct dev_pm_ops dwc3_dev_pm_ops = { > >> SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume) > >> + .complete = dwc3_complete, > > why is this done on complete? Why can't it be done at the end of > dwc3_resume()? Again, no idea. I didn't actually tried to suspend/resume. Maybe the original author can shed a light on it. Thanks, Mauro
Hi, Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: >> > I tested here, together with the Hikey 970 phy RFC patches I sent >> > last week. >> > >> > Without this patch, the USB HID driver receives -EPROTO from >> > submitted URBs, causing it to enter into an endless reset cycle >> > on every 500 ms, at the hid_io_error() logic. >> >> > Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> >> > >> > If you prefer, I can re-submit this one with my SOB. >> >> Please do, but since you're changing device tree, I need Rob's acked-by. > > Ok, I'll do that. thanks >> > Em Sat, 20 Apr 2019 14:40:10 +0800 >> > Yu Chen <chenyu56@huawei.com> escreveu: >> > >> >> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core >> >> of Hisilicon Kirin Soc when dwc3 core act as host. >> >> is this Kirin-specific or is this something that we should do a revision >> check? > > I've no idea. I don't have any datasheets from this device. I see >> Why does it affect only Hikey kirin? > > As John Stultz didn't re-submit this one (and looking at the DT > between Kirin 960 and 970 from the original Kernel 4.9 official > drivers), I suspect that only Kirin 970 requires this quirk. > > It could well be due to some Dwc3 revision, but it could also be due > to some differences at the USB part of the SoC, as there are a the reason I ask is that if it's caused by dwc3 revision, then we don't need the extra dt property, we can rely on a revision check. If it's something that can't be detected in runtime, then we need a property. > few other things different between hikey 960 and 970: it has a > different PHY driver, and there are also some differences at the > USB HUB which is connected into it. > > On both devices, the USB physical ports are actually connected > into a HUB. In the case of Hikey 970, the hub seems to be a > TI TUSB8041 4-Port Hub: > > $ lsusb > Bus 002 Device 002: ID 0451:8140 Texas Instruments, Inc. TUSB8041 4-Port Hub > Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub > Bus 001 Device 004: ID 090c:1000 Silicon Motion, Inc. - Taiwan (formerly Feiya Technology Corp.) Flash Drive > Bus 001 Device 003: ID 413c:301a Dell Computer Corp. Dell MS116 Optical Mouse > Bus 001 Device 002: ID 0451:8142 Texas Instruments, Inc. TUSB8041 4-Port Hub > Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > >> What's the dwc3 revision on >> that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)? > > GSNPSID = 0x33313130 This isn't even listed as a known revision in dwc3/core.h. Thinh, could the issue being described here caused by a known Erratum with this particular revision? >> >> + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); >> >> + reg |= DWC3_GUCTL3_SPLITDISABLE; >> >> + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); >> >> + } >> >> +} >> >> +#else >> >> +#define dwc3_complete NULL >> >> #endif /* CONFIG_PM_SLEEP */ >> >> >> >> static const struct dev_pm_ops dwc3_dev_pm_ops = { >> >> SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume) >> >> + .complete = dwc3_complete, >> >> why is this done on complete? Why can't it be done at the end of >> dwc3_resume()? > > Again, no idea. I didn't actually tried to suspend/resume. > > Maybe the original author can shed a light on it. yeah, would be nice :-)
Em Mon, 07 Sep 2020 17:04:31 +0300 Felipe Balbi <balbi@kernel.org> escreveu: > >> static const struct dev_pm_ops dwc3_dev_pm_ops = { > >> SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume) > >> + .complete = dwc3_complete, > > why is this done on complete? Why can't it be done at the end of > dwc3_resume()? Updating my answer: No. I just tested moving the code from .complete to the end of .resume. After moving it, the HID won't be able to properly enable the device at resume time (logs filtered to show only relevant msgs): # echo reboot > /sys/power/disk; echo disk > /sys/power/state ... [ 29.051314] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 29.059811] OOM killer disabled. [ 30.171526] PM: hibernation: Creating image: [ 30.171526] PM: hibernation: Need to copy 164192 pages [ 30.171526] PM: hibernation: Image created (164192 pages copied) [ 30.171644] Enabling non-boot CPUs ... [ 30.293953] dwc3 ff100000.dwc3: dwc3_resume [ 30.424549] usb usb1-port1: status 0101 change 0001 [ 30.428538] usb usb2-port1: status 0203 change 0001 [ 30.536609] usb 1-1: Waited 0ms for CONNECT [ 30.543822] usb 1-1: finish reset-resume [ 30.544619] usb 2-1: Waited 0ms for CONNECT [ 30.558869] usb 2-1: finish reset-resume [ 30.676706] usb 1-1: reset high-speed USB device number 2 using xhci-hcd [ 30.882194] hub 1-1:1.0: hub_reset_resume [ 30.888479] hub 1-1:1.0: enabling power on all ports [ 30.960767] usb 2-1: reset SuperSpeed Gen 1 USB device number 2 using xhci-hcd [ 30.994229] hub 2-1:1.0: hub_reset_resume [ 31.000574] usb 1-1-port1: status 0301 change 0001 [ 31.002088] hub 2-1:1.0: enabling power on all ports [ 31.009079] usb 1-1-port2: status 0101 change 0001 [ 31.132633] usb 1-1.2: Waited 0ms for CONNECT [ 31.132665] usb 1-1.1: Waited 0ms for CONNECT [ 31.140560] usb 1-1.2: finish reset-resume [ 31.148509] usb 1-1.1: finish reset-resume [ 31.240760] usb 1-1.2: reset high-speed USB device number 4 using xhci-hcd [ 32.471492] dwc3 ff100000.dwc3: dwc3_complete [ 32.480512] PM: Cannot find swap device, try swapon -a [ 32.489532] PM: Cannot get swap writer [ 32.575790] Restarting tasks ... [ 32.575891] hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0000 [ 32.575894] hub 2-0:1.0: state 7 ports 1 chg 0000 evt 0000 [ 32.575896] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002 [ 32.576570] hub 2-1:1.0: state 7 ports 4 chg 0000 evt 0000 [ 32.576876] done. [ 32.600094] hub 2-1:1.0: hub_suspend [ 32.642090] PM: hibernation: hibernation exit [ 32.650255] usb 2-1: usb auto-suspend, wakeup 1 [ 32.668581] hub 2-0:1.0: hub_suspend [ 33.122384] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002 [ 33.378220] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd [ 33.698394] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000 [ 34.882365] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002 [ 35.138217] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd [ 35.458617] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000 [ 36.642392] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002 [ 36.898207] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd [ 37.218598] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000 [ 37.672658] kirin9xx-drm e8600000.dpe: [drm:dss_disable_vblank [kirin9xx_drm]] dss_disable_vblank [ 38.402368] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002 [ 38.658174] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd [ 38.978594] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000 [ 40.162361] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0002 [ 40.418148] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd ... [ 397.698132] usb 1-1.1: reset low-speed USB device number 3 using xhci-hcd [ 398.018565] hub 1-1:1.0: state 7 ports 4 chg 0000 evt 0000 Basically, after resume, the code behaves just like the quirk were not applied. Explaining with more details, without the splitdisable quirk, what happens is that the USB URBs return -EPROTO errors, causing hid-core.c to call hid_io_error(). This function starts a reset work (usbhid->reset_work), which ends calling (at hid_reset()): usb_queue_reset_device(usbhid->intf); This happens on every HZ/2 intervals, causing an endless reset loop. Thanks, Mauro
Em Tue, 08 Sep 2020 09:09:46 +0300 Felipe Balbi <balbi@kernel.org> escreveu: > >> Why does it affect only Hikey kirin? > > > > As John Stultz didn't re-submit this one (and looking at the DT > > between Kirin 960 and 970 from the original Kernel 4.9 official > > drivers), I suspect that only Kirin 970 requires this quirk. > > > > It could well be due to some Dwc3 revision, but it could also be due > > to some differences at the USB part of the SoC, as there are a > > the reason I ask is that if it's caused by dwc3 revision, then we don't > need the extra dt property, we can rely on a revision check. If it's > something that can't be detected in runtime, then we need a property. Yeah, if it would be possible to auto-detect, that would be better. Yet, hard to say if it would be possible to do that, without access to the Dwc3 docs, and without different hardware for testing it. > > few other things different between hikey 960 and 970: it has a > > different PHY driver, and there are also some differences at the > > USB HUB which is connected into it. > > > > On both devices, the USB physical ports are actually connected > > into a HUB. In the case of Hikey 970, the hub seems to be a > > TI TUSB8041 4-Port Hub: > > > > $ lsusb > > Bus 002 Device 002: ID 0451:8140 Texas Instruments, Inc. TUSB8041 4-Port Hub > > Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub > > Bus 001 Device 004: ID 090c:1000 Silicon Motion, Inc. - Taiwan (formerly Feiya Technology Corp.) Flash Drive > > Bus 001 Device 003: ID 413c:301a Dell Computer Corp. Dell MS116 Optical Mouse > > Bus 001 Device 002: ID 0451:8142 Texas Instruments, Inc. TUSB8041 4-Port Hub > > Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub > > > >> What's the dwc3 revision on > >> that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)? > > > > GSNPSID = 0x33313130 > > This isn't even listed as a known revision in dwc3/core.h. Thinh, could > the issue being described here caused by a known Erratum with this > particular revision? Maybe. Again, hard to tell without any documentation from Synopsys. Maybe some of the Synopsys folks at USB MK can shed a light. Thanks, Mauro
Felipe Balbi wrote: > Hi, > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: >>>> I tested here, together with the Hikey 970 phy RFC patches I sent >>>> last week. >>>> >>>> Without this patch, the USB HID driver receives -EPROTO from >>>> submitted URBs, causing it to enter into an endless reset cycle >>>> on every 500 ms, at the hid_io_error() logic. >>>> Tested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> >>>> >>>> If you prefer, I can re-submit this one with my SOB. >>> Please do, but since you're changing device tree, I need Rob's acked-by. >> Ok, I'll do that. > thanks > >>>> Em Sat, 20 Apr 2019 14:40:10 +0800 >>>> Yu Chen <chenyu56@huawei.com> escreveu: >>>> >>>>> SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core >>>>> of Hisilicon Kirin Soc when dwc3 core act as host. >>> is this Kirin-specific or is this something that we should do a revision >>> check? >> I've no idea. I don't have any datasheets from this device. > I see > >>> Why does it affect only Hikey kirin? >> As John Stultz didn't re-submit this one (and looking at the DT >> between Kirin 960 and 970 from the original Kernel 4.9 official >> drivers), I suspect that only Kirin 970 requires this quirk. >> >> It could well be due to some Dwc3 revision, but it could also be due >> to some differences at the USB part of the SoC, as there are a > the reason I ask is that if it's caused by dwc3 revision, then we don't > need the extra dt property, we can rely on a revision check. If it's > something that can't be detected in runtime, then we need a property. > >> few other things different between hikey 960 and 970: it has a >> different PHY driver, and there are also some differences at the >> USB HUB which is connected into it. >> >> On both devices, the USB physical ports are actually connected >> into a HUB. In the case of Hikey 970, the hub seems to be a >> TI TUSB8041 4-Port Hub: >> >> $ lsusb >> Bus 002 Device 002: ID 0451:8140 Texas Instruments, Inc. TUSB8041 4-Port Hub >> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub >> Bus 001 Device 004: ID 090c:1000 Silicon Motion, Inc. - Taiwan (formerly Feiya Technology Corp.) Flash Drive >> Bus 001 Device 003: ID 413c:301a Dell Computer Corp. Dell MS116 Optical Mouse >> Bus 001 Device 002: ID 0451:8142 Texas Instruments, Inc. TUSB8041 4-Port Hub >> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub >> >>> What's the dwc3 revision on >>> that SoC (grep SNPSID /sys/kernel/debugfs/*dwc3/regdump)? >> GSNPSID = 0x33313130 > This isn't even listed as a known revision in dwc3/core.h. Thinh, could > the issue being described here caused by a known Erratum with this > particular revision? If you have a STAR issue number, then I may be able to look it up. Btw, GNSPSID register is the ID for DWC_usb31 IP. For DWC_usb31 and DWC_usb32, we need to read the version number and version type registers (offset 0xc1a0 and 0xc1a4 respectively). We need to add these registers to the register dump. BR, Thinh > >>>>> + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); >>>>> + reg |= DWC3_GUCTL3_SPLITDISABLE; >>>>> + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); >>>>> + } >>>>> +} >>>>> +#else >>>>> +#define dwc3_complete NULL >>>>> #endif /* CONFIG_PM_SLEEP */ >>>>> >>>>> static const struct dev_pm_ops dwc3_dev_pm_ops = { >>>>> SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume) >>>>> + .complete = dwc3_complete, >>> why is this done on complete? Why can't it be done at the end of >>> dwc3_resume()? >> Again, no idea. I didn't actually tried to suspend/resume. >> >> Maybe the original author can shed a light on it. > yeah, would be nice :-) >
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index a1b126f90261..c3ef6bd2b0d4 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -117,6 +117,7 @@ static void __dwc3_set_mode(struct work_struct *work) struct dwc3 *dwc = work_to_dwc(work); unsigned long flags; int ret; + u32 reg; if (dwc->dr_mode != USB_DR_MODE_OTG) return; @@ -169,6 +170,11 @@ static void __dwc3_set_mode(struct work_struct *work) phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST); phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST); phy_calibrate(dwc->usb2_generic_phy); + if (dwc->dis_split_quirk) { + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); + reg |= DWC3_GUCTL3_SPLITDISABLE; + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); + } } break; case DWC3_GCTL_PRTCAP_DEVICE: @@ -1306,6 +1312,9 @@ static void dwc3_get_properties(struct dwc3 *dwc) dwc->dis_metastability_quirk = device_property_read_bool(dev, "snps,dis_metastability_quirk"); + dwc->dis_split_quirk = device_property_read_bool(dev, + "snps,dis-split-quirk"); + dwc->lpm_nyet_threshold = lpm_nyet_threshold; dwc->tx_de_emphasis = tx_de_emphasis; @@ -1825,10 +1834,27 @@ static int dwc3_resume(struct device *dev) return 0; } + +static void dwc3_complete(struct device *dev) +{ + struct dwc3 *dwc = dev_get_drvdata(dev); + u32 reg; + + if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST && + dwc->dis_split_quirk) { + dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n"); + reg = dwc3_readl(dwc->regs, DWC3_GUCTL3); + reg |= DWC3_GUCTL3_SPLITDISABLE; + dwc3_writel(dwc->regs, DWC3_GUCTL3, reg); + } +} +#else +#define dwc3_complete NULL #endif /* CONFIG_PM_SLEEP */ static const struct dev_pm_ops dwc3_dev_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume) + .complete = dwc3_complete, SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume, dwc3_runtime_idle) }; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 1528d395b156..28475e301ad9 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -136,6 +136,7 @@ #define DWC3_GEVNTCOUNT(n) (0xc40c + ((n) * 0x10)) #define DWC3_GHWPARAMS8 0xc600 +#define DWC3_GUCTL3 0xc60c #define DWC3_GFLADJ 0xc630 /* Device Registers */ @@ -370,6 +371,9 @@ /* Global User Control Register 2 */ #define DWC3_GUCTL2_RST_ACTBITLATER BIT(14) +/* Global User Control Register 3 */ +#define DWC3_GUCTL3_SPLITDISABLE BIT(14) + /* Device Configuration Register */ #define DWC3_DCFG_DEVADDR(addr) ((addr) << 3) #define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f) @@ -1030,6 +1034,7 @@ struct dwc3_scratchpad_array { * 2 - No de-emphasis * 3 - Reserved * @dis_metastability_quirk: set to disable metastability quirk. + * @dis_split_quirk: set to disable split boundary. * @imod_interval: set the interrupt moderation interval in 250ns * increments or 0 to disable. */ @@ -1216,6 +1221,8 @@ struct dwc3 { unsigned dis_metastability_quirk:1; + unsigned dis_split_quirk:1; + u16 imod_interval; };
SPLIT_BOUNDARY_DISABLE should be set for DesignWare USB3 DRD Core of Hisilicon Kirin Soc when dwc3 core act as host. Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Felipe Balbi <balbi@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: John Stultz <john.stultz@linaro.org> Cc: Binghui Wang <wangbinghui@hisilicon.com> Signed-off-by: Yu Chen <chenyu56@huawei.com> --- v4: * Add dwc3_complete definition while CONFIG_PM_SLEEP does not defined. * Add description for 'dis_split_quirk'. --- --- drivers/usb/dwc3/core.c | 26 ++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 7 +++++++ 2 files changed, 33 insertions(+)