linux-kernel - Re: [PATCH 2/2] remoteproc: imx_dsp_rproc: Wait for suspend ACK only if WAIT_FW

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6a6e90fb-c8be-49ad-83b4-4b602489a616@nxp.com>
Date: Wed, 3 Dec 2025 20:09:10 +0200
From: Iuliana Prodan <iuliana.prodan@....com>
To: Bjorn Andersson <andersson@...nel.org>,
 "Iuliana Prodan (OSS)" <iuliana.prodan@....nxp.com>
Cc: Mathieu Poirier <mathieu.poirier@...aro.org>,
 Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
 "S.J. Wang" <shengjiu.wang@....com>, Fabio Estevam <festevam@...il.com>,
 Daniel Baluta <daniel.baluta@....com>, imx@...ts.linux.dev,
 linux-remoteproc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Pengutronix Kernel Team <kernel@...gutronix.de>
Subject: Re: [PATCH 2/2] remoteproc: imx_dsp_rproc: Wait for suspend ACK only
 if WAIT_FW_READY is set

On 11/26/2025 10:21 PM, Bjorn Andersson wrote:
> On Tue, Nov 25, 2025 at 02:49:03PM +0200, Iuliana Prodan (OSS) wrote:
>> From: Iuliana Prodan <iuliana.prodan@....com>
>>
>> The DSP suspend path currently waits unconditionally
>> for a suspend ack from the firmware.
>> This breaks firmwares that do not implement the
>> mailbox-based READY handshake, as the DSP never
>> responds and system suspend fails with -EBUSY.
>>
> 
> But if the firmware doesn't implement "mailbox-based READY handshake",
> do you still want to send the RP_MBOX_SUSPEND_SYSTEM message?
> 
If FEATURE_DONT_WAIT_FW_READY is set, it means the remote does not send 
any feedback to the host and does not handle specific messages. In this 
case, it’s better not to send the message at all.

BTW, to avoid confusion with FW_READY macros, I’ll add a new patch to 
rename these macros to FW_CONFIRMATION, since they are used both for 
boot confirmation and other firmware acknowledgments.

> If so, can you clarify here in the commit message that the firmware
> expects the mailbox-based message, and only the "handshake" part should
> be omitted.
> 
> If that part isn't implemented either, then I think you should fix the
> code to not poke the mailbox in the first place.
> 
Yes, will fix in v2.

> 
> Also, wrap your commit message at 75 characters, please.
> 
>> The driver already uses the WAIT_FW_READY flag to
>> indicate that the firmware supports the READY
>> handshake at boot. Apply the same logic during
>> suspend: only wait for the suspend ack when the
>> firmware is expected to support it.
>>
>> Signed-off-by: Iuliana Prodan <iuliana.prodan@....com>
>> ---
>>   drivers/remoteproc/imx_dsp_rproc.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
>> index fc0470aa72c1..e25dbe32ef79 100644
>> --- a/drivers/remoteproc/imx_dsp_rproc.c
>> +++ b/drivers/remoteproc/imx_dsp_rproc.c
>> @@ -1327,10 +1327,11 @@ static int imx_dsp_suspend(struct device *dev)
>>   	}
>>   
>>   	/*
>> -	 * DSP need to save the context at suspend.
>> -	 * Here waiting the response for DSP, then power can be disabled.
>> +	 * The DSP must save its context during suspend.
> 
> Please double check that this comment reflect above conclusion.
> 
This is valid for some firmware, like Xtensa Audio Firmware (XAF), where 
we use both firmware-ready replay and the messages are handled on the 
remote side, where the DSP context is saved.

> Regards,
> Bjorn
> 
>> +	 * Wait for a response from the DSP if required before disabling power.
>>   	 */
>> -	if (!wait_for_completion_timeout(&priv->pm_comp, msecs_to_jiffies(100)))
>> +	if (priv->flags & WAIT_FW_READY &&
>> +	    !wait_for_completion_timeout(&priv->pm_comp, msecs_to_jiffies(100)))
>>   		return -EBUSY;
>>   
>>   out:
>> -- 
>> 2.34.1
>>