Hi,

On Thu, Oct 29, 2026 at 08:31:34PM +0100, Quentin Schulz wrote:
> Hi Jakob,
> 
> On 10/29/26 11:06 AM, Jakob Unterwurzacher wrote:
> > This series, together with a few out-of-tree patches (see below), makes
> > suspend-to-RAM on RK3588-Tiger work, down to a power consumption of
> > about 0.4 W. Patches 1-3 apply to all boards with an RK806 PMIC
> > connected via SPI, which includes most RK3588 boards.
> > 
> > Patch 1 ("Input: rk805-pwrkey - enable the wakeup interrupt during system suspend")
> > arms the power key interrupt as a wakeup source, so that the
> > power key can wake up the system. This is done in the regular suspend
> > phase, because the SPI bus to the PMIC can no longer be used in the
> > noirq phase.
> > 
> > Patch 2 ("mfd: rk8xx-core: keep the PMIC interrupt disabled during system suspend")
> > keeps the PMIC interrupt disabled across system suspend, so that its
> > handler does not run before the bus to the PMIC has resumed.
> > 
> > Patch 3 ("mfd: rk8xx-spi: add system sleep PM ops")
> > hooks up the existing rk8xx suspend/resume callbacks for the SPI
> > variant, which so far lacked them.
> > 
> > Patch 4 ("arm64: dts: rockchip: put the PMIC into sleep mode in suspend on RK3588 Tiger")
> > configures the RK806 PWRCTRL1 pin on RK3588 Tiger for its sleep
> > function, so that the regulator suspend states take effect. This lowers
> > the module's power consumption in suspend-to-RAM from 1.09 W to 0.43 W.
> > 
> > The patches do not depend on each other and can be applied through
> > their respective trees.
> > 
> 
> Then have them in separate series. Series should be as small as possible,
> down to a single commit if possible.

I suggest to send patch 2+3 as one series considering patch 3 does
not make much sense without patch 2.

> > Testing
> > -------
> > 
> > Tested on RK3588-Tiger on the Haikou baseboard, on top of the v7.4 merge
> > window (a8530b69fc4dd): 30 consecutive suspend-to-RAM cycles, each woken
> > up by the PMIC power key, all resumed without errors in the kernel log.
> > USB devices (a SuperSpeed hub with an Ethernet adapter, and its
> > High-Speed part) stayed connected across suspend.
> > 
> > Out-of-tree patches
> > -------------------
> > 
> > To test this series, out-of-tree patches were needed and used:
> > 
> > 1) Upstream TF-A does not complete suspend-to-RAM on this board yet, so the
> > tests used Rockchip's BL31 (rkbin v1.47), configured through a small
> > out-of-tree driver. The plan is to get suspend working in TF-A instead.
> > 
> 
> I'm guessing you(r LLM) meant upstream TF-A here? Or what is it that you
> mean?
> 
> > 2) The inno-usb2 PHY has to be resumed when its 480 MHz clock is enabled,
> > otherwise EHCI/OHCI resume hangs. This is addressed by Sebastian
> > Reichel's inno-usb2 series [1].
> 
> Having access to your downstream branch, you have a patch generated by your
> LLM and not Sebastian's patch series. Either use what Sebastian posted, or
> be more careful in your choice of words as I read the above as implying you
> used his series.
>
> > [1] https://lore.kernel.org/linux-phy/20261001-phy-rockchip-inno-usb2-clock-fix-v2-4-e01bf509359e@collabora.com/

Getting a Tested-by on v5, which finally tamed Sashiko would be
nice:

https://lore.kernel.org/linux-phy/20261110-phy-rockchip-inno-usb2-clock-fix-v5-0-d2e24582ab22@collabora.com/

> > 3) The GPU and NPU domain supplies were kept always-on. genpd powers
> > these domains on in the noirq resume phase, when their regulators (on
> > I2C/SPI) cannot be reached yet. I'm not sure what to do here, yet.
>
> > Known issues
> > ------------
> > 
> > PCIe devices do not survive resume; this depends on system PM
> > support in pcie-dw-rockchip [2].
> > 
> > [2] https://lore.kernel.org/r/20260316-rockchip-pcie-system-suspend-v5-0-5bb5ad37d643@collabora.com
> > 
> 
> There already is a v6 from almost two months ago. Did you actually test with
> the v5 or are you just hinting at what could be helping resume PCIe devices?
> 
> I'm sure Sebastian would be happy to receive a nice Tested-by tag if you did
> indeed test that. Link to v6: https://lore.kernel.org/all/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com/

FWIW v9 got merged:

https://lore.kernel.org/linux-pci/179407115382.87267.12160393716568806842.b4-ty@b4/

> > Tool use
> > --------
> > 
> > These patches were developed with an LLM-based coding assistant (Claude
> > Code). It ran the suspend/resume tests on the hardware, analysed the
> > kernel logs to find the causes of the failures, and drafted the code
> > changes and commit messages. I reviewed all changes and tested them as
> > described above.
> > 
> 
> All your patches are missing the Signed-off-by, required to signify DCO[1].
> LLMs are forbidden to add it, so I guess you missed that when reviewing the
> patches before sending. Please triple check that you agree to all criteria
> listed in the documentation before adding the SoB to patches generated by
> LLMs[2].

I had a look at the patches and they all look sensible to me, but
not much worth without the SoB.

Greetings,

-- Sebastian

> [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
> [2] https://docs.kernel.org/process/coding-assistants.html#signed-off-by-and-developer-certificate-of-origin
> 
> Cheers,
> Quentin