在 2025/4/29 2:48, Adam Young 写道:

On 4/24/25 09:03, lihuisong (C) wrote:

quoted

quoted

+    rc = mctp_pcc_initialize_mailbox(dev, &mctp_pcc_ndev->inbox,
+                     context.inbox_index);
+    if (rc)
+        goto free_netdev;
+    mctp_pcc_ndev->inbox.client.rx_callback = 
mctp_pcc_client_rx_callback;

It is good to move the assignemnt of  rx_callback pointer to 
initialize inbox mailbox.

The other changes are fine, but this one I do not agree with.

The rx callback only makes sense for one of the two mailboxes, and 
thus is not appropriate for a generic function.

Either  initialize_mailbox needs more complex logic, or would blindly 
assign the callback to both mailboxes, neither of which simplifies or 
streamlines the code.  That function emerged as a way to reduce 
duplication.  Lets keep it that way.

It depends on you. But please reply my below comment. I didn't see any 
change about it in next version.

-->

+static netdev_tx_t mctp_pcc_tx(struct sk_buff *skb, struct net_device 
*ndev)
+{
+    struct mctp_pcc_ndev *mpnd = netdev_priv(ndev);
+    struct mctp_pcc_hdr *mctp_pcc_header;
+    void __iomem *buffer;
+    unsigned long flags;
+    int len = skb->len;
+    int rc;
+
+    rc = skb_cow_head(skb, sizeof(struct mctp_pcc_hdr));
+    if (rc)
+        goto err_drop;
+
+    mctp_pcc_header = skb_push(skb, sizeof(struct mctp_pcc_hdr));
+    mctp_pcc_header->signature = cpu_to_le32(PCC_MAGIC | 
mpnd->outbox.index);
+    mctp_pcc_header->flags = cpu_to_le32(PCC_HEADER_FLAGS);
+    memcpy(mctp_pcc_header->mctp_signature, MCTP_SIGNATURE,
+           MCTP_SIGNATURE_LENGTH);
+    mctp_pcc_header->length = cpu_to_le32(len + MCTP_SIGNATURE_LENGTH);
+
+    spin_lock_irqsave(&mpnd->lock, flags);
+    buffer = mpnd->outbox.chan->shmem;
+    memcpy_toio(buffer, skb->data, skb->len);
+ mpnd->outbox.chan->mchan->mbox->ops->send_data(mpnd->outbox.chan->mchan,
+                            NULL);
+    spin_unlock_irqrestore(&mpnd->lock, flags);
+

Why does it not need to know if the packet is sent successfully?
It's possible for the platform not to finish to send the packet after 
executing this unlock.
In this moment, the previous packet may be modified by the new packet to 
be sent.