Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Fri, 1 Dec 2023 02:05:15 -0600
From:   Nishanth Menon <nm@ti.com>
To:     Thomas Richard <thomas.richard@bootlin.com>
CC:     Tero Kristo <kristo@kernel.org>,
        Santosh Shilimkar <ssantosh@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <thomas.petazzoni@bootlin.com>, <gregory.clement@bootlin.com>,
        <u-kumar1@ti.com>, <khilman@baylibre.com>, <vibhore@ti.com>,
        <vishalm@ti.com>, <d-gole@ti.com>
Subject: Re: [PATCH 2/5] firmware: ti_sci: add suspend/resume support for irqs
Message-ID: <20231201080515.6g4wvjvays7ik647@passport>
References: <20231129-j7200-tisci-s2r-v1-0-c1d5964ed574@bootlin.com>
 <20231129-j7200-tisci-s2r-v1-2-c1d5964ed574@bootlin.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231129-j7200-tisci-s2r-v1-2-c1d5964ed574@bootlin.com>
Precedence: bulk

On 16:31-20231129, Thomas Richard wrote:
> On j7200, during suspend to ram, the SoC is powered-off.

A53  and main domain is powered off in the case of J7200 - there are
different nuanced suspend-to-ram scenarios involved which the system
control firmware manages for us (Linux).

> The irqs configuration is lost, so it shall be restored at resume.
> The ti-sci has an internal list of irqs updated at each set/free.
> All irqs in this list are restored at resume.

This I don't disagree - but apply to all K3 SoCs. But what I do disagree
is that this is sufficient enough to have suspend-to-ram actually
functional. there are additional handshakes that are needed and the
driver just has a single suspend-resume handler..

git log --oneline drivers/firmware/ti_sci.c tells us that we have
already had a bunch of introduction, revert of suspend-resume handler
(too much for my comfort). I wish I didn't have to say this, but I
guess having seen TFA[1], U-boot[2] series (not that the kernel patch
discussion is independent of TFA/U-boot discussions), and having known
that the AM62x team is also working towards a suspend-resume scheme
(CCying Kevin and AM62x folks) - Since we all share a single driver,
I suggest we get together and draw through the arch of how this flow
will look like and not create additional confusion in the public list
on this till we have a settled down view (also the reason I haven't
picked up am62x patches so far either).

For now, I am going to punt this series until I see clarity overall
and I am assured that we have a relatively final path for all users of
TISCI driver/lib/infra.


[1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/23992/7
[2] https://lore.kernel.org/u-boot/20231109140738.GF6601@bill-the-cat/

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D