Linux Input/HID development
 help / color / mirror / Atom feed
From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Quentin Schulz <quentin.schulz@cherry.de>
Cc: Jakob Unterwurzacher <jakobunt@gmail.com>,
	 Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Joseph Chen <chenjh@rock-chips.com>, Lee Jones <lee@kernel.org>,
	 Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	 Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	 linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	 linux-input@vger.kernel.org, mfd@lists.linux.dev,
	devicetree@vger.kernel.org,
	 Jakob Unterwurzacher <jakob.unterwurzacher@cherry.de>
Subject: Re: [PATCH 0/4] Suspend-to-RAM fixes for RK3588 + SPI-connected RK806 PMIC, enable PMIC sleep on RK3588 Tiger
Date: Thu, 12 Nov 2026 01:03:59 +0100	[thread overview]
Message-ID: <avT_xrrgeThffbpc@venus> (raw)
In-Reply-To: <a220ebd0-a28d-44cc-9a80-62382018348c@cherry.de>

[-- Attachment #1: Type: text/plain, Size: 5509 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply	other threads:[~2026-11-12  0:04 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 10:06 [PATCH 0/4] Suspend-to-RAM fixes for RK3588 + SPI-connected RK806 PMIC, enable PMIC sleep on RK3588 Tiger Jakob Unterwurzacher
2026-10-29 10:06 ` [PATCH 1/4] Input: rk805-pwrkey - enable the wakeup interrupt during system suspend Jakob Unterwurzacher
2026-11-12  0:25   ` Dmitry Torokhov
2026-10-29 10:06 ` [PATCH 2/4] mfd: rk8xx-core: keep the PMIC interrupt disabled " Jakob Unterwurzacher
2026-10-29 10:21   ` sashiko-bot
2026-10-29 10:06 ` [PATCH 3/4] mfd: rk8xx-spi: add system sleep PM ops Jakob Unterwurzacher
2026-10-29 10:06 ` [PATCH 4/4] arm64: dts: rockchip: put the PMIC into sleep mode in suspend on RK3588 Tiger Jakob Unterwurzacher
2026-10-29 19:31 ` [PATCH 0/4] Suspend-to-RAM fixes for RK3588 + SPI-connected RK806 PMIC, enable PMIC sleep " Quentin Schulz
2026-11-12  0:03   ` Sebastian Reichel [this message]

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=avT_xrrgeThffbpc@venus \
    --to=sebastian.reichel@collabora.com \
    --cc=chenjh@rock-chips.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=heiko@sntech.de \
    --cc=jakob.unterwurzacher@cherry.de \
    --cc=jakobunt@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mfd@lists.linux.dev \
    --cc=quentin.schulz@cherry.de \
    --cc=robh@kernel.org \
    /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