> +/**
> + * mucse_fw_send_cmd_wait - Send cmd req and wait for response
> + * @hw: Pointer to the HW structure
> + * @req: Pointer to the cmd req structure
> + * @reply: Pointer to the fw reply structure
> + *
> + * mucse_fw_send_cmd_wait sends req to pf-cm3 mailbox and wait
> + * reply from fw.
> + *
> + * Returns 0 on success, negative on failure
> + **/
> +static int mucse_fw_send_cmd_wait(struct mucse_hw *hw,
> + struct mbx_fw_cmd_req *req,
> + struct mbx_fw_cmd_reply *reply)
> +{
> + int err;
> + int retry_cnt = 3;
> +
> + if (!hw || !req || !reply || !hw->mbx.ops.read_posted)
Can this happen?
If this is not supposed to happen, it is better the driver opps, so
you get a stack trace and find where the driver is broken.
> + return -EINVAL;
> +
> + /* if pcie off, nothing todo */
> + if (pci_channel_offline(hw->pdev))
> + return -EIO;
What can cause it to go offline? Is this to do with PCIe hotplug?
> +
> + if (mutex_lock_interruptible(&hw->mbx.lock))
> + return -EAGAIN;
mutex_lock_interruptable() returns -EINTR, which is what you should
return, not -EAGAIN.
> +
> + err = hw->mbx.ops.write_posted(hw, (u32 *)req,
> + L_WD(req->datalen + MBX_REQ_HDR_LEN),
> + MBX_FW);
> + if (err) {
> + mutex_unlock(&hw->mbx.lock);
> + return err;
> + }
> +
> +retry:
> + retry_cnt--;
> + if (retry_cnt < 0)
> + return -EIO;
> +
> + err = hw->mbx.ops.read_posted(hw, (u32 *)reply,
> + L_WD(sizeof(*reply)),
> + MBX_FW);
> + if (err) {
> + mutex_unlock(&hw->mbx.lock);
> + return err;
> + }
> +
> + if (reply->opcode != req->opcode)
> + goto retry;
> +
> + mutex_unlock(&hw->mbx.lock);
> +
> + if (reply->error_code)
> + return -reply->error_code;
The mbox is using linux error codes?
> +#define FLAGS_DD BIT(0) /* driver clear 0, FW must set 1 */
> +/* driver clear 0, FW must set only if it reporting an error */
> +#define FLAGS_ERR BIT(2)
> +
> +/* req is little endian. bigendian should be conserened */
> +struct mbx_fw_cmd_req {
> + u16 flags; /* 0-1 */
> + u16 opcode; /* 2-3 enum GENERIC_CMD */
> + u16 datalen; /* 4-5 */
> + u16 ret_value; /* 6-7 */
If this is little endian, please use __le16, __le32 etc, so that the
static analysers will tell you if you are missing cpu_to_le32 etc.
Andrew
