Re: [PATCH 1/8] ASoC: SOF: amd: Fix for incorrect acp error satus register offset

["Mukunda,Vijendar" <vijendar.mukunda@xxxxxxx>]

On 07/08/24 10:43, Vijendar Mukunda wrote:
> Adding 'dsp_intr_base' to ACP error status register offset in irq handler
> points to wrong register offset. ACP error status register offset got
> changed from ACP 6.0 onwards. Add 'acp_error_stat' descriptor field and
> update the value based on the ACP variant.

Please ignore this patch series. Will split out fixes and refactoring
patches (which has patch dependencies) and will send it
separately.
>
> Fixes: 0e44572a28a4 ("ASoC: SOF: amd: Add helper callbacks for ACP's DMA configuration")
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> ---
>  sound/soc/sof/amd/acp-dsp-offset.h | 3 ++-
>  sound/soc/sof/amd/acp.c            | 5 +++--
>  sound/soc/sof/amd/acp.h            | 1 +
>  sound/soc/sof/amd/pci-acp63.c      | 1 +
>  sound/soc/sof/amd/pci-rmb.c        | 1 +
>  sound/soc/sof/amd/pci-rn.c         | 1 +
>  6 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/sound/soc/sof/amd/acp-dsp-offset.h b/sound/soc/sof/amd/acp-dsp-offset.h
> index 59afbe2e0f42..66968efda869 100644
> --- a/sound/soc/sof/amd/acp-dsp-offset.h
> +++ b/sound/soc/sof/amd/acp-dsp-offset.h
> @@ -76,7 +76,8 @@
>  #define DSP_SW_INTR_CNTL_OFFSET			0x0
>  #define DSP_SW_INTR_STAT_OFFSET			0x4
>  #define DSP_SW_INTR_TRIG_OFFSET			0x8
> -#define ACP_ERROR_STATUS			0x18C4
> +#define ACP3X_ERROR_STATUS			0x18C4
> +#define ACP6X_ERROR_STATUS			0x1A4C
>  #define ACP3X_AXI2DAGB_SEM_0			0x1880
>  #define ACP5X_AXI2DAGB_SEM_0			0x1884
>  #define ACP6X_AXI2DAGB_SEM_0			0x1874
> diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
> index 7b122656efd1..d0b7d1c54248 100644
> --- a/sound/soc/sof/amd/acp.c
> +++ b/sound/soc/sof/amd/acp.c
> @@ -92,6 +92,7 @@ static int config_dma_channel(struct acp_dev_data *adata, unsigned int ch,
>  			      unsigned int idx, unsigned int dscr_count)
>  {
>  	struct snd_sof_dev *sdev = adata->dev;
> +	const struct sof_amd_acp_desc *desc = get_chip_info(sdev->pdata);
>  	unsigned int val, status;
>  	int ret;
>  
> @@ -102,7 +103,7 @@ static int config_dma_channel(struct acp_dev_data *adata, unsigned int ch,
>  					    val & (1 << ch), ACP_REG_POLL_INTERVAL,
>  					    ACP_REG_POLL_TIMEOUT_US);
>  	if (ret < 0) {
> -		status = snd_sof_dsp_read(sdev, ACP_DSP_BAR, ACP_ERROR_STATUS);
> +		status = snd_sof_dsp_read(sdev, ACP_DSP_BAR, desc->acp_error_stat);
>  		val = snd_sof_dsp_read(sdev, ACP_DSP_BAR, ACP_DMA_ERR_STS_0 + ch * sizeof(u32));
>  
>  		dev_err(sdev->dev, "ACP_DMA_ERR_STS :0x%x ACP_ERROR_STATUS :0x%x\n", val, status);
> @@ -404,7 +405,7 @@ static irqreturn_t acp_irq_handler(int irq, void *dev_id)
>  		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->ext_intr_stat, ACP_ERROR_IRQ_MASK);
>  		snd_sof_dsp_write(sdev, ACP_DSP_BAR, base + ACP_SW0_I2S_ERROR_REASON, 0);
>  		snd_sof_dsp_write(sdev, ACP_DSP_BAR, base + ACP_SW1_I2S_ERROR_REASON, 0);
> -		snd_sof_dsp_write(sdev, ACP_DSP_BAR, base + ACP_ERROR_STATUS, 0);
> +		snd_sof_dsp_write(sdev, ACP_DSP_BAR, desc->acp_error_stat, 0);
>  		irq_flag = 1;
>  	}
>  
> diff --git a/sound/soc/sof/amd/acp.h b/sound/soc/sof/amd/acp.h
> index ec9170b3f068..6ac853ff6093 100644
> --- a/sound/soc/sof/amd/acp.h
> +++ b/sound/soc/sof/amd/acp.h
> @@ -203,6 +203,7 @@ struct sof_amd_acp_desc {
>  	u32 probe_reg_offset;
>  	u32 reg_start_addr;
>  	u32 reg_end_addr;
> +	u32 acp_error_stat;
>  	u32 sdw_max_link_count;
>  	u64 sdw_acpi_dev_addr;
>  };
> diff --git a/sound/soc/sof/amd/pci-acp63.c b/sound/soc/sof/amd/pci-acp63.c
> index 54d42f83ce9e..c3da70549995 100644
> --- a/sound/soc/sof/amd/pci-acp63.c
> +++ b/sound/soc/sof/amd/pci-acp63.c
> @@ -35,6 +35,7 @@ static const struct sof_amd_acp_desc acp63_chip_info = {
>  	.ext_intr_cntl = ACP6X_EXTERNAL_INTR_CNTL,
>  	.ext_intr_stat	= ACP6X_EXT_INTR_STAT,
>  	.ext_intr_stat1	= ACP6X_EXT_INTR_STAT1,
> +	.acp_error_stat = ACP6X_ERROR_STATUS,
>  	.dsp_intr_base	= ACP6X_DSP_SW_INTR_BASE,
>  	.sram_pte_offset = ACP6X_SRAM_PTE_OFFSET,
>  	.hw_semaphore_offset = ACP6X_AXI2DAGB_SEM_0,
> diff --git a/sound/soc/sof/amd/pci-rmb.c b/sound/soc/sof/amd/pci-rmb.c
> index 4bc30951f8b0..194b7ff37e9e 100644
> --- a/sound/soc/sof/amd/pci-rmb.c
> +++ b/sound/soc/sof/amd/pci-rmb.c
> @@ -33,6 +33,7 @@ static const struct sof_amd_acp_desc rembrandt_chip_info = {
>  	.pgfsm_base	= ACP6X_PGFSM_BASE,
>  	.ext_intr_stat	= ACP6X_EXT_INTR_STAT,
>  	.dsp_intr_base	= ACP6X_DSP_SW_INTR_BASE,
> +	.acp_error_stat = ACP6X_ERROR_STATUS,
>  	.sram_pte_offset = ACP6X_SRAM_PTE_OFFSET,
>  	.hw_semaphore_offset = ACP6X_AXI2DAGB_SEM_0,
>  	.fusion_dsp_offset = ACP6X_DSP_FUSION_RUNSTALL,
> diff --git a/sound/soc/sof/amd/pci-rn.c b/sound/soc/sof/amd/pci-rn.c
> index e08875bdfa8b..bff2d979ea6a 100644
> --- a/sound/soc/sof/amd/pci-rn.c
> +++ b/sound/soc/sof/amd/pci-rn.c
> @@ -33,6 +33,7 @@ static const struct sof_amd_acp_desc renoir_chip_info = {
>  	.pgfsm_base	= ACP3X_PGFSM_BASE,
>  	.ext_intr_stat	= ACP3X_EXT_INTR_STAT,
>  	.dsp_intr_base	= ACP3X_DSP_SW_INTR_BASE,
> +	.acp_error_stat = ACP3X_ERROR_STATUS,
>  	.sram_pte_offset = ACP3X_SRAM_PTE_OFFSET,
>  	.hw_semaphore_offset = ACP3X_AXI2DAGB_SEM_0,
>  	.acp_clkmux_sel	= ACP3X_CLKMUX_SEL,