Hi Bingbu,
Just a small comment...
On Thu, Jan 11, 2024 at 02:55:21PM +0800, bingbu.cao@xxxxxxxxx wrote:
> +int ipu6_fw_isys_close(struct ipu6_isys *isys)
> +{
> + struct device *dev = &isys->adev->auxdev.dev;
> + int retry = IPU6_ISYS_CLOSE_RETRY;
> + unsigned long flags;
> + void *fwcom;
> + int ret;
> +
> + /*
> + * Stop the isys fw. Actual close takes
> + * some time as the FW must stop its actions including code fetch
> + * to SP icache.
> + * spinlock to wait the interrupt handler to be finished
> + */
> + spin_lock_irqsave(&isys->power_lock, flags);
> + ret = ipu6_fw_com_close(isys->fwcom);
> + fwcom = isys->fwcom;
> + isys->fwcom = NULL;
> + spin_unlock_irqrestore(&isys->power_lock, flags);
> + if (ret)
> + dev_err(dev, "Device close failure: %d\n", ret);
> +
> + /* release probably fails if the close failed. Let's try still */
> + do {
> + usleep_range(400, 500);
> + ret = ipu6_fw_com_release(fwcom, 0);
> + retry--;
> + } while (ret && retry);
This can be written as:
ret = read_poll_timeout(ipu6_fw_com_release, ret, !ret, 400,
2000000, true, fwcom, 0);
Other similar constructs, too, should be replaced.
--
Sakari Ailus
