Messages in this thread | | | Subject | Re: [alsa-devel] [PATCH v3 6/6] ASoC: amd: Added ACP3x system resume and runtime pm | From | Pierre-Louis Bossart <> | Date | Thu, 7 Nov 2019 09:34:28 -0600 |
| |
> +static int acp3x_power_on(void __iomem *acp3x_base) > +{ > + u32 val; > + u32 timeout = 0; > + int ret = 0; > + > + val = rv_readl(acp3x_base + mmACP_PGFSM_STATUS); > + > + if (val == 0) > + return val; > + > + if (!((val & ACP_PGFSM_STATUS_MASK) == > + ACP_POWER_ON_IN_PROGRESS)) > + rv_writel(ACP_PGFSM_CNTL_POWER_ON_MASK, > + acp3x_base + mmACP_PGFSM_CONTROL); > + while (++timeout < 500) { > + val = rv_readl(acp3x_base + mmACP_PGFSM_STATUS); > + if (!val) > + break; > + udelay(1); > + if (timeout > 500) {
your timeout cannot reach 500, isn't this dead code?
> + pr_err("ACP is Not Powered ON\n"); > + return -ETIMEDOUT; > + } > + } > +} > + > +static int acp3x_power_off(void __iomem *acp3x_base) > +{ > + u32 val; > + u32 timeout = 0; > + > + rv_writel(ACP_PGFSM_CNTL_POWER_OFF_MASK, > + acp3x_base + mmACP_PGFSM_CONTROL); > + while (++timeout < 500) { > + val = rv_readl(acp3x_base + mmACP_PGFSM_STATUS); > + if ((val & ACP_PGFSM_STATUS_MASK) == ACP_POWERED_OFF) > + return 0; > + udelay(1); > + if (timeout > 500) {
same here, the timeout handling looks broken
> + pr_err("ACP is Not Powered OFF\n"); > + return -ETIMEDOUT; > + } > + } > +} > + > + > +static int acp3x_reset(void __iomem *acp3x_base) > +{ > + u32 val, timeout; > + > + rv_writel(1, acp3x_base + mmACP_SOFT_RESET); > + timeout = 0; > + while (++timeout < 100) { > + val = rv_readl(acp3x_base + mmACP_SOFT_RESET); > + if ((val & ACP3x_SOFT_RESET__SoftResetAudDone_MASK) || > + timeout > 100) {
and here, how can timeout > 100 if the outer loops controls it.
> + if (val & ACP3x_SOFT_RESET__SoftResetAudDone_MASK) > + break; > + return -ENODEV; > + } > + cpu_relax(); > + } > + rv_writel(0, acp3x_base + mmACP_SOFT_RESET); > + timeout = 0; > + while (++timeout < 100) { > + val = rv_readl(acp3x_base + mmACP_SOFT_RESET); > + if (!val) > + break; > + if (timeout > 100)
same here
> +static int snd_acp3x_suspend(struct device *dev) extra spaces?
> +{ > + int status; > + struct acp3x_dev_data *adata = dev_get_drvdata(dev); > + > + status = acp3x_deinit(adata->acp3x_base); > + if (status) > + dev_err(dev, "ACP de-init failed\n"); > + else > + dev_info(dev, "ACP de-initialized\n"); > + > + return 0; > +} > +static int snd_acp3x_resume(struct device *dev)
extra spaces?
| |