Re: [PATCH RESEND v5 04/16] ASoC: mediatek: mt8365: Add common header

[AngeloGioacchino Del Regno]

Il 14/06/24 09:27, Alexandre Mergnat ha scritto:
> Add header files for register definition and structure.
>
> Signed-off-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
> ---
>   sound/soc/mediatek/mt8365/mt8365-afe-common.h | 491 +++++++++++++
>   sound/soc/mediatek/mt8365/mt8365-reg.h        | 991 ++++++++++++++++++++++++++
>   2 files changed, 1482 insertions(+)
>
> diff --git a/sound/soc/mediatek/mt8365/mt8365-afe-common.h b/sound/soc/mediatek/mt8365/mt8365-afe-common.h
> new file mode 100644
> index 000000000000..4d8f8c4b19e3
> --- /dev/null
> +++ b/sound/soc/mediatek/mt8365/mt8365-afe-common.h
> @@ -0,0 +1,491 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Mediatek 8365 audio driver common definitions
> + *
> + * Copyright (c) 2024 MediaTek Inc.
> + * Authors: Jia Zeng <jia.zeng@xxxxxxxxxxxx>
> + *          Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
> + */
> +
> +#ifndef _MT8365_AFE_COMMON_H_
> +#define _MT8365_AFE_COMMON_H_
> +
> +#define COMMON_CLOCK_FRAMEWORK_API
> +#define IDLE_TASK_DRIVER_API
> +#define ENABLE_AFE_APLL_TUNER

Those three definitions do not exist upstream. Please remove.

> +
> +#include <linux/clk.h>
> +#include <linux/list.h>
> +#include <linux/regmap.h>
> +#include <sound/soc.h>
> +#include <sound/asound.h>
> +#include "../common/mtk-base-afe.h"
> +#include "mt8365-reg.h"
> +
> +#define ENUM_TO_STR(enum) #enum

Unused definition

> +
> +#define snd_soc_dai_stream_active_playback(dai) \
> +		snd_soc_dai_stream_active(dai, SNDRV_PCM_STREAM_PLAYBACK)
> +#define snd_soc_dai_stream_active_capture(dai) \
> +		snd_soc_dai_stream_active(dai, SNDRV_PCM_STREAM_CAPTURE)
> +

Those are used only once and only in mt8365-dai-pcm.c, and I just noticed that.

Can you please just remove those two and directly call snd_soc_dai_stream_active()
with the right params in function mt8365_dai_pcm1_prepare()?

> +enum {
> +	MT8365_AFE_MEMIF_DL1,
> +	MT8365_AFE_MEMIF_DL2,
> +	MT8365_AFE_MEMIF_TDM_OUT,

..snip..

> +
> +#ifdef CONFIG_MTK_HIFIXDSP_SUPPORT

This configuration option doesn't exist.

Please remove the ifdef and the enclosed code entirely, as it's unused.

> +struct mt8365_adsp_data {
> +	/* information adsp supply */
> +	bool adsp_on;
> +	int (*hostless_active)(void);
> +	/* information afe supply */
> +	int (*set_afe_memif)(struct mtk_base_afe *afe,
> +			     int memif_id,
> +			     unsigned int rate,
> +			     unsigned int channels,
> +			     snd_pcm_format_t format);
> +	int (*set_afe_memif_enable)(struct mtk_base_afe *afe,
> +				    int memif_id,
> +				    unsigned int rate,
> +				    unsigned int period_size,
> +				    int enable);
> +	void (*get_afe_memif_sram)(struct mtk_base_afe *afe,
> +				   int memif_id,
> +				   unsigned int *paddr,
> +				   unsigned int *size);
> +	void (*set_afe_init)(struct mtk_base_afe *afe);
> +	void (*set_afe_uninit)(struct mtk_base_afe *afe);
> +};
> +#endif
> +
> +struct mt8365_afe_private {
> +	struct clk *clocks[MT8365_CLK_NUM];
> +	struct regmap *topckgen;
> +	struct mt8365_fe_dai_data fe_data[MT8365_AFE_MEMIF_NUM];
> +	struct mt8365_be_dai_data be_data[MT8365_AFE_BACKEND_NUM];
> +	struct mt8365_control_data ctrl_data;
> +	struct mt8365_gasrc_data gasrc_data[MT8365_TDM_ASRC_NUM];
> +#ifdef CONFIG_MTK_HIFIXDSP_SUPPORT
> +	struct mt8365_adsp_data adsp_data;

ditto

> +#endif
> +	int afe_on_ref_cnt;
> +	int top_cg_ref_cnt[MT8365_TOP_CG_NUM];
> +	void __iomem *afe_sram_vir_addr;
> +	unsigned int afe_sram_phy_addr;
> +	unsigned int afe_sram_size;
> +	/* locks */
> +	spinlock_t afe_ctrl_lock;
> +	struct mutex afe_clk_mutex;	/* Protect & sync APLL TUNER registers access*/
> +#ifdef CONFIG_DEBUG_FS
> +	struct dentry *debugfs_dentry[MT8365_AFE_DEBUGFS_NUM];
> +#endif
> +	int apll_tuner_ref_cnt[MT8365_AFE_APLL_NUM];
> +	unsigned int tdm_out_mode;
> +	unsigned int cm2_mux_input;
> +
> +	/* dai */
> +	bool dai_on[MT8365_AFE_BACKEND_END];
> +	void *dai_priv[MT8365_AFE_BACKEND_END];
> +};
> +

....

> +#ifdef CONFIG_MTK_HIFIXDSP_SUPPORT

same

> +struct mtk_base_afe *mt8365_afe_pcm_get_info(void);
> +#endif
> +
> +int mt8365_dai_i2s_register(struct mtk_base_afe *afe);
> +int mt8365_dai_set_priv(struct mtk_base_afe *afe,
> +			int id,
> +			int priv_size,
> +			const void *priv_data);
> +

Everything else looks good.

After applying the proposed cleanups,
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>

Cheers,
Angelo