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

[Peter Geis]

On Tue, Dec 10, 2024 at 5:04 AM Dragan Simic <dsimic@xxxxxxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxxxxxx>

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

>
> > ---
> >
> >  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@RK3328_PD_HEVC {
> >                               reg = <RK3328_PD_HEVC>;
> > +                             clocks = <&cru SCLK_VENC_CORE>;
> >                               #power-domain-cells = <0>;
> >                       };
> >                       power-domain@RK3328_PD_VIDEO {