mailbox: omap-mailbox: Flush out pending msgs before entering suspend

[RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend

Posted by Beleswar Padhi 3 months, 2 weeks ago

There may be pending messages in the mailbox FIFO that are not consumed
by the remote processor for various reasons; the remote processor may
already be powered off or may be in a bad state. Instead of aborting
suspend because of these pending messages, flush the FIFOs and proceed
with suspend. Pending messages could also be restored in the resume
context, but since remote processors are typically rebooted during
suspend/resume today, there is no point in restoring stale messages.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
 drivers/mailbox/omap-mailbox.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 680243751d62..5e6373911630 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -341,13 +341,10 @@ static int omap_mbox_suspend(struct device *dev)
 	if (pm_runtime_status_suspended(dev))
 		return 0;
 
-	for (fifo = 0; fifo < mdev->num_fifos; fifo++) {
-		if (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo))) {
-			dev_err(mdev->dev, "fifo %d has unexpected unread messages\n",
-				fifo);
-			return -EBUSY;
-		}
-	}
+	/* Flush out pending mbox messages before entering suspend */
+	for (fifo = 0; fifo < mdev->num_fifos; fifo++)
+		while (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo)) != 0)
+			mbox_read_reg(mdev, MAILBOX_MESSAGE(fifo));
 
 	for (usr = 0; usr < mdev->num_users; usr++) {
 		reg = MAILBOX_IRQENABLE(mdev->intr_type, usr);
-- 
2.34.1

Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend

Posted by Andrew Davis 3 months, 2 weeks ago

On 10/22/25 5:20 AM, Beleswar Padhi wrote:
> There may be pending messages in the mailbox FIFO that are not consumed
> by the remote processor for various reasons; the remote processor may
> already be powered off or may be in a bad state. Instead of aborting
> suspend because of these pending messages, flush the FIFOs and proceed
> with suspend. Pending messages could also be restored in the resume
> context, but since remote processors are typically rebooted during
> suspend/resume today, there is no point in restoring stale messages.
> 
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> ---
>   drivers/mailbox/omap-mailbox.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
> index 680243751d62..5e6373911630 100644
> --- a/drivers/mailbox/omap-mailbox.c
> +++ b/drivers/mailbox/omap-mailbox.c
> @@ -341,13 +341,10 @@ static int omap_mbox_suspend(struct device *dev)
>   	if (pm_runtime_status_suspended(dev))
>   		return 0;
>   
> -	for (fifo = 0; fifo < mdev->num_fifos; fifo++) {
> -		if (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo))) {
> -			dev_err(mdev->dev, "fifo %d has unexpected unread messages\n",
> -				fifo);
> -			return -EBUSY;
> -		}
> -	}
> +	/* Flush out pending mbox messages before entering suspend */
> +	for (fifo = 0; fifo < mdev->num_fifos; fifo++)
> +		while (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo)) != 0)
> +			mbox_read_reg(mdev, MAILBOX_MESSAGE(fifo));

I'm still not convinced just throwing out messages is the correct thing
to do here, but for now at very least let's print some warning here when
messages get zapped.

Andrew

>   
>   	for (usr = 0; usr < mdev->num_users; usr++) {
>   		reg = MAILBOX_IRQENABLE(mdev->intr_type, usr);

Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend

Posted by Beleswar Prasad Padhi 3 months, 2 weeks ago

Hi Andrew,

On 22/10/25 21:37, Andrew Davis wrote:
> On 10/22/25 5:20 AM, Beleswar Padhi wrote:
>> There may be pending messages in the mailbox FIFO that are not consumed
>> by the remote processor for various reasons; the remote processor may
>> already be powered off or may be in a bad state. Instead of aborting
>> suspend because of these pending messages, flush the FIFOs and proceed
>> with suspend. Pending messages could also be restored in the resume
>> context, but since remote processors are typically rebooted during
>> suspend/resume today, there is no point in restoring stale messages.
>>
>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>> ---
>>   drivers/mailbox/omap-mailbox.c | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
>> index 680243751d62..5e6373911630 100644
>> --- a/drivers/mailbox/omap-mailbox.c
>> +++ b/drivers/mailbox/omap-mailbox.c
>> @@ -341,13 +341,10 @@ static int omap_mbox_suspend(struct device *dev)
>>       if (pm_runtime_status_suspended(dev))
>>           return 0;
>>   -    for (fifo = 0; fifo < mdev->num_fifos; fifo++) {
>> -        if (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo))) {
>> -            dev_err(mdev->dev, "fifo %d has unexpected unread messages\n",
>> -                fifo);
>> -            return -EBUSY;
>> -        }
>> -    }
>> +    /* Flush out pending mbox messages before entering suspend */
>> +    for (fifo = 0; fifo < mdev->num_fifos; fifo++)
>> +        while (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo)) != 0)
>> +            mbox_read_reg(mdev, MAILBOX_MESSAGE(fifo));
>
> I'm still not convinced just throwing out messages is the correct thing
> to do here, but for now at very least let's print some warning here when
> messages get zapped.


I am also considering:
1. Mailbox queues for RTOS-RTOS communication could be
firewalled for safety/FFI usecases. In that case, the above flush
would result in an exception.
2. This driver is common to OMAP SoCs which supported proper
suspend/resume, meaning those rprocs could consume pending
messages in resume. So clearing out messages from Linux
might not be the best thing to do here.

What else can we do here? Should we just fallback to letting
Linux see only it's required queues for IPC? By setting
"ti,mbox-num-fifos = <4>"?

