linux-kernel - Re: [PATCH 4/6] arm64: dts: rockchip: add hevc power domain clock to rk3328

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMdYzYquvGBzfWqvUOku7m1sM9qxmBHAL95cANDViUCT1oEEhQ@mail.gmail.com>
Date: Tue, 10 Dec 2024 08:23:22 -0500
From: Peter Geis <pgwipeout@...il.com>
To: Dragan Simic <dsimic@...jaro.org>
Cc: Heiko Stuebner <heiko@...ech.de>, Alex Bee <knaerzche@...il.com>, 
	Conor Dooley <conor+dt@...nel.org>, Diederik de Haas <didi.debian@...ow.org>, 
	Johan Jonker <jbx6244@...il.com>, Jonas Karlman <jonas@...boo.se>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Liang Chen <cl@...k-chips.com>, Rob Herring <robh@...nel.org>, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org, 
	shironeko <shironeko@...aguri.club>
Subject: Re: [PATCH 4/6] arm64: dts: rockchip: add hevc power domain clock to rk3328

On Tue, Dec 10, 2024 at 8:13 AM Peter Geis <pgwipeout@...il.com> wrote:
>
> On Tue, Dec 10, 2024 at 5:04 AM Dragan Simic <dsimic@...jaro.org> wrote:
> >
> > Hello Peter,
> >
> > On 2024-12-10 02:30, Peter Geis wrote:
> > > There is a race condition at startup between disabling power domains
> > > not
> > > used and disabling clocks not used on the rk3328. When the clocks are
> > > disabled first, the hevc power domain fails to shut off leading to a
> > > splat of failures. Add the hevc core clock to the rk3328 power domain
> > > node to prevent this condition.
> > >
> > > rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 3-....
> > > }
> > > 1087 jiffies s: 89 root: 0x8/.
> > > rcu: blocking rcu_node structures (internal RCU debug):
> > > Sending NMI from CPU 0 to CPUs 3:
> > > NMI backtrace for cpu 3
> > > CPU: 3 UID: 0 PID: 86 Comm: kworker/3:3 Not tainted 6.12.0-rc5+ #53
> > > Hardware name: Firefly ROC-RK3328-CC (DT)
> > > Workqueue: pm genpd_power_off_work_fn
> > > pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > pc : regmap_unlock_spinlock+0x18/0x30
> > > lr : regmap_read+0x60/0x88
> > > sp : ffff800081123c00
> > > x29: ffff800081123c00 x28: ffff2fa4c62cad80 x27: 0000000000000000
> > > x26: ffffd74e6e660eb8 x25: ffff2fa4c62cae00 x24: 0000000000000040
> > > x23: ffffd74e6d2f3ab8 x22: 0000000000000001 x21: ffff800081123c74
> > > x20: 0000000000000000 x19: ffff2fa4c0412000 x18: 0000000000000000
> > > x17: 77202c31203d2065 x16: 6c6469203a72656c x15: 6c6f72746e6f632d
> > > x14: 7265776f703a6e6f x13: 2063766568206e69 x12: 616d6f64202c3431
> > > x11: 347830206f742030 x10: 3430303034783020 x9 : ffffd74e6c7369e0
> > > x8 : 3030316666206e69 x7 : 205d383738353733 x6 : 332e31202020205b
> > > x5 : ffffd74e6c73fc88 x4 : ffffd74e6c73fcd4 x3 : ffffd74e6c740b40
> > > x2 : ffff800080015484 x1 : 0000000000000000 x0 : ffff2fa4c0412000
> > > Call trace:
> > > regmap_unlock_spinlock+0x18/0x30
> > > rockchip_pmu_set_idle_request+0xac/0x2c0
> > > rockchip_pd_power+0x144/0x5f8
> > > rockchip_pd_power_off+0x1c/0x30
> > > _genpd_power_off+0x9c/0x180
> > > genpd_power_off.part.0.isra.0+0x130/0x2a8
> > > genpd_power_off_work_fn+0x6c/0x98
> > > process_one_work+0x170/0x3f0
> > > worker_thread+0x290/0x4a8
> > > kthread+0xec/0xf8
> > > ret_from_fork+0x10/0x20
> > > rockchip-pm-domain ff100000.syscon:power-controller: failed to get ack
> > > on
> > > domain 'hevc', val=0x88220
> > >
> > > Fixes: 52e02d377a72 ("arm64: dts: rockchip: add core dtsi file for
> > > RK3328 SoCs")
> > > Signed-off-by: Peter Geis <pgwipeout@...il.com>
> >
> > While I was unable to formally verify this clock assignment,
> > i.e. by using the RK3328 TRM or the downstream kernel source
> > from Rockchip, it makes perfect sense to me.  Thanks for the
> > patch, and please feel free to include:
> >
> > Reviewed-by: Dragan Simic <dsimic@...jaro.org>
>
> It is unfortunate the TRM doesn't include the clock maps, because they
> are extremely helpful when one can acquire them. It also doesn't help
> that the TRM register definition only referred to this as "pll". I was
> sent specifically the usb3 phy clock map for my work on the driver,
> which had the location of each switch and divider along with the
> register and bit that controlled it. That combined with the TRM
> register map allowed me to find this error.
>
> Thanks!
> Peter

Apologies, that's the wrong response for this one.
This patch was the result of educated guess combined with testing. I
grabbed all of the clocks that looked like they could affect things,
then tested them one at a time until I isolated them to this clock. It
lives alone with cpll as the parent and has no children according to
the clock summary. (Though the writeup i mistakenly included above
proves the clock map isn't always accurate).

Thanks,
Peter
>
> >
> > > ---
> > >
> > >  arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> > > b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> > > index 0597de415fe0..7d992c3c01ce 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> > > @@ -333,6 +333,7 @@ power: power-controller {
> > >
> > >                       power-domain@...328_PD_HEVC {
> > >                               reg = <RK3328_PD_HEVC>;
> > > +                             clocks = <&cru SCLK_VENC_CORE>;
> > >                               #power-domain-cells = <0>;
> > >                       };
> > >                       power-domain@...328_PD_VIDEO {