Re: [PATCH v6 01/10] ASoC: mediatek: common: modify mtk afe platform driver for mt8196
From: Chen-Yu Tsai <wenst@chromium.org>
Date: 2025-07-22 07:38:18
Also in:
linux-devicetree, linux-gpio, linux-mediatek, linux-sound, lkml
Hi, On Tue, Jul 8, 2025 at 7:33 PM Darren.Ye [off-list ref] wrote:
From: Darren Ye <redacted> Mofify the pcm pointer interface to support 64-bit address access.
^ Modify
And the subject should say the same, or shorter:
Support 64-bit addresses in PCM pointer callback
quoted hunk ↗ jump to hunk
Signed-off-by: Darren Ye <redacted> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Tested-by: Louis-Alexis Eyraud <redacted> --- .../mediatek/common/mtk-afe-platform-driver.c | 47 ++++++++++++------- .../mediatek/common/mtk-afe-platform-driver.h | 2 + 2 files changed, 33 insertions(+), 16 deletions(-)diff --git a/sound/soc/mediatek/common/mtk-afe-platform-driver.c b/sound/soc/mediatek/common/mtk-afe-platform-driver.c index 70fd05d5ff48..cab4ef035199 100644 --- a/sound/soc/mediatek/common/mtk-afe-platform-driver.c +++ b/sound/soc/mediatek/common/mtk-afe-platform-driver.c@@ -86,29 +86,44 @@ snd_pcm_uframes_t mtk_afe_pcm_pointer(struct snd_soc_component *component, const struct mtk_base_memif_data *memif_data = memif->data; struct regmap *regmap = afe->regmap; struct device *dev = afe->dev; - int reg_ofs_base = memif_data->reg_ofs_base; - int reg_ofs_cur = memif_data->reg_ofs_cur; - unsigned int hw_ptr = 0, hw_base = 0; - int ret, pcm_ptr_bytes; - - ret = regmap_read(regmap, reg_ofs_cur, &hw_ptr); - if (ret || hw_ptr == 0) { - dev_err(dev, "%s hw_ptr err\n", __func__); - pcm_ptr_bytes = 0; + unsigned int hw_ptr_lower32 = 0, hw_ptr_upper32 = 0; + unsigned int hw_base_lower32 = 0, hw_base_upper32 = 0; + unsigned long long hw_ptr = 0, hw_base = 0; + int ret; + unsigned long long pcm_ptr_bytes = 0; + + ret = regmap_read(regmap, memif_data->reg_ofs_cur, &hw_ptr_lower32); + if (ret || hw_ptr_lower32 == 0) {
I'm not sure how the hardware is, but I think hw_ptr_lower32 == 0 should no longer be a failure, since it could be 0x100000000 or some other address with the lower 32-bits all zero. Instead the check should be done once the addresses are put together.
+ dev_err(dev, "%s hw_ptr_lower32 err\n", __func__);
goto POINTER_RETURN_FRAMES;This is out of scope, but maybe this should just return zero directly to simplify reading.
}
- ret = regmap_read(regmap, reg_ofs_base, &hw_base);
- if (ret || hw_base == 0) {
- dev_err(dev, "%s hw_ptr err\n", __func__);
- pcm_ptr_bytes = 0;
- goto POINTER_RETURN_FRAMES;
+ if (memif_data->reg_ofs_cur_msb) {
+ ret = regmap_read(regmap, memif_data->reg_ofs_cur_msb, &hw_ptr_upper32);
+ if (ret) {
+ dev_err(dev, "%s hw_ptr_upper32 err\n", __func__);
+ goto POINTER_RETURN_FRAMES;
+ }
}
- pcm_ptr_bytes = hw_ptr - hw_base;
+ ret = regmap_read(regmap, memif_data->reg_ofs_base, &hw_base_lower32);
+ if (ret || hw_base_lower32 == 0) {Same here.
+ dev_err(dev, "%s hw_base_lower32 err\n", __func__);
+ goto POINTER_RETURN_FRAMES;
+ }
+ if (memif_data->reg_ofs_base_msb) {
+ ret = regmap_read(regmap, memif_data->reg_ofs_base_msb, &hw_base_upper32);
+ if (ret) {
+ dev_err(dev, "%s hw_base_upper32 err\n", __func__);
+ goto POINTER_RETURN_FRAMES;
+ }
+ }
+ hw_ptr = ((unsigned long long)hw_ptr_upper32 << 32) + hw_ptr_lower32;
+ hw_base = ((unsigned long long)hw_base_upper32 << 32) + hw_base_lower32;Instead the check should be here. And to follow the original logic, if either pointer value is zero, the function should return zero here directly. ChenYu
quoted hunk ↗ jump to hunk
POINTER_RETURN_FRAMES: - return bytes_to_frames(substream->runtime, pcm_ptr_bytes); + pcm_ptr_bytes = MTK_ALIGN_16BYTES(hw_ptr - hw_base); + return bytes_to_frames(substream->runtime, (ssize_t)pcm_ptr_bytes); } EXPORT_SYMBOL_GPL(mtk_afe_pcm_pointer);diff --git a/sound/soc/mediatek/common/mtk-afe-platform-driver.h b/sound/soc/mediatek/common/mtk-afe-platform-driver.h index fcc923b88f12..71070b26f8f8 100644 --- a/sound/soc/mediatek/common/mtk-afe-platform-driver.h +++ b/sound/soc/mediatek/common/mtk-afe-platform-driver.h@@ -12,6 +12,8 @@ #define AFE_PCM_NAME "mtk-afe-pcm" extern const struct snd_soc_component_driver mtk_afe_pcm_platform; +#define MTK_ALIGN_16BYTES(x) ((x) & GENMASK_ULL(39, 4)) + struct mtk_base_afe; struct snd_pcm; struct snd_soc_component; --2.45.2