All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@kernel.org>
To: Dejin Zheng <zhengdejin5@gmail.com>
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Subject: Re: [PATCH v3] usb: dwc3: core: fix a issue about clear connect state
Date: Wed, 28 Oct 2020 15:57:03 +0200	[thread overview]
Message-ID: <87y2jqlahc.fsf@kernel.org> (raw)
In-Reply-To: <20201028125812.GA59692@nuc8i5>

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]


Hi,

Dejin Zheng <zhengdejin5@gmail.com> writes:
>> Dejin Zheng <zhengdejin5@gmail.com> writes:
>> > According to Synopsys Programming Guide chapter 2.2 Register Resets,
>> > it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft
>> > reset, if DWC3 controller as a slave device and stay connected with a usb
>> > host, then, while rebooting linux, it will fail to reinitialize dwc3 as a
>> > slave device when the DWC3 controller did not power off. because the
>> > connection status is incorrect, so we also need to clear DCTL.RUN_STOP
>> > bit for disabling connect when doing core soft reset. There will still
>> > be other stale configuration in DCTL, so reset the other fields of DCTL
>> > to the default value 0.
>> 
>> This commit log is a bit hard to understand. When does this problem
>> actually happen? It seems like it's in the case of, perhaps, kexecing
>> into a new kernel, is that right?
>> 
> It happens when entering the kernel for the second time after the reboot
> command.
>
>> At the time dwc3_core_soft_reset() is called, the assumption is that
>> we're starting with a clean core, from power up. If we have stale
>> configuration from a previous run, we should fix this on the exit
>> path. Note that if we're reaching probe with pull up connected, we
>> already have issues elsewhere.
>> 
>> I think this is not the right fix for the problem.
>>
> I think you are right, Thinh also suggested me fix it on the exit path
> in the previous patch v2. Do you think I can do these cleanups in the
> shutdown hook of this driver? Balbi, is there a more suitable place to
> do this by your rich experience? Thanks!

I don't think shutdown is called during removal, I'm not sure. I think
we had some fixes done in shutdown time, though. Test it out, but make
sure there are no issues with a regular modprobe cycle.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

  reply	other threads:[~2020-10-29  0:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20 13:58 [PATCH v3] usb: dwc3: core: fix a issue about clear connect state Dejin Zheng
2020-10-27  8:33 ` Felipe Balbi
2020-10-28 13:46   ` Dejin Zheng
2020-10-28 13:57     ` Felipe Balbi [this message]
2020-10-28 14:43       ` Dejin Zheng
2020-10-30 15:15       ` Dejin Zheng

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=87y2jqlahc.fsf@kernel.org \
    --to=balbi@kernel.org \
    --cc=Thinh.Nguyen@synopsys.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=zhengdejin5@gmail.com \
    /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.