On 10/20/2017 3:57 PM, Minas Harutyunyan wrote: > On 10/20/2017 12:20 AM, John Stultz wrote: >> On Wed, Oct 18, 2017 at 11:46 PM, Minas Harutyunyan >> <Minas.Harutyunyan@xxxxxxxxxxxx> wrote: >>> Could you please apply this patch. Please not apply your patch series >>> "[PATCH 0/3] dwc2 fixes for edge cases on hikey" to check only below patch. >>> If you confirm that this patch fix your issue with "Transaction Error" >>> and " ChHltd set, but reason is unknown" I'll submit to LKML as final patch. >>> Then can be applied your patch series, without patch 1/3 (changing in >>> connector id status change) to correctly set UDC states. >>> >>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index >>> f4ef159b538e..7da22152df68 100644 >>> --- a/drivers/usb/dwc2/hcd.c >>> +++ b/drivers/usb/dwc2/hcd.c >>> @@ -331,6 +331,9 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg) >>> usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); >>> usbcfg &= ~(GUSBCFG_HNPCAP | GUSBCFG_SRPCAP); >>> >>> + /* Set HS/FS Timeout Calibration */ >>> + usbcfg |= GUSBCFG_TOUTCAL(7); >>> + >>> switch (hsotg->hw_params.op_mode) { >>> case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: >>> if (hsotg->params.otg_cap == >> >> >> So while using this patch and the earlier one from Vardan, I don't see >> the "Transaction Error" >> and " ChHltd set, but reason is unknown" messages, but I do see: >> >> [ 272.290459] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290476] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290491] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290507] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290522] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290538] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290554] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290570] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290585] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290687] dwc2 f72c0000.usb: dwc2_hsotg_ep_stop_xfr: timeout DIEPINT.NAKEFF >> [ 272.290702] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> [ 272.290717] dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently >> in Host mode >> >> After which the USB eth device stops functioning. >> >> So its not really much functional change from without the patch from >> my perspective. >> >> Additionally, I'm still not seeing any disconnect calls when removing >> the B plug. >> >> Re-adding my three patches ontop of this change and Vardan's does fix >> both the UDC state handling and avoids usb devices from failing when >> the board is in host mode and the /dev/usb-ffs/adb/ep0 file is closed >> by adbd. >> >> >> thanks >> -john >> > > Hi John Stultz, > > It's good news that you not see more "Transaction error" and "ChHlt set, > but reason is unknown"! > > To avoid "Mode Mismatch" interrupt please apply your patch 2/3. > > Additionally I see that in your setup used UTMI+ 8-bit PHY but > GUSBCFG.USBTRDTIM set to 5, but should be 9. > In device mode in function dwc2_hsotg_core_init_disconnected() GUSBCFG > related fields TOUTCAL and USBTRDTIM setting again, but in Host mode > these fields are not set at all. This can be reason why using your patch > 1/3 you overcome TOUTCAL and USBTRDTIM issue. > > So, suggesting another patch for USBTRDTIM field bellow: > > @@ -2311,10 +2314,17 @@ static int dwc2_core_init(struct dwc2_hsotg > *hsotg, bool initial_setup) > */ > static void dwc2_core_host_init(struct dwc2_hsotg *hsotg) > { > - u32 hcfg, hfir, otgctl; > + u32 hcfg, hfir, otgctl, usbcfg, trdtrim; > > dev_dbg(hsotg->dev, "%s(%p)\n", __func__, hsotg); > > + /* Set USBTrdTim value */ > + usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); > + usbcfg &= ~GUSBCFG_USBTRDTIM_MASK; > + trdtrim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5; > + usbcfg |= (trdtrim << GUSBCFG_USBTRDTIM_SHIFT); > + dwc2_writel(usbcfg, hsotg->regs + GUSBCFG); > + > /* Restart the Phy Clock */ > dwc2_writel(0, hsotg->regs + PCGCTL); > > > Please apply follow patches: > 1. Vardan's patch. > 2. Patch for TOUTCAL. > 3. Patch for USBTRDTIM (see above). > 4. Your patch 2/3 to avoid "Mode Mismatch" interrupts. > 5. Your patch 3/3 to set udc state to "not attached". > > Unfortunately, we haven't hikey board in our lab and can't fully test > patches on our side. > > > Thanks, > Minas > Hi John Stultz, Could you please verify on your setup follow patches: 1. Vardan's patch. 2. Patch for TOUTCAL&USBTRDTIM programming (new version see below). 4. Your patch 2/3 to avoid "Mode Mismatch" interrupts. 5. Your patch 3/3 to set udc state to "not attached". 6. Your patch 1/3, but remove dwc2_hsotg_core_init_disconnected() function call from Host starting brnch, keep *only* dwc2_hsotg_disconnect() to change UDC state to "not attached". diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index f4ef159b538e..1f65464055b9 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -2311,10 +2311,17 @@ static int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup) */ static void dwc2_core_host_init(struct dwc2_hsotg *hsotg) { - u32 hcfg, hfir, otgctl; + u32 hcfg, hfir, otgctl, usbcfg, val; dev_dbg(hsotg->dev, "%s(%p)\n", __func__, hsotg); + /* Set HS/FS Timeout Calibration and USBTrdTim */ + usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); + usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_USBTRDTIM_MASK); + val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5; + usbcfg |= (GUSBCFG_TOUTCAL(7) | (val << GUSBCFG_USBTRDTIM_SHIFT)); + dwc2_writel(usbcfg, hsotg->regs + GUSBCFG); + /* Restart the Phy Clock */ dwc2_writel(0, hsotg->regs + PCGCTL); Thanks, Minas -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html