[PATCH v8 7/7] clk: rockchip: implement proper GATE_LINK support

[Sebastian Reichel]

Hello Chad,

On Sun, Mar 17, 2024 at 07:34:41PM -0400, Chad LeClair wrote:
> On 3/8/24 08:27, Sebastian Reichel wrote:
> > [removed DT people from CC, since this is all about clocks now]
> > 
> > Hello Ilya,
> > 
> > On Fri, Mar 08, 2024 at 10:23:31AM +0300, Ilya K wrote:
> >> This change seems to make my Orange Pi 5 (RK3588S) lock up on boot
> >> :( Any suggestions on how to debug this? It doesn't really log
> >> much...
> > 
> > I suppose with this change you explicitly mean the last patch, which
> > has not yet been applied? That patch effectively allows some clocks
> > to be disabled, that have previously been marked as critical (and
> > thus always-on).
> > 
> > If that results in a boot lockup, I expect you have a peripheral
> > driver, that does not enable a required clock (e.g. because of a
> > missing clock reference).
> 
> Another Orange Pi 5 owner here...  I likewise ran into trouble with
> this patch.  I have been playing with your rk3588 branch from the
> Collabora gitlab and was also experiencing hangs on boot on my OPi5.  I
> bisected your branch and found that the GATE_LINK support commit was the
> problem for me.

Ok.

> Digging in, I found that the problem behind the hang was that I was not
> getting the aclk_nvm_root

That matches what Ilya found (there were some more off-list messages
because of HTML mail issues). I suspected, that HCLK_SD (which is
sourced from SCMI) might need HCLK_NVM.

> which in turn caused DMA transactions to the SFC driver to hang.
> 
> ...
> [   2.804519] rockchip-sfc fe2b0000.spi: DMA wait for transfer finish
> timeout
> [   2.805127] rockchip-sfc fe2b0000.spi: xfer data failed ret -110 dir 1
> ...
> 
> Setting aclk_nvm_root to critical allows the system to boot.  However
> not all is well as I do get errors like the following which also seem to
> indicate further clock problems:
> [   6.296554] rockchip-pm-domain
> fd8d8000.power-management:power-controller: failed to set idle on domain
> 'vo0', val=0

I see the same message for AV1 and I've also seen failed idle
message with the rockchip kernel. I have not yet debugged what is
going on with that.

> I assume that the con-ops of your patch is analogous to the downstream
> "clock-link" driver.  (ie. you are looking for PM events to cause the
> pm_clk_suspend()/pm_clk_resume() routines to enable/disable the linked
> (second parent) clocks).  This wasn't happening for me.  Unlike in the
> downstream implementation, the gate_link devices' dev->driver pointer
> was null. So that when rpm_resume() was being called, it would make its
> way to pm_generic_runtime_resume() which then would do nothing.  The end
> result is that I would have a prepare count of 1 on aclk_nvm_root, but
> an enable count of 0 so it would get disabled (orig patch w/o marking
> this critical).
> 
> I did a quick hacky proof of concept where I set up the pm ops on the
> clk-rk3588 driver to point to the pm_clk_suspend/pm_clk_resume routines
> like downstream did.  I used device_bind_driver() to forceably bind the
> clk-rk3588 driver to the gate link devices it was setting up.
> 
> This worked for me.  With those changes I then was able to boot without
> needing to set aclk_nvm_root as critical.  Further, it cleaned up the
> power-controller error messages on the sdio and vo0 domains.
> 
> The force bind I did doesn't feel like a particularly clean solution to
> me.  However, I am not knowledgeable enough in the (platform) device
> model to know what the right way to do this is without fully doing what
> downstream did and having DT nodes for the various gate link devices.
> But it seems to prove out that having the driver bound to the devices
> with the relevant runtime pm ops is the missing piece in my use case on
> the OPi5.

Can you test again using rk3588-test [0] (which is based on today's
master branch) from today as base? Ideally it should work with your
Orange Pi 5, since it contains "clk: rockchip: rk3588: mark hclk_nvm
critical".

I also worked on a cleaner solution for the issue you described and
integrated it in the patch adding proper gate clock support. So
please also test with HCLK_NVM not being marked as CRITICAL.

[0] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commits/rk3588-test

If everything is fine I will prepare a v9.

> Hope this additional data point helps!

Thanks for the detailed analysis.

Greetings,

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20240320/3e8d5e40/attachment.sig>