| 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
| ||
|
Message-ID: <20c226de-c53a-41f3-b432-6f75a6f83e75@ti.com>
Date: Mon, 3 Nov 2025 16:10:33 -0600
From: Andrew Davis <afd@...com>
To: Beleswar Padhi <b-padhi@...com>, <jassisinghbrar@...il.com>,
<linux-kernel@...r.kernel.org>, <hiago.franco@...adex.com>,
<francesco.dolcini@...adex.com>
CC: <hnagalla@...com>, <u-kumar1@...com>, <nm@...com>, <vigneshr@...com>
Subject: Re: [PATCH v2] mailbox: omap-mailbox: Check for pending msgs only
when mbox is exclusive
On 11/3/25 2:11 PM, Beleswar Padhi wrote:
> On TI K3 devices, the mailbox resides in the Always-On power domain
> (LPSC_main_alwayson) and is shared among multiple processors. The
> mailbox is not solely exclusive to Linux.
>
> Currently, the suspend path checks all FIFO queues for pending messages
> and blocks suspend if any are present. This behavior is unnecessary for
> K3 devices, since some of the FIFOs are used for RTOS<->RTOS
> communication and are independent of Linux.
>
> For FIFOs used in Linux<->RTOS communication, any pending message would
> trigger an interrupt, which naturally prevents suspend from completing.
> Hence, there is no need for the mailbox driver to explicitly check for
> pending messages on K3 platforms.
>
> Introduce a device match flag to indicate whether the mailbox instance
> is exclusive to Linux, and skip the pending message check for
> non-exclusive instances (such as in K3).
>
> Fixes: a49f991e740f ("arm64: dts: ti: k3-am62-verdin: Add missing cfg for TI IPC Firmware")
> Closes: https://lore.kernel.org/all/sid7gtg5vay5qgicsl6smnzwg5mnneoa35cempt5ddwjvedaio@hzsgcx6oo74l/
> Signed-off-by: Beleswar Padhi <b-padhi@...com>
> ---
> Cc: Francesco Dolcini <francesco.dolcini@...adex.com>
> Cc: Hiago De Franco <hiago.franco@...adex.com>
> Please help in testing the patch on Toradex platforms.
>
> Testing Done:
> 1. Tested Boot across all TI K3 EVM/SK boards.
> 2. Tested IPC on all TI K3 J7* EVM/SK boards (& AM62x SK).
> 3. Tested mbox driver probe & device boot on AM57x-evm (OMAP5 based).
> 4. Tested that the patch generates no new warnings/errors.
>
> Changes since v1:
> 1. Use device_get_match_data() in probe and store result for re-use.
>
> Link to v1:
> https://lore.kernel.org/all/20251103075920.2611642-1-b-padhi@ti.com/
>
> Changes since RFC:
> 1. Skip checking pending messages instead of flushing
> them explicitly for K3 devices.
>
> Link to RFC Version:
> https://lore.kernel.org/all/20251022102015.1345696-1-b-padhi@ti.com/
>
> drivers/mailbox/omap-mailbox.c | 35 +++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
> index 680243751d62..17fe6545875d 100644
> --- a/drivers/mailbox/omap-mailbox.c
> +++ b/drivers/mailbox/omap-mailbox.c
> @@ -68,6 +68,7 @@ struct omap_mbox_fifo {
>
> struct omap_mbox_match_data {
> u32 intr_type;
> + bool is_exclusive;
> };
>
> struct omap_mbox_device {
> @@ -78,6 +79,7 @@ struct omap_mbox_device {
> u32 num_users;
> u32 num_fifos;
> u32 intr_type;
> + const struct omap_mbox_match_data *mbox_data;
Not a fan of this, you could have stored just the relevant
flag from the match data here in the instance data, not a
pointer to the whole const struct. The issue being now you
have the same info stored in two places, `intr_type` in the
match data above, and intr_type on the line above that.
Looking more into it, the use of `intr_type` doesn't need to be
stored in each mbox's instance data either, but that is an existing
issue. For now I don't want to hold up an otherwise good bugfix
over that, but we will want to come back here and clean some of
this up next cycle, for now,
Reviewed-by: Andrew Davis <afd@...com>
> };
>
> struct omap_mbox {
> @@ -341,11 +343,13 @@ 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;
> + if (mdev->mbox_data->is_exclusive) {
> + 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;
> + }
> }
> }
>
> @@ -378,8 +382,9 @@ static const struct dev_pm_ops omap_mbox_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(omap_mbox_suspend, omap_mbox_resume)
> };
>
> -static const struct omap_mbox_match_data omap2_data = { MBOX_INTR_CFG_TYPE1 };
> -static const struct omap_mbox_match_data omap4_data = { MBOX_INTR_CFG_TYPE2 };
> +static const struct omap_mbox_match_data omap2_data = { MBOX_INTR_CFG_TYPE1, true };
> +static const struct omap_mbox_match_data omap4_data = { MBOX_INTR_CFG_TYPE2, true };
> +static const struct omap_mbox_match_data am654_data = { MBOX_INTR_CFG_TYPE2, false };
>
> static const struct of_device_id omap_mailbox_of_match[] = {
> {
> @@ -396,11 +401,11 @@ static const struct of_device_id omap_mailbox_of_match[] = {
> },
> {
> .compatible = "ti,am654-mailbox",
> - .data = &omap4_data,
> + .data = &am654_data,
> },
> {
> .compatible = "ti,am64-mailbox",
> - .data = &omap4_data,
> + .data = &am654_data,
> },
> {
> /* end */
> @@ -449,7 +454,6 @@ static int omap_mbox_probe(struct platform_device *pdev)
> struct omap_mbox_fifo *fifo;
> struct device_node *node = pdev->dev.of_node;
> struct device_node *child;
> - const struct omap_mbox_match_data *match_data;
> struct mbox_controller *controller;
> u32 intr_type, info_count;
> u32 num_users, num_fifos;
> @@ -462,11 +466,6 @@ static int omap_mbox_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> - match_data = of_device_get_match_data(&pdev->dev);
> - if (!match_data)
> - return -ENODEV;
> - intr_type = match_data->intr_type;
> -
> if (of_property_read_u32(node, "ti,mbox-num-users", &num_users))
> return -ENODEV;
>
> @@ -483,6 +482,12 @@ static int omap_mbox_probe(struct platform_device *pdev)
> if (!mdev)
> return -ENOMEM;
>
> + mdev->mbox_data = device_get_match_data(&pdev->dev);
> + if (!mdev->mbox_data)
> + return -ENODEV;
> +
> + intr_type = mdev->mbox_data->intr_type;
> +
> mdev->mbox_base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(mdev->mbox_base))
> return PTR_ERR(mdev->mbox_base);
Powered by blists - more mailing lists