All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@kernel.org>
To: John Stultz <john.stultz@linaro.org>,
	Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux USB List <linux-usb@vger.kernel.org>,
	John Youn <John.Youn@synopsys.com>,
	stable <stable@vger.kernel.org>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Wesley Cheng <wcheng@codeaurora.org>,
	Ferry Toth <fntoth@gmail.com>, Yu Chen <chenyu56@huawei.com>
Subject: Re: [PATCH v3] usb: dwc3: core: Do core softreset when switch mode
Date: Sat, 17 Apr 2021 09:25:03 +0300	[thread overview]
Message-ID: <87k0p1mnr4.fsf@kernel.org> (raw)
In-Reply-To: <CALAqxLUgKsJS5Hy=N1KDNP7+v1Y0TxW19m9iD0S4ySq-5qhgjQ@mail.gmail.com>


Hi,

John Stultz <john.stultz@linaro.org> writes:
> On Fri, Apr 16, 2021 at 12:49 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>>
>> John Stultz wrote:
>> > On Fri, Apr 16, 2021 at 3:47 AM Felipe Balbi <balbi@kernel.org> wrote:
>> >>
>> >>
>> >> Hi,
>> >>
>> >> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> >>
>> >>> From: Yu Chen <chenyu56@huawei.com>
>> >>> From: John Stultz <john.stultz@linaro.org>
>> >>>
>> >>> According to the programming guide, to switch mode for DRD controller,
>> >>> the driver needs to do the following.
>> >>>
>> >>> To switch from device to host:
>> >>> 1. Reset controller with GCTL.CoreSoftReset
>> >>> 2. Set GCTL.PrtCapDir(host mode)
>> >>> 3. Reset the host with USBCMD.HCRESET
>> >>> 4. Then follow up with the initializing host registers sequence
>> >>>
>> >>> To switch from host to device:
>> >>> 1. Reset controller with GCTL.CoreSoftReset
>> >>> 2. Set GCTL.PrtCapDir(device mode)
>> >>> 3. Reset the device with DCTL.CSftRst
>> >>> 4. Then follow up with the initializing registers sequence
>> >>>
>> >>> Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
>> >>> switching from host to device. John Stult reported a lockup issue seen
>> >>> with HiKey960 platform without these steps[1]. Similar issue is observed
>> >>> with Ferry's testing platform[2].
>> >>>
>> >>> So, apply the required steps along with some fixes to Yu Chen's and John
>> >>> Stultz's version. The main fixes to their versions are the missing wait
>> >>> for clocks synchronization before clearing GCTL.CoreSoftReset and only
>> >>> apply DCTL.CSftRst when switching from host to device.
>> >>>
>> >>> [1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.org/__;!!A4F2R9G_pg!L4TLb25Nkq0DF2qrCPWW13PUq4idhDn5QSZhgvnVAy7wJiYFOSSouSptwo9nOzIdPD4j$
>> >>> [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail.com/__;!!A4F2R9G_pg!L4TLb25Nkq0DF2qrCPWW13PUq4idhDn5QSZhgvnVAy7wJiYFOSSouSptwo9nO21VT8q7$
>> >>>
>> >>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> >>> Cc: Ferry Toth <fntoth@gmail.com>
>> >>> Cc: Wesley Cheng <wcheng@codeaurora.org>
>> >>> Cc: <stable@vger.kernel.org>
>> >>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly")
>> >>> Signed-off-by: Yu Chen <chenyu56@huawei.com>
>> >>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> >>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>> >>
>> >> I still have concerns about the soft reset, but I won't block you guys
>> >> from fixing Hikey's problem :-)
>> >>
>> >> The only thing I would like to confirm is that this has been verified
>> >> with hundreds of swaps happening as quickly as possible. DWC3 should
>> >> still be functional after several hundred swaps.
>> >>
>> >> Can someone confirm this is the case? (I'm assuming this can be
>> >> scripted)
>> >
>> > I unfortunately don't have an easy way to automate the switching right
>> > off. But I'll try to hack up the mux switch driver to provide an
>> > interface we can script against.
>> >
>>
>> FYI, you can do the following:
>>
>> 1) Enable "usb-role-switch" DT property if not already done so
>> 2) Add userspace control:
>>
>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>> index e2b68bb770d1..b203e3d87291 100644
>> --- a/drivers/usb/dwc3/drd.c
>> +++ b/drivers/usb/dwc3/drd.c
>> @@ -555,6 +555,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc)
>>                 mode = DWC3_GCTL_PRTCAP_DEVICE;
>>         }
>>
>> +       dwc3_role_switch.allow_userspace_control = true;
>>         dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
>>         dwc3_role_switch.set = dwc3_usb_role_switch_set;
>>         dwc3_role_switch.get = dwc3_usb_role_switch_get;
>>
>> 3) Write a script to do the following:
>>
>> # echo host > /sys/class/usb_role/<UDC>/role
>>
>> and
>>
>> # echo device > /sys/class/usb_role/<UDC>/role
>
> Thanks so much for this. So I ran both of those commands in a while
> loop for awhile and didn't see any trouble.
>
> HiKey960 is interesting as well because we have a mux switch, which is
> sort of an intermediary roll switcher (it gets the role switch signal
> from the tcpci_rt1711h, tweaks some gpios and then signals the dwc3).
> So I also did the above tweaks to the mux-switch and had it switching
> between device/none and validated the onboard hub came up and down
> along with the dwc3 core.
>
> Everything still looks good here.

