public inbox for linux-rockchip@lists.infradead.org
 help / color / mirror / Atom feed
From: Sam Edwards <cfsworks@gmail.com>
To: Heiko Stuebner <heiko@sntech.de>, Rob Herring <robh+dt@kernel.org>
Cc: linux-rockchip@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	"Daniel Kukieła" <daniel@kukiela.pl>,
	"Sven Rademakers" <sven.rademakers@gmail.com>,
	"Joshua Riek" <jjriek@verizon.net>,
	"Sam Edwards" <CFSworks@gmail.com>
Subject: [PATCH] arm64: dts: rockchip: rk3588: Fix USB PD clocks
Date: Fri, 15 Dec 2023 19:10:19 -0700	[thread overview]
Message-ID: <20231216021019.1543811-1-CFSworks@gmail.com> (raw)

The QoS blocks saved/restored when toggling the PD_USB power domain are
clocked by ACLK_USB. Attempting to access these memory regions without
that clock running will result in an indefinite CPU stall.

The PD_USB node wasn't specifying this clock dependency, resulting in
hangs when trying to toggle the power domain (either on or off), unless
we get "lucky" and have ACLK_USB running for another reason at the time.
This "luck" can result from the bootloader leaving USB powered/clocked,
and if no built-in driver wants USB, Linux will disable the unused
PD+CLK on boot when {pd,clk}_ignore_unused aren't given. This can also
be unlucky because the two cleanup tasks run in parallel and race: if
the CLK is disabled first, the PD deactivation stalls the boot. In any
case, the PD cannot then be reenabled (if e.g. the driver loads later)
once the clock has been stopped.

Fix this by specifying a dependency on ACLK_USB, instead of only
ACLK_USB_ROOT. The child-parent relationship means the former implies
the latter anyway. By the same token, remove the redundant HCLK_USB_ROOT
reference, since that's also implied by its HCLK_HOST* grandchildren.

Fixes: c9211fa2602b8 ("arm64: dts: rockchip: Add base DT for rk3588 SoC")
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---

Hi list,

Feel free to tell me to knock the HCLK_USB_ROOT change out of there, since it's
not strictly necessary for this fix. It was a "while I'm at it, let's remove
this redundant reference" thing. :)

I also do not know quite enough about debugfs to know how to manually request
clocks and PDs, but if such a mechanism exists, it would be a good way to
confirm the correctness of this fix: ensure ACLK_USB is stopped, then try to
toggle PD_USB a few times.

Cheers,
Sam

---
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index fd1b5d02ba16..32d7e49f03d3 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1341,8 +1341,7 @@ power-domain@RK3588_PD_RGA31 {
 			power-domain@RK3588_PD_USB {
 				reg = <RK3588_PD_USB>;
 				clocks = <&cru PCLK_PHP_ROOT>,
-					 <&cru ACLK_USB_ROOT>,
-					 <&cru HCLK_USB_ROOT>,
+					 <&cru ACLK_USB>,
 					 <&cru HCLK_HOST0>,
 					 <&cru HCLK_HOST_ARB0>,
 					 <&cru HCLK_HOST1>,
-- 
2.41.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

             reply	other threads:[~2023-12-16  2:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-16  2:10 Sam Edwards [this message]
2023-12-24 19:54 ` [PATCH] arm64: dts: rockchip: rk3588: Fix USB PD clocks Heiko Stuebner

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=20231216021019.1543811-1-CFSworks@gmail.com \
    --to=cfsworks@gmail.com \
    --cc=daniel@kukiela.pl \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=jjriek@verizon.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=robh+dt@kernel.org \
    --cc=sven.rademakers@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox