From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Roy Luo <royluo@google.com>,
mathias.nyman@intel.com, quic_ugoswami@quicinc.com,
Thinh.Nguyen@synopsys.com, gregkh@linuxfoundation.org,
michal.pecio@gmail.com, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org
Subject: Re: [PATCH v1] Revert "usb: xhci: Implement xhci_handshake_check_state() helper"
Date: Mon, 19 May 2025 15:52:52 +0300 [thread overview]
Message-ID: <8f023425-3f9b-423c-9459-449d0835c608@linux.intel.com> (raw)
In-Reply-To: <20250517043942.372315-1-royluo@google.com>
On 17.5.2025 7.39, Roy Luo wrote:
> This reverts commit 6ccb83d6c4972ebe6ae49de5eba051de3638362c.
>
> Commit 6ccb83d6c497 ("usb: xhci: Implement xhci_handshake_check_state()
> helper") was introduced to workaround watchdog timeout issues on some
> platforms, allowing xhci_reset() to bail out early without waiting
> for the reset to complete.
>
> Skipping the xhci handshake during a reset is a dangerous move. The
> xhci specification explicitly states that certain registers cannot
> be accessed during reset in section 5.4.1 USB Command Register (USBCMD),
> Host Controller Reset (HCRST) field:
> "This bit is cleared to '0' by the Host Controller when the reset
> process is complete. Software cannot terminate the reset process
> early by writinga '0' to this bit and shall not write any xHC
> Operational or Runtime registers until while HCRST is '1'."
>
> This behavior causes a regression on SNPS DWC3 USB controller with
> dual-role capability. When the DWC3 controller exits host mode and
> removes xhci while a reset is still in progress, and then tries to
> configure its hardware for device mode, the ongoing reset leads to
> register access issues; specifically, all register reads returns 0.
> These issues extend beyond the xhci register space (which is expected
> during a reset) and affect the entire DWC3 IP block, causing the DWC3
> device mode to malfunction.
I agree with you and Thinh that waiting for the HCRST bit to clear during
reset is the right thing to do, especially now when we know skipping it
causes issues for SNPS DWC3, even if it's only during remove phase.
But reverting this patch will re-introduce the issue originally worked
around by Udipto Goswami, causing regression.
Best thing to do would be to wait for HCRST to clear for all other platforms
except the one with the issue.
Udipto Goswami, can you recall the platforms that needed this workaroud?
and do we have an easy way to detect those?
Thanks
Mathias
next prev parent reply other threads:[~2025-05-19 12:52 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-17 4:39 [PATCH v1] Revert "usb: xhci: Implement xhci_handshake_check_state() helper" Roy Luo
2025-05-19 12:52 ` Mathias Nyman [this message]
2025-05-19 18:13 ` Udipto Goswami
2025-05-19 22:32 ` Michał Pecio
2025-05-20 12:30 ` Udipto Goswami
2025-05-20 16:18 ` Mathias Nyman
2025-05-22 2:21 ` Roy Luo
2025-05-22 12:24 ` Mathias Nyman
2025-05-22 19:19 ` Roy Luo
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=8f023425-3f9b-423c-9459-449d0835c608@linux.intel.com \
--to=mathias.nyman@linux.intel.com \
--cc=Thinh.Nguyen@synopsys.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=michal.pecio@gmail.com \
--cc=quic_ugoswami@quicinc.com \
--cc=royluo@google.com \
--cc=stable@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox