Re: [PATCH] mailbox: check mailbox queue is full or not

[Tanmay Shah <tanmay.shah@amd.com>]

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
>>>>
> 
>