On Thu, Sep 04, 2025 at 01:06:09PM -0400, Adam Young wrote:

Answers inline.

On 9/4/25 07:00, Sudeep Holla wrote:

[...]

quoted

Who will change this value as it is fixed to false always.
That makes the whole pcc_write_to_buffer() reduntant. It must go away.
Also why can't you use tx_prepare callback here. I don't like these changes
at all as I find these redundant. Sorry for not reviewing it in time.
I was totally confused with your versioning and didn't spot the mailbox/pcc
changes in between and assumed it is just MCTP net driver changes. My mistake.

This was a case of leaving the default as is to not-break the existing
mailbox clients.

The maibox client can over ride it in its driver setup.

What if driver changes in the middle of an ongoing transaction ? That
doesn't sound like a good idea to me.

You didn't respond as why tx_prepare callback can be used to do exactly
same thing ?

quoted

quoted

+	void *(*rx_alloc)(struct mbox_client *cl,  int size);

Why this can't be in rx_callback ?

Because that is too late.

The problem is that the client needs  to allocate the memory that the
message comes in in order to hand it off.

In the case of a network device, the rx_alloc code is going to return the
memory are of a struct sk_buff. The Mailbox does not know how to allocate
this. If the driver just kmallocs memory for the return message, we would
have a re-copy of the message.

I still don't understand the requirement. The PCC users has access to shmem
and can do what they want in rx_callback, so I don't see any reason for
this API.

This is really a mailbox-api level issue, but I was trying to limit the
scope of my changes as much as possible.

Please explain the issue. Sorry if I have missed, pointer are enough if
already present in some mail thread.

The PCC mailbox code really does not match the abstractions of the mailbox
in general.  The idea that copying into and out of the buffer is done by
each individual driver leads to a lot of duplicated code.  With this change,
most of the other drivers could now be re-written to let the mailbox manage
the copying, while letting the mailbox client specify only how to allocate
the message buffers.

Yes that's because each user have their own requirement. You can do what
you want in rx_callback.

Much of this change  was driven by the fact that the PCC mailbox does not
properly check the flags before allowing writes to the rx channel, and that
code is not exposed to the driver.  Thus, it was impossible to write
everything in the rx callback regardless. This work was based on Huisong's
comments on version 21 of the patch series.

Pointers please, sorry again. But I really don't like the merged code and
looking for ways to clean it up as well as address the requirement if it
is not available esp. if we have to revert this change.

-- 
Regards,
Sudeep