Thanks,
Beleswar

>
> Andrew
>
>>         for (usr = 0; usr < mdev->num_users; usr++) {
>>           reg = MAILBOX_IRQENABLE(mdev->intr_type, usr);
>

Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend

Posted by Hari Nagalla 3 months, 2 weeks ago

On 10/22/25 23:38, Beleswar Prasad Padhi wrote:
>> I'm still not convinced just throwing out messages is the correct thing
>> to do here, but for now at very least let's print some warning here when
>> messages get zapped.
> 
> I am also considering:
> 1. Mailbox queues for RTOS-RTOS communication could be
> firewalled for safety/FFI usecases. In that case, the above flush
> would result in an exception.
Yes, flushing all mailbox messages during suspend is not a good 
solution, as there can be in flight RTOS-RTOS communications for non 
participating cores.
> 2. This driver is common to OMAP SoCs which supported proper
> suspend/resume, meaning those rprocs could consume pending
> messages in resume. So clearing out messages from Linux
> might not be the best thing to do here.
> 
> What else can we do here? Should we just fallback to letting
> Linux see only it's required queues for IPC? By setting
> "ti,mbox-num-fifos = <4>"?
This makes the assumption that the first 4 FIFOs of the mailbox are 
used? and why 4? are you assuming first 2 for Linux IPC and next 2 for 
RTOS-RTOS communications?

How about, let the mailbox driver simply save and restore the config 
registers and ignore the FIFO messages? i.e if they are lost with the 
main domain going into OFF, so be it. Let the remote cores handle any 
missing messages. More often the remote cores may start afresh after a 
suspend to RAM.

Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend

Posted by Hiago De Franco 3 months, 2 weeks ago

On 22.10.2025 15:50, Beleswar Padhi wrote:
> There may be pending messages in the mailbox FIFO that are not consumed
> by the remote processor for various reasons; the remote processor may
> already be powered off or may be in a bad state. Instead of aborting
> suspend because of these pending messages, flush the FIFOs and proceed
> with suspend. Pending messages could also be restored in the resume
> context, but since remote processors are typically rebooted during
> suspend/resume today, there is no point in restoring stale messages.
>
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>

Thanks for your patch, I tested it on Verdin AM62 and it is working
fine.

Please add the following tags:

Closes: https://lore.kernel.org/all/sid7gtg5vay5qgicsl6smnzwg5mnneoa35cempt5ddwjvedaio@hzsgcx6oo74l/
Fixes: 1d6161617c10 ("arm64: dts: ti: k3-am62-ti-ipc-firmware:
Refactor IPC cfg into new dtsi")

With that,

Tested-by: Hiago De Franco <hiago.franco@toradex.com> # Verdin AM62

Best regards,
Hiago.

> ---
>  drivers/mailbox/omap-mailbox.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
> index 680243751d62..5e6373911630 100644
> --- a/drivers/mailbox/omap-mailbox.c
> +++ b/drivers/mailbox/omap-mailbox.c
> @@ -341,13 +341,10 @@ static int omap_mbox_suspend(struct device *dev)
>      if (pm_runtime_status_suspended(dev))
>          return 0;
>
> -    for (fifo = 0; fifo < mdev->num_fifos; fifo++) {
> -        if (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo))) {
> -            dev_err(mdev->dev, "fifo %d has unexpected unread messages\n",
> -                fifo);
> -            return -EBUSY;
> -        }
> -    }
> +    /* Flush out pending mbox messages before entering suspend */
> +    for (fifo = 0; fifo < mdev->num_fifos; fifo++)
> +        while (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo)) != 0)
> +            mbox_read_reg(mdev, MAILBOX_MESSAGE(fifo));
>
>      for (usr = 0; usr < mdev->num_users; usr++) {
>          reg = MAILBOX_IRQENABLE(mdev->intr_type, usr);
> --
> 2.34.1
>

On Wed, Oct 22, 2025 at 7:20 AM Beleswar Padhi <b-padhi@ti.com> wrote:
>
> There may be pending messages in the mailbox FIFO that are not consumed
> by the remote processor for various reasons; the remote processor may
> already be powered off or may be in a bad state. Instead of aborting
> suspend because of these pending messages, flush the FIFOs and proceed
> with suspend. Pending messages could also be restored in the resume
> context, but since remote processors are typically rebooted during
> suspend/resume today, there is no point in restoring stale messages.
>
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> ---
>  drivers/mailbox/omap-mailbox.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
> index 680243751d62..5e6373911630 100644
> --- a/drivers/mailbox/omap-mailbox.c
> +++ b/drivers/mailbox/omap-mailbox.c
> @@ -341,13 +341,10 @@ static int omap_mbox_suspend(struct device *dev)
>         if (pm_runtime_status_suspended(dev))
>                 return 0;
>
> -       for (fifo = 0; fifo < mdev->num_fifos; fifo++) {
> -               if (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo))) {
> -                       dev_err(mdev->dev, "fifo %d has unexpected unread messages\n",
> -                               fifo);
> -                       return -EBUSY;
> -               }
> -       }
> +       /* Flush out pending mbox messages before entering suspend */
> +       for (fifo = 0; fifo < mdev->num_fifos; fifo++)
> +               while (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo)) != 0)
> +                       mbox_read_reg(mdev, MAILBOX_MESSAGE(fifo));
>
>         for (usr = 0; usr < mdev->num_users; usr++) {
>                 reg = MAILBOX_IRQENABLE(mdev->intr_type, usr);
> --
> 2.34.1
>