On Tue, Aug 12, 2025 at 05:14:15PM +0100, Vadim Fedorenko wrote:
> On 12/08/2025 10:39, Dong Yibo wrote:
> > Initialize basic mbx_fw ops, such as get_capability, reset phy
> > and so on.
> >
> > Signed-off-by: Dong Yibo <dong100@xxxxxxxxx>
> > +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...
>
Ok, I will try to update this.
> > + do {
> > + err = hw->mbx.ops->read_posted(hw, (u32 *)reply,
> > + L_WD(sizeof(*reply)));
> > + if (err) {
> > + mutex_unlock(&hw->mbx.lock);
> > + return err;
> > + }
> > + } while (--retry_cnt >= 0 && reply->opcode != req->opcode);
> > + mutex_unlock(&hw->mbx.lock);
> > + if (retry_cnt < 0 || reply->error_code)
> > + return -EIO;
> > + return 0;
> > +}
> > +
> > +/**
> > + * mucse_fw_get_capability - Get hw abilities from fw
> > + * @hw: pointer to the HW structure
> > + * @abil: pointer to the hw_abilities structure
> > + *
> > + * mucse_fw_get_capability tries to get hw abilities from
> > + * hw.
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +static int mucse_fw_get_capability(struct mucse_hw *hw,
> > + struct hw_abilities *abil)
> > +{
> > + struct mbx_fw_cmd_reply reply;
> > + struct mbx_fw_cmd_req req;
> > + int err;
> > +
> > + memset(&req, 0, sizeof(req));
> > + memset(&reply, 0, sizeof(reply));
>
> probably
>
> struct mbx_fw_cmd_reply reply = {};
> struct mbx_fw_cmd_req req = {};
>
> will look better. in the other functions as well..
>
>
>
Got it, I will update it.
Thanks for your feedback.
