Re: [PATCH net-next v7 4/5] net: rnpgbe: Add basic mbx_fw support

On Tue, Aug 26, 2025 at 02:39:07PM +0200, Andrew Lunn wrote:

quoted

Yes. It is not safe, so I 'must wait_event_timeout before free cookie'....
But is there a safe way to do it?
Maybe:
->allocate cookie
  -> map it to an unique id
    ->set the id to req->cookie
      ->receive response and check id valid? Then access cookie?

This is part of why adding cookies in a separate patch with a good
commit message is important.

Please take a step back. What is the big picture? Why do you need a
cookie? What is it used for? If you describe what your requirements
are, we might be able to suggest a better solution, or point you at a
driver you can copy code from.

	Andrew

I try to explain the it:

driver-->fw, we has two types request:
1. without response, such as mucse_mbx_ifinsmod
2. with response, such as mucse_fw_get_macaddr

fw --> driver, we has one types request:
1. link status (link speed, duplex, pause status...)

fw tiggers irq when it sends response or request.
In order to handle link status timely, we do an irqhandle like this:

static int rnpgbe_rcv_msg_from_fw(struct mucse *mucse)
{
        u32 msgbuf[MUCSE_FW_MAILBOX_WORDS];
        struct mucse_hw *hw = &mucse->hw;
        struct mbx_fw_cmd_reply *reply;
        int retval;
	/* read mbx data out */
        retval = mucse_read_mbx(hw, msgbuf, MUCSE_FW_MAILBOX_WORDS);
        if (retval)
                return retval;

        reply = (struct mbx_fw_cmd_reply *)msgbuf;
	/* judge request or response */
        if (le16_to_cpu(reply->flags) & FLAGS_DD) {
		/* if it is a response, call wake_up(cookie) */
                return rnpgbe_mbx_fw_reply_handler(mucse,
                                (struct mbx_fw_cmd_reply *)msgbuf);
        } else {
		/* if it is a request, handle link status */
                return rnpgbe_mbx_fw_req_handler(mucse,
                                (struct mbx_fw_cmd_req *)msgbuf);
        }
}

And driver requests with response is bellow 'without' irqhandle:

static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
				  struct mbx_fw_cmd_req *req,
				  struct mbx_fw_cmd_reply *reply)
{
...
	mucse_write_posted_mbx(hw, (u32 *)req, len);

	...
	/* but as irqhandle be added, mbx data is read out in the
	 * handler, mucse_read_posted_mbx cannot read anything */
	mucse_read_posted_mbx(hw, (u32 *)reply, sizeof(*reply));

}

To solve mucse_read_posted_mbx cannot read data with irq, we add 'cookie'.
After mucse_write_posted_mbx, call wait_event_timeout. wake_up is called
in irqhandle.

Thanks for your feedback.