Re: [PATCH 2/3] remoteproc: k3-dsp: Fix usage of omap_mbox_message and mbox_msg_t

[Andrew Davis <afd@ti.com>]

On 9/9/26 10:48 AM, Mathieu Poirier wrote:
> On Fri, 4 Sept 2026 at 13:56, Andrew Davis <afd@ti.com> wrote:
>>
>> The type of message sent using omap-mailbox is always u32. The definition
>> of mbox_msg_t is uintptr_t which is wrong as that type changes based on
>> the architecture (32bit vs 64bit). Make the type fixed to u32.
>>
>> This then means we need to fix the helper macro omap_mbox_message so it
>> doesn't cast to u32 twice. What this macro should be doing is converting
>> from the message type returned from the mailbox framework into the
>> omap-mailbox type. When fixing this we should add a macro for the other
>> direction (from omap-mailbox type to something that can be used with
>> mbox_send_message).
>>
>> After these changes, make use of the new macros as appropriate.
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>>   drivers/remoteproc/omap_remoteproc.c | 10 +++++-----
>>   drivers/remoteproc/ti_k3_common.c    | 10 ++--------
>>   include/linux/omap-mailbox.h         |  5 +++--
>>   3 files changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
>> index 6ed0f28edac9c..efff95b8e363f 100644
>> --- a/drivers/remoteproc/omap_remoteproc.c
>> +++ b/drivers/remoteproc/omap_remoteproc.c
>> @@ -499,7 +499,7 @@ static void omap_rproc_mbox_callback(struct mbox_client *client, void *data)
>>                                                  client);
>>          struct device *dev = oproc->rproc->dev.parent;
>>          const char *name = oproc->rproc->name;
>> -       u32 msg = (u32)data;
>> +       mbox_msg_t msg = omap_mbox_from_message(data);
>>
> 
> This patch looks familiar - was it submitted before but not merged?
> 

Checking the archives I see I sent a patch a couple years back that
removed the omap-mailbox.h header as it wasn't really used anywhere.
This patch kinda goes the other way and fixes the header so it has
some use.

> I'm fine with this set but would like an RB/TB from someone else at TI
> (for all 3 patches).
> 

Sure, let me see if I can get someone else here to independently test this
and report back on list.

Thanks,
Andrew

> Thanks,
> Mathieu
> 
>>          dev_dbg(dev, "mbox msg: 0x%x\n", msg);
>>
>> @@ -550,7 +550,7 @@ static void omap_rproc_kick(struct rproc *rproc, int vqid)
>>          }
>>
>>          /* send the index of the triggered virtqueue in the mailbox payload */
>> -       ret = mbox_send_message(oproc->mbox, (void *)vqid);
>> +       ret = mbox_send_message(oproc->mbox, omap_mbox_to_message(vqid));
>>          if (ret < 0)
>>                  dev_err(dev, "failed to send mailbox message, status = %d\n",
>>                          ret);
>> @@ -628,7 +628,7 @@ static int omap_rproc_start(struct rproc *rproc)
>>           * Note that the reply will _not_ arrive immediately: this message
>>           * will wait in the mailbox fifo until the remote processor is booted.
>>           */
>> -       ret = mbox_send_message(oproc->mbox, (void *)RP_MBOX_ECHO_REQUEST);
>> +       ret = mbox_send_message(oproc->mbox, omap_mbox_to_message(RP_MBOX_ECHO_REQUEST));
>>          if (ret < 0) {
>>                  dev_err(dev, "mbox_send_message failed: %d\n", ret);
>>                  goto put_mbox;
>> @@ -777,13 +777,13 @@ static int _omap_rproc_suspend(struct rproc *rproc, bool auto_suspend)
>>          struct omap_rproc *oproc = rproc->priv;
>>          unsigned long to = msecs_to_jiffies(DEF_SUSPEND_TIMEOUT);
>>          unsigned long ta = jiffies + to;
>> -       u32 suspend_msg = auto_suspend ?
>> +       mbox_msg_t suspend_msg = auto_suspend ?
>>                                  RP_MBOX_SUSPEND_AUTO : RP_MBOX_SUSPEND_SYSTEM;
>>          int ret;
>>
>>          reinit_completion(&oproc->pm_comp);
>>          oproc->suspend_acked = false;
>> -       ret = mbox_send_message(oproc->mbox, (void *)suspend_msg);
>> +       ret = mbox_send_message(oproc->mbox, omap_mbox_to_message(suspend_msg));
>>          if (ret < 0) {
>>                  dev_err(dev, "PM mbox_send_message failed: %d\n", ret);
>>                  return ret;
>> diff --git a/drivers/remoteproc/ti_k3_common.c b/drivers/remoteproc/ti_k3_common.c
>> index 3cb8ae5d72f67..4b6da3363f6c2 100644
>> --- a/drivers/remoteproc/ti_k3_common.c
>> +++ b/drivers/remoteproc/ti_k3_common.c
>> @@ -54,7 +54,7 @@ void k3_rproc_mbox_callback(struct mbox_client *client, void *data)
>>          struct k3_rproc *kproc = container_of(client, struct k3_rproc, client);
>>          struct device *dev = kproc->rproc->dev.parent;
>>          struct rproc *rproc = kproc->rproc;
>> -       u32 msg = (u32)(uintptr_t)(data);
>> +       mbox_msg_t msg = omap_mbox_from_message(data);
>>
>>          dev_dbg(dev, "mbox msg: 0x%x\n", msg);
>>
>> @@ -94,15 +94,9 @@ void k3_rproc_kick(struct rproc *rproc, int vqid)
>>   {
>>          struct k3_rproc *kproc = rproc->priv;
>>          struct device *dev = kproc->dev;
>> -       u32 msg = (u32)vqid;
>>          int ret;
>>
>> -       /*
>> -        * Send the index of the triggered virtqueue in the mailbox payload.
>> -        * NOTE: msg is cast to uintptr_t to prevent compiler warnings when
>> -        * void* is 64bit. It is safely cast back to u32 in the mailbox driver.
>> -        */
>> -       ret = mbox_send_message(kproc->mbox, (void *)(uintptr_t)msg);
>> +       ret = mbox_send_message(kproc->mbox, omap_mbox_to_message(vqid));
>>          if (ret < 0)
>>                  dev_err(dev, "failed to send mailbox message, status = %d\n",
>>                          ret);
>> diff --git a/include/linux/omap-mailbox.h b/include/linux/omap-mailbox.h
>> index 3cc5c4ed7f5a6..e45f8c3841c23 100644
>> --- a/include/linux/omap-mailbox.h
>> +++ b/include/linux/omap-mailbox.h
>> @@ -6,8 +6,9 @@
>>   #ifndef OMAP_MAILBOX_H
>>   #define OMAP_MAILBOX_H
>>
>> -typedef uintptr_t mbox_msg_t;
>> +typedef u32 mbox_msg_t;
>>
>> -#define omap_mbox_message(data) (u32)(mbox_msg_t)(data)
>> +#define omap_mbox_to_message(data) ((void *)(uintptr_t)(data))
>> +#define omap_mbox_from_message(data) ((mbox_msg_t)(uintptr_t)(data))
>>
>>   #endif /* OMAP_MAILBOX_H */
>> --
>> 2.39.2
>>