* [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend @ 2025-10-22 10:20 Beleswar Padhi 2025-10-22 13:45 ` Hiago De Franco 2025-10-22 16:07 ` Andrew Davis 0 siblings, 2 replies; 9+ messages in thread From: Beleswar Padhi @ 2025-10-22 10:20 UTC (permalink / raw) To: jassisinghbrar, linux-kernel Cc: hiagofranco, afd, hnagalla, u-kumar1, b-padhi 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend 2025-10-22 10:20 [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend Beleswar Padhi @ 2025-10-22 13:45 ` Hiago De Franco 2025-10-22 16:07 ` Andrew Davis 1 sibling, 0 replies; 9+ messages in thread From: Hiago De Franco @ 2025-10-22 13:45 UTC (permalink / raw) To: Beleswar Padhi Cc: jassisinghbrar, linux-kernel, afd, hnagalla, u-kumar1, Hiago De Franco 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend 2025-10-22 10:20 [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend Beleswar Padhi 2025-10-22 13:45 ` Hiago De Franco @ 2025-10-22 16:07 ` Andrew Davis 2025-10-23 4:38 ` Beleswar Prasad Padhi 1 sibling, 1 reply; 9+ messages in thread From: Andrew Davis @ 2025-10-22 16:07 UTC (permalink / raw) To: Beleswar Padhi, jassisinghbrar, linux-kernel Cc: hiagofranco, hnagalla, u-kumar1 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); ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend 2025-10-22 16:07 ` Andrew Davis @ 2025-10-23 4:38 ` Beleswar Prasad Padhi 2025-10-23 17:18 ` Hari Nagalla 0 siblings, 1 reply; 9+ messages in thread From: Beleswar Prasad Padhi @ 2025-10-23 4:38 UTC (permalink / raw) To: Andrew Davis, jassisinghbrar, linux-kernel Cc: hiagofranco, hnagalla, u-kumar1 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); > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend 2025-10-23 4:38 ` Beleswar Prasad Padhi @ 2025-10-23 17:18 ` Hari Nagalla 2025-10-24 4:48 ` Beleswar Prasad Padhi 0 siblings, 1 reply; 9+ messages in thread From: Hari Nagalla @ 2025-10-23 17:18 UTC (permalink / raw) To: Beleswar Prasad Padhi, Andrew Davis, jassisinghbrar, linux-kernel Cc: hiagofranco, u-kumar1 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend 2025-10-23 17:18 ` Hari Nagalla @ 2025-10-24 4:48 ` Beleswar Prasad Padhi 2025-10-24 14:08 ` Andrew Davis 0 siblings, 1 reply; 9+ messages in thread From: Beleswar Prasad Padhi @ 2025-10-24 4:48 UTC (permalink / raw) To: Hari Nagalla, Andrew Davis, jassisinghbrar, linux-kernel Cc: hiagofranco, u-kumar1 Hi Hari, On 23/10/25 22:48, Hari Nagalla wrote: > 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? 4 is just an example. The mailbox FIFO assignment is also a firmware configuration. So the idea was to let Linux only care about the FIFOs it uses for Linux<->RTOS communication, ignoring the ones used for RTOS<->RTOS communication. > > 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. Hmm, this was how it was until 2016 when commit 9f0cee984a25 ("mailbox/omap: check for any unread messages during suspend") was added. Do you suggest reverting 9f0cee984a25? Hope it doesn't break any existing usecases for OMAP platforms. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend 2025-10-24 4:48 ` Beleswar Prasad Padhi @ 2025-10-24 14:08 ` Andrew Davis 2025-10-30 8:59 ` Francesco Dolcini 0 siblings, 1 reply; 9+ messages in thread From: Andrew Davis @ 2025-10-24 14:08 UTC (permalink / raw) To: Beleswar Prasad Padhi, Hari Nagalla, jassisinghbrar, linux-kernel Cc: hiagofranco, u-kumar1 On 10/23/25 11:48 PM, Beleswar Prasad Padhi wrote: > Hi Hari, > > On 23/10/25 22:48, Hari Nagalla wrote: >> 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? > > > 4 is just an example. The mailbox FIFO assignment is also > a firmware configuration. So the idea was to let Linux only > care about the FIFOs it uses for Linux<->RTOS > communication, ignoring the ones used for > RTOS<->RTOS communication. > The issue is Linux is still shutting down the whole mailbox, so it can't just care about it's own FIFOs as it is still deleting all stored messages, whether it checks them or not. >> >> 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. > > > Hmm, this was how it was until 2016 when commit > 9f0cee984a25 ("mailbox/omap: check for any unread > messages during suspend") was added. Do you > suggest reverting 9f0cee984a25? Hope it doesn't > break any existing usecases for OMAP platforms. > Checking for unread messages during suspend is still correct, what we then do when we find them is the open question. We can: * Not suspend (current situation) * Suspend anyway, deleting them but with a warning * Save and restore them Ignoring the problem by reverting 9f0cee984a25 is not a solution. Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend 2025-10-24 14:08 ` Andrew Davis @ 2025-10-30 8:59 ` Francesco Dolcini 2025-10-30 9:12 ` Beleswar Prasad Padhi 0 siblings, 1 reply; 9+ messages in thread From: Francesco Dolcini @ 2025-10-30 8:59 UTC (permalink / raw) To: Andrew Davis Cc: Beleswar Prasad Padhi, Hari Nagalla, jassisinghbrar, linux-kernel, hiagofranco, u-kumar1 On Fri, Oct 24, 2025 at 09:08:09AM -0500, Andrew Davis wrote: > On 10/23/25 11:48 PM, Beleswar Prasad Padhi wrote: > > Hi Hari, > > > > On 23/10/25 22:48, Hari Nagalla wrote: > > > On 10/22/25 23:38, Beleswar Prasad Padhi wrote: > > Hmm, this was how it was until 2016 when commit > > 9f0cee984a25 ("mailbox/omap: check for any unread > > messages during suspend") was added. Do you > > suggest reverting 9f0cee984a25? Hope it doesn't > > break any existing usecases for OMAP platforms. > > Checking for unread messages during suspend is still correct, > what we then do when we find them is the open question. We can: > > * Not suspend (current situation) > * Suspend anyway, deleting them but with a warning I would propose that we improve the current patch and just add a dev_warn when there are unread messages. Andrew? > * Save and restore them As of now our platforms (Toradex) are broken because of the changes in this series https://lore.kernel.org/all/20250908142826.1828676-1-b-padhi@ti.com/ (now in mainline), and I would love to get it fixed sooner than later. Francesco ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend 2025-10-30 8:59 ` Francesco Dolcini @ 2025-10-30 9:12 ` Beleswar Prasad Padhi 0 siblings, 0 replies; 9+ messages in thread From: Beleswar Prasad Padhi @ 2025-10-30 9:12 UTC (permalink / raw) To: Francesco Dolcini, Andrew Davis Cc: Hari Nagalla, jassisinghbrar, linux-kernel, hiagofranco, u-kumar1 On 30/10/25 14:29, Francesco Dolcini wrote: > On Fri, Oct 24, 2025 at 09:08:09AM -0500, Andrew Davis wrote: >> On 10/23/25 11:48 PM, Beleswar Prasad Padhi wrote: >>> Hi Hari, >>> >>> On 23/10/25 22:48, Hari Nagalla wrote: >>>> On 10/22/25 23:38, Beleswar Prasad Padhi wrote: >>> Hmm, this was how it was until 2016 when commit >>> 9f0cee984a25 ("mailbox/omap: check for any unread >>> messages during suspend") was added. Do you >>> suggest reverting 9f0cee984a25? Hope it doesn't >>> break any existing usecases for OMAP platforms. >> Checking for unread messages during suspend is still correct, >> what we then do when we find them is the open question. We can: >> >> * Not suspend (current situation) >> * Suspend anyway, deleting them but with a warning > I would propose that we improve the current patch and just add a > dev_warn when there are unread messages. For systems which have FIFOs firewalled, this flushing will result in exception. So we can't go with this. > > Andrew? > >> * Save and restore them > As of now our platforms (Toradex) are broken because of the changes in > this series https://lore.kernel.org/all/20250908142826.1828676-1-b-padhi@ti.com/ > (now in mainline), and I would love to get it fixed sooner than later. I've been working on this, and will send a DT patch today to limit the num-fifos for each cluster. Thanks, Beleswar ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-10-30 9:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-22 10:20 [RFC PATCH] mailbox: omap-mailbox: Flush out pending msgs before entering suspend Beleswar Padhi 2025-10-22 13:45 ` Hiago De Franco 2025-10-22 16:07 ` Andrew Davis 2025-10-23 4:38 ` Beleswar Prasad Padhi 2025-10-23 17:18 ` Hari Nagalla 2025-10-24 4:48 ` Beleswar Prasad Padhi 2025-10-24 14:08 ` Andrew Davis 2025-10-30 8:59 ` Francesco Dolcini 2025-10-30 9:12 ` Beleswar Prasad Padhi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®