Re: [PATCH 3/11] ASoC: amd: ps: add ACP6x PCI revision ids

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: amd: ps: ACP6X_REG_END truncates SoundWire DMA scratch PTE range
--

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

ASoC: amd: ps: add ACP6x PCI revision ids

This commit adds ACP6x (Rembrandt) PCI revision IDs, the ACP6x register
range, and the SoundWire controller ACPI address to the Pink Sardine
header so the driver can bind to ACP6x devices.

> diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h
> index d0cfbd4dafc17..b7d2666b27fc7 100644
> --- a/sound/soc/amd/ps/acp63.h
> +++ b/sound/soc/amd/ps/acp63.h
> @@ -9,8 +9,12 @@
>  #include <sound/acp63_chip_offset_byte.h>
>  
>  #define ACP_DEVICE_ID 0x15E2
> +#define ACP6X_REG_START		0x1240000
> +#define ACP6X_REG_END		0x1250200

[Severity: Medium]
Should ACP6X_REG_END be 0x125C000 instead of 0x1250200?

Defining ACP6X_REG_END as 0x1250200 (matching the PDM-only Yellow Carp
header in sound/soc/amd/yc/acp6x.h) sizes the ACP6x register range to
0x10200 bytes. When snd_acp63_probe() sets adata->reg_range to
ACP6X_REG_END - ACP6X_REG_START, create_acp63_platform_devs() creates the
IORESOURCE_MEM resource for amd_ps_sdw_dma with a size of 0x10201 bytes,
which acp63_sdw_platform_probe() maps via devm_ioremap().

However, SoundWire DMA page table entries in ACP scratch memory start past
0x10200:

sound/soc/amd/ps/acp63.h:
    #define SDW0_PTE_OFFSET		0x400
    ...
    #define ACP_SDW_PTE_OFFSET		0x100
    #define SDW_PTE_OFFSET(i)		(SDW0_PTE_OFFSET + ((i) * 0x600))

When acp63_config_dma() in sound/soc/amd/ps/ps-sdw-dma.c programs the
SoundWire DMA PTEs during PCM hw_params or resume, it writes to offsets
0x10400 through 0x10B80 past acp_base (since ACP_SCRATCH_REG_0 is 0x10000):

sound/soc/amd/ps/ps-sdw-dma.c:acp63_config_dma() {
    ...
    sdw_dma_pte_offset = SDW_PTE_OFFSET(stream->instance);
    val = sdw_dma_pte_offset + (stream_id * ACP_SDW_PTE_OFFSET);
    ...
    for (page_idx = 0; page_idx < stream->num_pages; page_idx++) {
        ...
        writel(low, acp_base + ACP_SCRATCH_REG_0 + val);
        high |= BIT(31);
        writel(high, acp_base + ACP_SCRATCH_REG_0 + val + 4);
    ...
}

This accesses MMIO offsets beyond the declared resource and devm_ioremap()
size. Note that commit 253ce07d2a09 ("ASoC: amd: ps: modify ACP register
end address macro") previously updated ACP63_REG_END from 0x1250200 to
0x125C000 for this reason, and sound/soc/sof/amd/pci-rmb.c also defines
ACP6x_REG_END as 0x125C000.

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