On Thu, Sep 04, 2025 at 12:45:43AM +0200, Andrew Lunn wrote: > > +/** > > + * mucse_mbx_powerup - Echo fw to powerup > > + * @hw: pointer to the HW structure > > + * @is_powerup: true for powerup, false for powerdown > > + * > > + * mucse_mbx_powerup echo fw to change working frequency > > + * to normal after received true, and reduce working frequency > > + * if false. > > + * > > + * Return: 0 on success, negative errno on failure > > + **/ > > +int mucse_mbx_powerup(struct mucse_hw *hw, bool is_powerup) > > +{ > > + struct mbx_fw_cmd_req req = {}; > > + int len; > > + int err; > > + > > + build_powerup(&req, is_powerup); > > + len = le16_to_cpu(req.datalen); > > + mutex_lock(&hw->mbx.lock); > > + > > + if (is_powerup) { > > + err = mucse_write_posted_mbx(hw, (u32 *)&req, > > + len); > > + } else { > > + err = mucse_write_mbx_pf(hw, (u32 *)&req, > > + len); > > + } > > It looks odd that this is asymmetric. Why is a different low level > function used between power up and power down? > > > +int mucse_mbx_reset_hw(struct mucse_hw *hw) > > +{ > > + struct mbx_fw_cmd_reply reply = {}; > > + struct mbx_fw_cmd_req req = {}; > > + > > + build_reset_hw_req(&req); > > + return mucse_fw_send_cmd_wait(hw, &req, &reply); > > +} > > And this uses a third low level API different to power up and power > down? > > Andrew > There are 3 APIs, they has some different .... 1. mucse_write_mbx_pf just sends mbx. /** * mucse_write_mbx_pf - Place a message in the mailbox * @hw: pointer to the HW structure * @msg: the message buffer * @size: length of buffer * */ 2. mucse_write_posted_mbx sends mbx and wait fw read out. /** * mucse_write_posted_mbx - Write a message to the mailbox, wait for ack * @hw: pointer to the HW structure * @msg: the message buffer * @size: length of buffer */ powerdown is called when driver is removed, nothing to do with fw later, no need to wait fw's ack. Of course, it can use mucse_write_posted_mbx to wait when powerdown if you want me to do it? 3. mucse_fw_send_cmd_wait sends mbx, wait fw read out, and wait for response. /** * 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 */ Thanks for feedback.