Sounds good, happy to see so many platforms supported by Thinh's
change. Thanks for doing this work, Thinh :-)

-- 
balbi

  reply	other threads:[~2021-04-17  6:25 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15  2:23 [PATCH] usb: dwc3: core: Do core softreset when switch mode Thinh Nguyen
2021-04-15  6:23 ` Felipe Balbi
2021-04-15  7:10   ` Thinh Nguyen
2021-04-15  7:53     ` Greg Kroah-Hartman
2021-04-15  8:04       ` Thinh Nguyen
2021-04-15 10:45     ` Felipe Balbi
2021-04-15 14:57       ` Thinh Nguyen
2021-04-15 22:31         ` Thinh Nguyen
2021-04-15 16:29 ` [PATCH v2] " Thinh Nguyen
2021-04-15 19:54   ` John Stultz
2021-04-15 20:11     ` Thinh Nguyen
2021-04-15 22:20   ` [PATCH v3] " Thinh Nguyen
2021-04-15 22:23     ` Thinh Nguyen
2021-04-16 21:17       ` Ferry Toth
2021-04-17  2:27         ` Thinh Nguyen
2021-04-17 14:22           ` Ferry Toth
2021-04-17 16:32             ` Ferry Toth
2021-04-18 23:03               ` Thinh Nguyen
2021-04-19  8:43                 ` Andy Shevchenko
2021-04-19 20:24                   ` Ferry Toth
2021-04-19  9:47                 ` Felipe Balbi
2021-04-19 20:15                 ` Ferry Toth
2021-04-19 21:23                   ` Thinh Nguyen
2021-04-20 19:55                     ` Ferry Toth
2021-04-21 19:01                       ` Thinh Nguyen
2021-04-21 22:30                         ` Ferry Toth
2021-04-22 20:55                         ` Ferry Toth
2021-04-22 21:58                           ` Thinh Nguyen
2021-04-23  7:18                             ` Ferry Toth
2021-04-16  0:12     ` John Stultz
2021-04-16  3:28       ` John Stultz
2021-04-16  9:10         ` Ferry Toth
2021-04-16 10:47     ` Felipe Balbi
2021-04-16 19:05       ` Wesley Cheng
2021-04-16 19:38       ` John Stultz
2021-04-16 19:49         ` Thinh Nguyen
2021-04-16 21:08           ` John Stultz
2021-04-17  6:25             ` Felipe Balbi [this message]
2021-04-19 19:49               ` Thinh Nguyen
2021-04-19 21:26     ` Wesley Cheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k0p1mnr4.fsf@kernel.org \
    --to=balbi@kernel.org \
    --cc=John.Youn@synopsys.com \
    --cc=Thinh.Nguyen@synopsys.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=chenyu56@huawei.com \
    --cc=fntoth@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=wcheng@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.