Re: [PATCH v3 4/5] net: rnpgbe: Add basic mbx_fw support
From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Date: 2025-08-13 13:33:53
Also in:
linux-doc, lkml
On 13/08/2025 10:52, Yibo Dong wrote:
On Tue, Aug 12, 2025 at 05:14:15PM +0100, Vadim Fedorenko wrote:quoted
On 12/08/2025 10:39, Dong Yibo wrote:quoted
Initialize basic mbx_fw ops, such as get_capability, reset phy and so on. Signed-off-by: Dong Yibo <dong100@mucse.com> +static int mucse_fw_send_cmd_wait(struct mucse_hw *hw, + struct mbx_fw_cmd_req *req, + struct mbx_fw_cmd_reply *reply) +{ + int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN; + int retry_cnt = 3; + int err; + + err = mutex_lock_interruptible(&hw->mbx.lock); + if (err) + return err; + err = hw->mbx.ops->write_posted(hw, (u32 *)req, + L_WD(len)); + if (err) {> + mutex_unlock(&hw->mbx.lock); + return err; + }it might look a bit cleaner if you add error label and have unlock code once in the end of the function...If it is more cleaner bellow? static int mucse_fw_send_cmd_wait(struct mucse_hw *hw, struct mbx_fw_cmd_req *req, struct mbx_fw_cmd_reply *reply) { int len = le16_to_cpu(req->datalen) + MBX_REQ_HDR_LEN; int retry_cnt = 3; int err; err = mutex_lock_interruptible(&hw->mbx.lock); if (err) return err; err = hw->mbx.ops->write_posted(hw, (u32 *)req, L_WD(len)); if (err) goto quit; do { err = hw->mbx.ops->read_posted(hw, (u32 *)reply, L_WD(sizeof(*reply))); if (err) goto quit; } while (--retry_cnt >= 0 && reply->opcode != req->opcode); mutex_unlock(&hw->mbx.lock); if (retry_cnt < 0) return -ETIMEDOUT; if (reply->error_code) return -EIO; return 0; quit: mutex_unlock(&hw->mbx.lock); return err; }
Maybe:
} while (--retry_cnt >= 0 && reply->opcode != req->opcode);
quit:
mutex_unlock(&hw->mbx.lock);
if (!err && retry_cnt < 0)
return -ETIMEDOUT;
if (!err && reply->error_code)
return -EIO;
return err;
looks cleaner