linux-kernel - Re: [PATCH V3 6/8] soundwire: amd: add runtime pm ops for AMD SoundWire manager driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <72b601b9-93bf-b9f5-c898-c63ad46e5167@linux.intel.com>
Date:   Tue, 21 Feb 2023 11:10:30 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Vijendar Mukunda <Vijendar.Mukunda@....com>, vkoul@...nel.org
Cc:     alsa-devel@...a-project.org, amadeuszx.slawinski@...ux.intel.com,
        Mario.Limonciello@....com, Sunil-kumar.Dommati@....com,
        Basavaraj.Hiregoudar@....com, Mastan.Katragadda@....com,
        Arungopal.kondaveeti@....com,
        Bard Liao <yung-chuan.liao@...ux.intel.com>,
        Sanyog Kale <sanyog.r.kale@...el.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3 6/8] soundwire: amd: add runtime pm ops for AMD
 SoundWire manager driver


> +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;