[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <09b6bf14-752d-45b2-9cbb-f64f7b6703ee@amd.com>
Date: Mon, 29 Sep 2025 14:35:24 -0500
From: Tanmay Shah <tanmay.shah@....com>
To: Mathieu Poirier <mathieu.poirier@...aro.org>, Peng Fan
<peng.fan@....nxp.com>
CC: <jassisinghbrar@...il.com>, <andersson@...nel.org>,
<linux-kernel@...r.kernel.org>, <linux-remoteproc@...r.kernel.org>
Subject: Re: [PATCH] mailbox: check mailbox queue is full or not
On 9/29/25 2:26 PM, Tanmay Shah wrote:
>
>
> On 9/29/25 9:45 AM, Mathieu Poirier wrote:
>> On Sun, Sep 28, 2025 at 03:56:41PM +0800, Peng Fan wrote:
>>> Hi,
>>>
>>> On Fri, Sep 26, 2025 at 10:40:09AM -0500, Tanmay Shah wrote:
>>>>>> ---
>>>>>> drivers/mailbox/mailbox.c | 24 ++++++++++++++++++++++++
>>>>>> drivers/remoteproc/xlnx_r5_remoteproc.c | 4 ++++
>>>>>> include/linux/mailbox_client.h | 1 +
>>>>>
>>>>> The mailbox and remoteproc should be separated.
>>>>>
>>>>
[...]
>
>>>>
>>>>>
>>>>>> + res = (chan->msg_count == (MBOX_TX_QUEUE_LEN - 1));
>>
>> Please have a look at this condition again - the implementation of
>> addr_to_rbuf() [1] is checking for space differently.
>>
>> [1]. https://elixir.bootlin.com/linux/v6.17/source/drivers/mailbox/
>> mailbox.c#L32
>>
Here Ack as well. I think it should be same as what's there in add_to_rbuf.
>>>>>> + spin_unlock_irqrestore(&chan->lock, flags);
>>>>>> +
>>>>>> + return res;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(mbox_queue_full);
>>>>>
>>>>> add_to_rbuf is able to return ENOBUFS when call mbox_send_message.
>>>>> Does checking mbox_send_message return value works for you?
>>>>>
>>>>
>>>> That is the problem. mbox_send_message uses add_to_rbuf and fails.
>>>> But during
>>>> failure, it prints warning message:
>>>>
>>>> dev_err(chan->mbox->dev, "Try increasing MBOX_TX_QUEUE_LEN\n");
>>>>
>>>> In some cases there are lot of such messages on terminal. Functionally
>>>> nothing is wrong and everything is working but user keeps getting false
>>>> positive warning about increasing mbox tx queue length. That is why
>>>> we need
>>>> API to check if mbox queue length is full or not before doing
>>>> mbox_send_message. Not all clients need to use it, but some cane
>>>> make use of
>>>> it.
>>>
>>> I think check whether mbox_send_message returns -ENOBUFS or not should
>>> work for you. If the "Try increasing MBOX_TX_QUEUE_LEN" message
>>> bothers you, it could be update to dev_dbg per my understanding.
>>>
>>
>> This new API is trying to avoid calling mbox_send_message(), no
>> checking if it
>> succeeded or not. Moving dev_err() nto dev_dbg() is also the wrong
>> approach.
>>
>
> Correct.
>
>>> Regards,
>>> Peng
>>>
>>>>
>>>>
>>>>>> +
>>>>>> /**
>>>>>> * mbox_send_message - For client to submit a message to be
>>>>>> * sent to the remote.
>>>>>
>>>>> Regards
>>>>> Peng
>>>>
>
>
Powered by blists - more mailing lists