> +static int amd_deinit_sdw_manager(struct amd_sdw_manager *amd_manager)
> +{
> + int ret;
> +
> + amd_disable_sdw_interrupts(amd_manager);
> + ret = amd_disable_sdw_manager(amd_manager);
> + return ret;
return amd_disable_sdw_manager(amd_manager); ?
> +}
> +static int amd_sdw_clock_stop(struct amd_sdw_manager *amd_manager)
> +{
> + u32 val;
> + u32 retry_count = 0;
> + int ret;
> +
> + ret = sdw_bus_prep_clk_stop(&amd_manager->bus);
> + if (ret < 0 && ret != -ENODATA) {
> + dev_err(amd_manager->dev, "prepare clock stop failed %d", ret);
> + return 0;
> + }
> + ret = sdw_bus_clk_stop(&amd_manager->bus);
> + if (ret < 0 && ret != -ENODATA) {
> + dev_err(amd_manager->dev, "bus clock stop failed %d", ret);
> + return 0;
> + }
> +
> + do {
> + val = acp_reg_readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
don't you need a minimal usleep_range to avoid re-reading the same
register over and over?
This is tied to the frame duration, isn't it? this is 20us typically.
> + if (val & AMD_SDW_CLK_STOP_DONE) {
> + amd_manager->clk_stopped = true;
> + break;
> + }
> + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT);
> +
> + if (!amd_manager->clk_stopped) {
> + dev_err(amd_manager->dev, "SDW%x clock stop failed\n", amd_manager->instance);
> + return 0;
> + }
> +
> + if (amd_manager->wake_en_mask)
> + acp_reg_writel(0x01, amd_manager->acp_mmio + ACP_SW_WAKE_EN(amd_manager->instance));
> +
> + dev_dbg(amd_manager->dev, "SDW%x clock stop successful\n", amd_manager->instance);
> + return 0;
> +}
> +
> +static int amd_sdw_clock_stop_exit(struct amd_sdw_manager *amd_manager)
> +{
> + int ret;
> + u32 val;
> + u32 retry_count = 0;
> +
> + if (amd_manager->clk_stopped) {
> + val = acp_reg_readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
> + val |= AMD_SDW_CLK_RESUME_REQ;
> + acp_reg_writel(val, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
> + do {
> + val = acp_reg_readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
> + if (val & AMD_SDW_CLK_RESUME_DONE)
> + break;
> + usleep_range(10, 100);
that's 10x range for sleep, that sounds a vague and suspicious, no?
> + } while (retry_count++ < AMD_SDW_CLK_STOP_MAX_RETRY_COUNT);
> + if (val & AMD_SDW_CLK_RESUME_DONE) {
> + acp_reg_writel(0, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
> + ret = sdw_bus_exit_clk_stop(&amd_manager->bus);
> + if (ret < 0)
> + dev_err(amd_manager->dev, "bus failed to exit clock stop %d\n",
> + ret);
> + amd_manager->clk_stopped = false;
> + }
> + }
> + if (amd_manager->clk_stopped) {
> + dev_err(amd_manager->dev, "SDW%x clock stop exit failed\n", amd_manager->instance);
> + return 0;
> + }
> + dev_dbg(amd_manager->dev, "SDW%x clock stop exit successful\n", amd_manager->instance);
> + return 0;
