Date:   Mon, 23 Oct 2017 09:19:51 +0000
From:   Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
To:     John Stultz <john.stultz@...aro.org>,
        Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
CC:     John Youn <John.Youn@...opsys.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Wei Xu <xuwei5@...ilicon.com>,
        Guodong Xu <guodong.xu@...aro.org>,
        "Amit Pundir" <amit.pundir@...aro.org>,
        YongQin Liu <yongqin.liu@...aro.org>,
        Douglas Anderson <dianders@...omium.org>,
        Chen Yu <chenyu56@...wei.com>,
        Felipe Balbi <felipe.balbi@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [RESEND x2][PATCH 0/3] dwc2 fixes for edge cases on hikey

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@...opsys.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
> 
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

Powered by blists - more mailing lists