[PATCH net-next v29 1/3] mailbox: add callback function for rx buffer allocation

Adam Young posted 3 patches 6 days, 4 hours ago
[PATCH net-next v29 1/3] mailbox: add callback function for rx buffer allocation
Posted by Adam Young 6 days, 4 hours ago
Allows the mailbox client to specify how to allocate the memory
that the mailbox controller uses to send the message to the client.

In the case of a network driver, the message should be allocated as
a struct sk_buff allocated and managed by the network subsystem.  The
two parameters passed back from the callback represent the sk_buff
itself and the data section inside the skbuff where the message gets
written.

For simpler cases where the client kmallocs a buffer or returns
static memory, both pointers should point to the same value.

Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
 include/linux/mailbox_client.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h
index c6eea9afb943..901184d0515e 100644
--- a/include/linux/mailbox_client.h
+++ b/include/linux/mailbox_client.h
@@ -21,6 +21,12 @@ struct mbox_chan;
  * @knows_txdone:	If the client could run the TX state machine. Usually
  *			if the client receives some ACK packet for transmission.
  *			Unused if the controller already has TX_Done/RTR IRQ.
+ * @rx_alloc		Optional callback that allows the driver
+ *			to allocate the memory used for receiving
+ *			messages.  The handle parameter is the value to return
+ *			to the client,buffer is the location the mailbox should
+ *			write to, and size it the size of the buffer to allocate.
+ *			inside the buffer where the mailbox should write the data.
  * @rx_callback:	Atomic callback to provide client the data received
  * @tx_prepare: 	Atomic callback to ask client to prepare the payload
  *			before initiating the transmission if required.
@@ -32,6 +38,7 @@ struct mbox_client {
 	unsigned long tx_tout;
 	bool knows_txdone;
 
+	void (*rx_alloc)(struct mbox_client *cl, void **handle, void **buffer, int size);
 	void (*rx_callback)(struct mbox_client *cl, void *mssg);
 	void (*tx_prepare)(struct mbox_client *cl, void *mssg);
 	void (*tx_done)(struct mbox_client *cl, void *mssg, int r);
-- 
2.43.0
Re: [PATCH net-next v29 1/3] mailbox: add callback function for rx buffer allocation
Posted by Sudeep Holla 5 days, 7 hours ago
Hi,

On Thu, Sep 25, 2025 at 03:00:24PM -0400, Adam Young wrote:
> Allows the mailbox client to specify how to allocate the memory
> that the mailbox controller uses to send the message to the client.
> 
> In the case of a network driver, the message should be allocated as
> a struct sk_buff allocated and managed by the network subsystem.  The
> two parameters passed back from the callback represent the sk_buff
> itself and the data section inside the skbuff where the message gets
> written.
> 
> For simpler cases where the client kmallocs a buffer or returns
> static memory, both pointers should point to the same value.
> 

I have posted a revert[1] of a patch that you got merged recently and
this change builds on top of that. Please start fresh with that patch/revert
applied and explain why you can't use tx_prepare and rx_callback as
suggested and then we can take it from there. It makes no sense to add
these optional callbacks the way it is done at the least. If you are
worried about duplication of shmem handling, IIUC, it is done intentionally
to give that flexibility to the clients. If you think it can be consolidated,
I would rather have them as a standard helper which can be assigned to
tx_prepare and rx_callback and convert few users to demonstrate the same.

So I definitely don't like this addition and NACK until all those details
are looked at and you have convinced it can't be achieved via those and need
more changes.

On the other hand, the consolidation is a good thought, but it can't be
addition without any other drivers except you new yet to be merged driver
as the sole user of that. If there are no users needing copying to/from
the shared memory like you driver, then I am happy to consider this but it
needs more thinking and reviewing for sure. I will also have a look.

Sorry for missing the review of the original patch that got merged, my bad.

-- 
Regards,
Sudeep

[1] https://lore.kernel.org/all/20250926153311.2202648-1-sudeep.holla@arm.com
Re: [PATCH net-next v29 1/3] mailbox: add callback function for rx buffer allocation
Posted by Simon Horman 5 days, 9 hours ago
On Thu, Sep 25, 2025 at 03:00:24PM -0400, Adam Young wrote:
> Allows the mailbox client to specify how to allocate the memory
> that the mailbox controller uses to send the message to the client.
> 
> In the case of a network driver, the message should be allocated as
> a struct sk_buff allocated and managed by the network subsystem.  The
> two parameters passed back from the callback represent the sk_buff
> itself and the data section inside the skbuff where the message gets
> written.
> 
> For simpler cases where the client kmallocs a buffer or returns
> static memory, both pointers should point to the same value.
> 
> Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
> ---
>  include/linux/mailbox_client.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h
> index c6eea9afb943..901184d0515e 100644
> --- a/include/linux/mailbox_client.h
> +++ b/include/linux/mailbox_client.h
> @@ -21,6 +21,12 @@ struct mbox_chan;
>   * @knows_txdone:	If the client could run the TX state machine. Usually
>   *			if the client receives some ACK packet for transmission.
>   *			Unused if the controller already has TX_Done/RTR IRQ.
> + * @rx_alloc		Optional callback that allows the driver

@rx_alloc:
        ^^^

Flagged by ./scripts/kernel-doc -none -Wall

> + *			to allocate the memory used for receiving
> + *			messages.  The handle parameter is the value to return
> + *			to the client,buffer is the location the mailbox should
> + *			write to, and size it the size of the buffer to allocate.
> + *			inside the buffer where the mailbox should write the data.
>   * @rx_callback:	Atomic callback to provide client the data received
>   * @tx_prepare: 	Atomic callback to ask client to prepare the payload
>   *			before initiating the transmission if required.
> @@ -32,6 +38,7 @@ struct mbox_client {
>  	unsigned long tx_tout;
>  	bool knows_txdone;
>  
> +	void (*rx_alloc)(struct mbox_client *cl, void **handle, void **buffer, int size);
>  	void (*rx_callback)(struct mbox_client *cl, void *mssg);
>  	void (*tx_prepare)(struct mbox_client *cl, void *mssg);
>  	void (*tx_done)(struct mbox_client *cl, void *mssg, int r);
> -- 
> 2.43.0
> 
>