Re: [alsa-devel] [PATCH v3 6/6] ASoC: amd: Added ACP3x system resume and runtime pm

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