Re: [PATCH 8/11] ASoC: amd: acp: add ACP6x SoundWire quirks and machine support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: amd: ACP6X_REG_END truncates MMIO range needed by acp63_config_dma()
--

commit d2bc6f9f5ef056a066db2b434d5d1ed6b31dee36
Author: Corey Smith <corsmith@gmail.com>

ASoC: amd: acp: add ACP6x SoundWire quirks and machine support

This commit adds ACP6x support to the generic AMD SoundWire machine driver by
reusing the ACP6.3 CPU pin mapping and the amd_ps_sdw_dma and acp_ps_pdm_dma
platform components. It also adds a DMI quirk for the HP Dragonfly Pro ONE
(board 8A7F).

> diff --git a/sound/soc/amd/acp/acp-sdw-legacy-mach.c b/sound/soc/amd/acp/acp-sdw-legacy-mach.c
> index a8b62817b2344..69880de6b99f7 100644
> --- a/sound/soc/amd/acp/acp-sdw-legacy-mach.c
> +++ b/sound/soc/amd/acp/acp-sdw-legacy-mach.c

[ ... ]

> @@ -337,6 +353,8 @@ static int create_sdw_dailinks(struct snd_soc_card *card,
>  		return -ENOMEM;
>  
>  	switch (amd_ctx->acp_rev) {
> +	case ACP6X_PCI_REV:
> +	case ACP6XR_PCI_REV:
>  	case ACP63_PCI_REV:
>  	case ACP70_PCI_REV:
>  	case ACP71_PCI_REV:

[Severity: Medium]
When create_sdw_dailinks() binds ACP6X_PCI_REV and ACP6XR_PCI_REV SoundWire
DAI links to amd_ps_sdw_dma.0, does acp63_sdw_dma_hw_params() ->
acp63_config_dma() end up writing SoundWire PTEs beyond the MMIO resource
range configured for ACP6x?

In sound/soc/amd/ps/acp63.h, ACP6X_REG_END is defined at offset 0x10200 from
ACP6X_REG_START:

    #define ACP6X_REG_START		0x1240000
    #define ACP6X_REG_END		0x1250200
    #define ACP63_REG_START		0x1240000
    #define ACP63_REG_END		0x125C000

In sound/soc/amd/ps/pci-ps.c, snd_acp63_probe() uses this range for ACP6x,
and create_acp63_platform_devs() passes it as the IORESOURCE_MEM resource to
the child platform devices:

    if (pci->revision == ACP6X_PCI_REV || pci->revision == ACP6XR_PCI_REV)
            adata->reg_range = ACP6X_REG_END - ACP6X_REG_START;
    else
            adata->reg_range = ACP63_REG_END - ACP63_REG_START;

acp63_sdw_platform_probe() in sound/soc/amd/ps/ps-sdw-dma.c then maps that
resource via devm_ioremap(&pdev->dev, res->start, resource_size(res)), which
only covers 0x10201 bytes (up to ACP_SCRATCH_REG_0 + 0x200).

However, when a SoundWire PCM substream is configured, acp63_config_dma() in
sound/soc/amd/ps/ps-sdw-dma.c writes PTEs into scratch memory at:

    sdw_dma_pte_offset = SDW_PTE_OFFSET(stream->instance);
    val = sdw_dma_pte_offset + (stream_id * ACP_SDW_PTE_OFFSET);
    ...
    writel(low, acp_base + ACP_SCRATCH_REG_0 + val);
    high |= BIT(31);
    writel(high, acp_base + ACP_SCRATCH_REG_0 + val + 4);

Since ACP_SCRATCH_REG_0 is 0x10000 and SDW_PTE_OFFSET(stream->instance) is
0x400 for SDW0 and 0xa00 for SDW1, these writes span offsets 0x10400..0x10b7f,
which is past the 0x10200 limit set by ACP6X_REG_END (unlike the 0x125C000
end address used in sound/soc/amd/acp/acp-pci.c and
sound/soc/sof/amd/pci-rmb.c).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028020552.35278-1-corsmith@gmail.com?part=8