On 7/7/21 9:45 PM, Pierre-Louis Bossart wrote:
Missing 'ASoC: amd' prefix in commit subject?
Will add prefix.
+static int acp5x_power_on(void __iomem *acp5x_base) +{
- u32 val;
- int timeout;
- val = acp_readl(acp5x_base + ACP_PGFSM_STATUS);
- if (val == 0)
return val;- if ((val & ACP_PGFSM_STATUS_MASK) !=
ACP_POWER_ON_IN_PROGRESS)acp_writel(ACP_PGFSM_CNTL_POWER_ON_MASK,acp5x_base + ACP_PGFSM_CONTROL);- timeout = 0;
- while (++timeout < 500) {
val = acp_readl(acp5x_base + ACP_PGFSM_STATUS);if (!val)Shouldn't you use something like if ((val & ACP_PGFSM_STATUS_MASK) == ACP_POWERED_ON) for symmetry with the power-off case?
Yes we can do that. will fix it.
return 0;udelay(1);- }
- return -ETIMEDOUT;
+}
+static int acp5x_power_off(void __iomem *acp5x_base) +{
- u32 val;
- int timeout;
- acp_writel(ACP_PGFSM_CNTL_POWER_OFF_MASK,
acp5x_base + ACP_PGFSM_CONTROL);- timeout = 0;
- while (++timeout < 500) {
val = acp_readl(acp5x_base + ACP_PGFSM_STATUS);if ((val & ACP_PGFSM_STATUS_MASK) == ACP_POWERED_OFF)return 0;udelay(1);- }
- return -ETIMEDOUT;
+}
+static int acp5x_reset(void __iomem *acp5x_base) +{
- u32 val;
- int timeout;
- acp_writel(1, acp5x_base + ACP_SOFT_RESET);
- timeout = 0;
- while (++timeout < 500) {
val = acp_readl(acp5x_base + ACP_SOFT_RESET);if (val & ACP_SOFT_RESET_SOFTRESET_AUDDONE_MASK)break;cpu_relax();- }
- acp_writel(0, acp5x_base + ACP_SOFT_RESET);
- timeout = 0;
- while (++timeout < 500) {
val = acp_readl(acp5x_base + ACP_SOFT_RESET);if (!val)return 0;cpu_relax();- }
- return -ETIMEDOUT;
+}
+static void acp5x_enable_interrupts(void __iomem *acp5x_base) +{
- acp_writel(0x01, acp5x_base + ACP_EXTERNAL_INTR_ENB);
+}
+static void acp5x_disable_interrupts(void __iomem *acp5x_base) +{
- acp_writel(ACP_EXT_INTR_STAT_CLEAR_MASK, acp5x_base +
ACP_EXTERNAL_INTR_STAT);- acp_writel(0x00, acp5x_base + ACP_EXTERNAL_INTR_CNTL);
- acp_writel(0x00, acp5x_base + ACP_EXTERNAL_INTR_ENB);
+}
+static int acp5x_init(void __iomem *acp5x_base) +{
- int ret;
- /* power on */
- ret = acp5x_power_on(acp5x_base);
- if (ret) {
pr_err("ACP5x power on failed\n");return ret;- }
- /* Reset */
- ret = acp5x_reset(acp5x_base);
- if (ret) {
pr_err("ACP5x reset failed\n");return ret;- }
- acp5x_enable_interrupts(acp5x_base);
- return 0;
+}
+static int acp5x_deinit(void __iomem *acp5x_base) +{
- int ret;
- acp5x_disable_interrupts(acp5x_base);
- /* Reset */
- ret = acp5x_reset(acp5x_base);
- if (ret) {
pr_err("ACP5x reset failed\n");return ret;- }
- /* power off */
- if (acp_power_gating) {
ret = acp5x_power_off(acp5x_base);if (ret) {pr_err("ACP5x power off failed\n");return ret;}- }
- return 0;
shouldn't you have a sequence for shutdown that ignores the acp_power_gating parameter?
BIOS will apply ACP Power gating(which is part of ACP power off sequence) when ACP enters D3 state. i.e BIOS will control ACP Power gating.
acp5x_power_off() API is no longer required.This was implemented to verify ACP Power gating during initial driver bring up.
We will drop acp5x_power_off() API, will remove acp_power_gating parameter and post the new version.