Re: SPI transfers in atomic context [Was: Re: [PATCH v1 1/1] mfd: rk8xx: Fix shutdown handler]

From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
To: "Uwe Kleine-König" <ukleinek@debian.org>,
	"Lee Jones" <lee@kernel.org>,
	"Sebastian Reichel" <sebastian.reichel@collabora.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Wolfram Sang" <wsa@the-dreams.de>
Cc: Urja <urja@urja.dev>, Heiko Stuebner <heiko@sntech.de>,
	linux-rockchip@lists.infradead.org, linux-spi@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel@collabora.com,
	stable@vger.kernel.org
Subject: Re: SPI transfers in atomic context [Was: Re: [PATCH v1 1/1] mfd: rk8xx: Fix shutdown handler]
Date: Fri, 28 Mar 2025 08:27:01 +0300	[thread overview]
Message-ID: <f9be4614-95ec-4b63-9cfd-0936a323b131@collabora.com> (raw)
In-Reply-To: <sg5kgo5qjqyzfyk5nyjbkpgvbx6sfb7agc67ch6wsdq3etrsbf@h6xbtfs45k4w>

On 3/20/25 13:10, Uwe Kleine-König wrote:
> Hi,
> 
> On Thu, Aug 01, 2024 at 05:22:24PM +0200, Sebastian Reichel wrote:
>> On Thu, Aug 01, 2024 at 02:18:23PM GMT, Lee Jones wrote:
>>>> +	/*
>>>> +	 * Currently the Rockchip SPI driver always sleeps when doing SPI
>>>> +	 * transfers. This is not allowed in the SYS_OFF_MODE_POWER_OFF
>>>> +	 * handler, so we are using the prepare handler as a workaround.
>>>> +	 * This should be removed once the Rockchip SPI driver has been
>>>> +	 * adapted.
>>>> +	 */
>>>
>>> So why not just adapt the SPI driver now?
>>
>> This patch is simple and thus can easily be backported, so that the
>> Acer Chromebook shutdown is fixed in the stable kernels. SPI based
>> rkxx has been using SYS_OFF_MODE_POWER_OFF_PREPARE from the start,
>> so it's not a regression.
>>
>> As far as I could see the SPI framework does not have something
>> comparable to the I2C .xfer_atomic handler. So fixing up the
>> Rockchip SPI driver probably involves creating some SPI core
>> helpers. I'm not yet sure about the best way to deal with this.
>> But I guess it will be better not having to backport all of the
>> requires changes to stable.
>>
>> In any case I think the next step in this direction is discussing
>> how to handle this in general for SPI.
>>
>>> What's the bet that if accepted, this hack is still here in 5 years time?
>>
>> Even if I don't work on this now, I would expect somebody to have
>> issues with broken shutdown on RK3588 boards before 5 years are
>> over :)
> 
> I'd like to have power-off working on Qnap TS-433 in the next Debian
> stable. With my Debian Kernel hat on I'd say cherry-picking such a
> commit (if it's in mainline) is acceptable. Backporting a major
> extension to the spi framework isn't.
> 
> So: Expectation confirmed! And while I agree that hacks are not nice,
> I prefer a hack now over a machine that doesn't shut down properly over
> the next five years (if Lee's expectation is also correct).
> 
> Can we maybe go forward and do both? Accept this hack patch now and work
> on spi to make atomic xfers possible?
> 
> Mark, are there concerns from your side? 
> Wolfram, are there things you would recommend to do differently in spi
> than what you have in i2c?

Hi, want let you know that I've started to work recently on atomic SPI
transfer support to have SPI shutdown working properly with this driver.
It's in progress.

Meanwhile this patch should've been merged a year ago because it fixes
the regression.

Lee, please apply it for -stable.

-- 
Best regards,
Dmitry

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip