Linux Input/HID development
 help / color / mirror / Atom feed
From: Quentin Schulz <quentin.schulz@cherry.de>
To: Jakob Unterwurzacher <jakobunt@gmail.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Joseph Chen <chenjh@rock-chips.com>, Lee Jones <lee@kernel.org>,
	Sebastian Reichel <sebastian.reichel@collabora.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>
Cc: 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, 29 Oct 2026 20:31:34 +0100	[thread overview]
Message-ID: <a220ebd0-a28d-44cc-9a80-62382018348c@cherry.de> (raw)
In-Reply-To: <20261029-dev-tiger-v7-4-pre-20261028-tiger-suspend-b4-v1-0-cb12621c1fb4@cherry.de>

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.

> 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.

> 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.
> 
> [1] https://lore.kernel.org/linux-phy/20261001-phy-rockchip-inno-usb2-clock-fix-v2-4-e01bf509359e@collabora.com/
> 
> 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/

> 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].

[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

      parent reply	other threads:[~2026-10-29 19:31 UTC|newest]

Thread overview: 7+ 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-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 ` Quentin Schulz [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=a220ebd0-a28d-44cc-9a80-62382018348c@cherry.de \
    --to=quentin.schulz@cherry.de \
    --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=robh@kernel.org \
    --cc=sebastian.reichel@collabora.com \
    /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