Re: [PATCH 3/5] ASoC: amd: acp7x: add helper functions and hw ops

[Mario Limonciello <superm1@kernel.org>]

On 5/7/26 13:11, Vijendar Mukunda wrote:
> Add ACP7.x init/deinit helper routines and wire up PCI hw ops for
> ACP7.D/7.E/7.F variants.
> 
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>   sound/soc/amd/acp7x/acp7x-common.c | 82 ++++++++++++++++++++++++++++++
>   sound/soc/amd/acp7x/acp7x.h        | 78 +++++++++++++++++++++++++++-
>   sound/soc/amd/acp7x/pci-acp7x.c    | 39 +++++++++++++-
>   3 files changed, 197 insertions(+), 2 deletions(-)
>   create mode 100644 sound/soc/amd/acp7x/acp7x-common.c
> 
> diff --git a/sound/soc/amd/acp7x/acp7x-common.c b/sound/soc/amd/acp7x/acp7x-common.c
> new file mode 100644
> index 000000000000..68f9b47766c4
> --- /dev/null
> +++ b/sound/soc/amd/acp7x/acp7x-common.c
> @@ -0,0 +1,82 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AMD ACP PCI driver callback routines for ACP7.x
> + * platforms.
> + *
> + * Copyright 2026 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/types.h>
> +
> +#include "acp7x.h"
> +
> +static int acp7x_power_on(void __iomem *acp_base)
> +{
> +	u32 val = 0;
> +
> +	val = readl(acp_base + ACP_PGFSM_STATUS);
> +	if (!(val & ACP7X_PGFSM_STATUS_MASK))
> +		return 0;
> +
> +	writel(ACP7X_PGFSM_CNTL_POWER_ON_MASK, acp_base + ACP_PGFSM_CONTROL);
> +	val = readl(acp_base + ACP_PGFSM_CONTROL);
> +	return readl_poll_timeout(acp_base + ACP_PGFSM_STATUS, val,
> +				  ((val & ACP7X_PGFSM_STATUS_MASK) == 0), DELAY_US, ACP7X_TIMEOUT);
> +}
> +
> +static int acp7x_reset(void __iomem *acp_base)
> +{
> +	u32 val;
> +	int ret;
> +
> +	writel(1, acp_base + ACP_SOFT_RESET);
> +	ret = readl_poll_timeout(acp_base + ACP_SOFT_RESET, val,
> +				 val & ACP_SOFT_RESET_SOFTRESET_AUDDONE_MASK,
> +				 DELAY_US, ACP7X_TIMEOUT);
> +	if (ret)
> +		return ret;
> +
> +	writel(0, acp_base + ACP_SOFT_RESET);
> +	return readl_poll_timeout(acp_base + ACP_SOFT_RESET, val, !val, DELAY_US, ACP7X_TIMEOUT);
> +}
> +
> +static int acp7x_init(void __iomem *acp_base, struct device *dev)
> +{
> +	int ret;
> +
> +	ret = acp7x_power_on(acp_base);
> +	if (ret) {
> +		dev_err(dev, "ACP power on failed\n");
> +		return ret;
> +	}
> +	writel(0x01, acp_base + ACP_CONTROL);
> +	ret = acp7x_reset(acp_base);
> +	if (ret) {
> +		dev_err(dev, "ACP reset failed\n");
> +		return ret;
> +	}
> +	writel(0, acp_base + ACP_ZSC_DSP_CTRL);
> +	return 0;
> +}
> +
> +static int acp7x_deinit(void __iomem *acp_base, struct device *dev)
> +{
> +	int ret;
> +
> +	ret = acp7x_reset(acp_base);
> +	if (ret) {
> +		dev_err(dev, "ACP reset failed\n");
> +		return ret;
> +	}
> +	writel(0x01, acp_base + ACP_ZSC_DSP_CTRL);
> +	return 0;
> +}
> +
> +void acp7x_hw_init_ops(struct acp_hw_ops *hw_ops)
> +{
> +	hw_ops->acp_init = acp7x_init;
> +	hw_ops->acp_deinit = acp7x_deinit;
> +}
> diff --git a/sound/soc/amd/acp7x/acp7x.h b/sound/soc/amd/acp7x/acp7x.h
> index b4586a2afae4..d3a57705385a 100644
> --- a/sound/soc/amd/acp7x/acp7x.h
> +++ b/sound/soc/amd/acp7x/acp7x.h
> @@ -8,6 +8,8 @@
>   #ifndef __SOUND_SOC_AMD_ACP7X_H
>   #define __SOUND_SOC_AMD_ACP7X_H
>   
> +#include <linux/device.h>
> +#include <linux/errno.h>
>   #include <linux/io.h>
>   #include <linux/pci.h>
>   #include <linux/types.h>
> @@ -22,10 +24,84 @@
>   #define ACP7E_PCI_REV		0x7E
>   #define ACP7F_PCI_REV		0x7F
>   
> -int snd_amd_acp_find_config(struct pci_dev *pci);
> +/* Common register helper bits used by acp7x-common.c */
> +#define ACP_SOFT_RESET_SOFTRESET_AUDDONE_MASK	0x00010001
> +
> +#define DELAY_US				5
> +#define ACP7X_TIMEOUT				5000
> +
> +#define ACP7X_PGFSM_CNTL_POWER_ON_MASK		7
> +#define ACP7X_PGFSM_STATUS_MASK			0x3F
> +
> +struct acp_hw_ops {
> +	int (*acp_init)(void __iomem *acp_base, struct device *dev);
> +	int (*acp_deinit)(void __iomem *acp_base, struct device *dev);
> +	int (*acp_suspend)(struct device *dev);
> +	int (*acp_resume)(struct device *dev);
> +	int (*acp_suspend_runtime)(struct device *dev);
> +	int (*acp_resume_runtime)(struct device *dev);
> +};
>   
>   struct acp7x_dev_data {
>   	void __iomem *acp7x_base;
> +	struct acp_hw_ops *hw_ops;
> +	u32 addr;
> +	u32 reg_range;
> +	u32 acp_rev;
>   };
>   
> +void acp7x_hw_init_ops(struct acp_hw_ops *hw_ops);
> +
> +static inline int acp_hw_init(struct acp7x_dev_data *adata, struct device *dev)
> +{
> +	if (adata && adata->hw_ops && adata->hw_ops->acp_init)
> +		return adata->hw_ops->acp_init(adata->acp7x_base, dev);
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int acp_hw_deinit(struct acp7x_dev_data *adata, struct device *dev)
> +{
> +	if (adata && adata->hw_ops && adata->hw_ops->acp_deinit)
> +		return adata->hw_ops->acp_deinit(adata->acp7x_base, dev);
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int acp_hw_suspend(struct device *dev)
> +{
> +	struct acp7x_dev_data *adata = dev_get_drvdata(dev);
> +
> +	if (adata && adata->hw_ops && adata->hw_ops->acp_suspend)
> +		return adata->hw_ops->acp_suspend(dev);
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int acp_hw_resume(struct device *dev)
> +{
> +	struct acp7x_dev_data *adata = dev_get_drvdata(dev);
> +
> +	if (adata && adata->hw_ops && adata->hw_ops->acp_resume)
> +		return adata->hw_ops->acp_resume(dev);
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int acp_hw_suspend_runtime(struct device *dev)
> +{
> +	struct acp7x_dev_data *adata = dev_get_drvdata(dev);
> +
> +	if (adata && adata->hw_ops && adata->hw_ops->acp_suspend_runtime)
> +		return adata->hw_ops->acp_suspend_runtime(dev);
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int acp_hw_runtime_resume(struct device *dev)
> +{
> +	struct acp7x_dev_data *adata = dev_get_drvdata(dev);
> +
> +	if (adata && adata->hw_ops && adata->hw_ops->acp_resume_runtime)
> +		return adata->hw_ops->acp_resume_runtime(dev);
> +	return -EOPNOTSUPP;
> +}
> +
> +int snd_amd_acp_find_config(struct pci_dev *pci);
> +
>   #endif /* __SOUND_SOC_AMD_ACP7X_H */
> diff --git a/sound/soc/amd/acp7x/pci-acp7x.c b/sound/soc/amd/acp7x/pci-acp7x.c
> index 476b3ae52634..237eb06e3cad 100644
> --- a/sound/soc/amd/acp7x/pci-acp7x.c
> +++ b/sound/soc/amd/acp7x/pci-acp7x.c
> @@ -16,6 +16,26 @@
>   
>   #include "acp7x.h"
>   
> +static int acp_hw_init_ops(struct acp7x_dev_data *adata, struct pci_dev *pci)
> +{
> +	adata->hw_ops = devm_kzalloc(&pci->dev, sizeof(struct acp_hw_ops),
> +				     GFP_KERNEL);
> +	if (!adata->hw_ops)
> +		return -ENOMEM;
> +
> +	switch (adata->acp_rev) {
> +	case ACP7D_PCI_REV:
> +	case ACP7E_PCI_REV:
> +	case ACP7F_PCI_REV:
> +		acp7x_hw_init_ops(adata->hw_ops);
> +		break;
> +	default:
> +		dev_err(&pci->dev, "ACP device not found\n");
> +		return -ENODEV;
> +	}
> +	return 0;
> +}
> +
>   static int snd_acp7x_probe(struct pci_dev *pci,
>   			   const struct pci_device_id *pci_id)
>   {
> @@ -27,7 +47,6 @@ static int snd_acp7x_probe(struct pci_dev *pci,
>   	flag = snd_amd_acp_find_config(pci);
>   	if (flag)
>   		return -ENODEV;
> -
>   	/* ACP PCI revision id check for ACP7.x platforms */
>   	switch (pci->revision) {
>   	case ACP7D_PCI_REV:
> @@ -60,8 +79,19 @@ static int snd_acp7x_probe(struct pci_dev *pci,
>   		ret = -ENOMEM;
>   		goto release_regions;
>   	}
> +	adata->addr = addr;
> +	adata->reg_range = ACP7X_REG_END - ACP7X_REG_START;
> +	adata->acp_rev = pci->revision;
>   	pci_set_master(pci);
>   	pci_set_drvdata(pci, adata);
> +	ret = acp_hw_init_ops(adata, pci);
> +	if (ret) {
> +		dev_err(&pci->dev, "ACP hw ops init failed\n");
> +		goto release_regions;
> +	}
> +	ret = acp_hw_init(adata, &pci->dev);
> +	if (ret)
> +		goto release_regions;
>   	return 0;
>   
>   release_regions:
> @@ -74,6 +104,13 @@ static int snd_acp7x_probe(struct pci_dev *pci,
>   
>   static void snd_acp7x_remove(struct pci_dev *pci)
>   {
> +	struct acp7x_dev_data *adata;
> +	int ret;
> +
> +	adata = pci_get_drvdata(pci);
> +	ret = acp_hw_deinit(adata, &pci->dev);
> +	if (ret)
> +		dev_err(&pci->dev, "ACP de-init failed\n");
>   	pci_release_regions(pci);
>   	pci_disable_device(pci);
>   }