Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

From: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
To: John Stultz <john.stultz@linaro.org>,
	Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
Cc: John Youn <John.Youn@synopsys.com>,
	lkml <linux-kernel@vger.kernel.org>,
	Wei Xu <xuwei5@hisilicon.com>, Guodong Xu <guodong.xu@linaro.org>,
	"Amit Pundir" <amit.pundir@linaro.org>,
	YongQin Liu <yongqin.liu@linaro.org>,
	Douglas Anderson <dianders@chromium.org>,
	Chen Yu <chenyu56@huawei.com>,
	Felipe Balbi <felipe.balbi@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey
Date: Fri, 20 Oct 2017 11:48:14 +0000	[thread overview]
Message-ID: <410670D7E743164D87FA6160E7907A560113A31032@am04wembxa.internal.synopsys.com> (raw)
In-Reply-To: CALAqxLWBrnOwgA3QATMY3NGQgm5GZOXQRSpeDtoo7OA7UwRCJA@mail.gmail.com

On 10/20/2017 12:20 AM, John Stultz wrote:
> On Wed, Oct 18, 2017 at 11:46 PM, Minas Harutyunyan
> <Minas.Harutyunyan@synopsys.com> 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
