[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eca17001-93ff-d379-1ab2-2927f1831e78@linux.intel.com>
Date: Fri, 9 Dec 2022 09:52:32 -0600
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To: Ajye Huang <ajye_huang@...pal.corp-partner.google.com>,
linux-kernel@...r.kernel.org
Cc: Mark Brown <broonie@...nel.org>,
Bard Liao <yung-chuan.liao@...ux.intel.com>,
Peter Ujfalusi <peter.ujfalusi@...ux.intel.com>,
Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
Akihiko Odaki <akihiko.odaki@...il.com>,
Yong Zhi <yong.zhi@...el.com>,
ye xingchen <ye.xingchen@....com.cn>,
Muralidhar Reddy <muralidhar.reddy@...el.com>,
"balamurugan . c" <balamurugan.c@...el.com>,
Libin Yang <libin.yang@...el.com>,
Jaroslav Kysela <perex@...ex.cz>,
Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
Cezary Rojewski <cezary.rojewski@...el.com>,
David Lin <CTLIN0@...oton.com>, Brent Lu <brent.lu@...el.com>,
Takashi Iwai <tiwai@...e.com>,
Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
alsa-devel@...a-project.org
Subject: Re: [PATCH v1] ASoC: Intel: sof_nau8825: add support for nau8825 with
amp nau8318
On 12/9/22 09:05, Ajye Huang wrote:
> This patch adds the driver data for two nau8318 speaker amplifiers on
> SSP1 and nau8825 on SSP0 for ADL platform.
> And reusing max98360's topology since DAI setting could be leveraged.
>
> Signed-off-by: Ajye Huang <ajye_huang@...pal.corp-partner.google.com>
> ---
> sound/soc/intel/boards/Kconfig | 1 +
> sound/soc/intel/boards/sof_nau8825.c | 23 +++++++++++++++++++
> .../intel/common/soc-acpi-intel-adl-match.c | 12 ++++++++++
> 3 files changed, 36 insertions(+)
>
> diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
> index a472de1909f4..3f68e9edd853 100644
> --- a/sound/soc/intel/boards/Kconfig
> +++ b/sound/soc/intel/boards/Kconfig
> @@ -554,6 +554,7 @@ config SND_SOC_INTEL_SOF_NAU8825_MACH
> select SND_SOC_RT1015P
> select SND_SOC_MAX98373_I2C
> select SND_SOC_MAX98357A
> + select SND_SOC_NAU8315
This looks inconsistent with the commit message. There are separate
Kconfigs for different codecs.
SND_SOC_NAU8315
SND_SOC_NAU8825
Which is it?
> select SND_SOC_DMIC
> select SND_SOC_HDAC_HDMI
> select SND_SOC_INTEL_HDA_DSP_COMMON
> diff --git a/sound/soc/intel/boards/sof_nau8825.c b/sound/soc/intel/boards/sof_nau8825.c
> index 27880224359d..0936450be153 100644
> --- a/sound/soc/intel/boards/sof_nau8825.c
> +++ b/sound/soc/intel/boards/sof_nau8825.c
> @@ -48,6 +48,7 @@
> #define SOF_MAX98373_SPEAKER_AMP_PRESENT BIT(15)
> #define SOF_MAX98360A_SPEAKER_AMP_PRESENT BIT(16)
> #define SOF_RT1015P_SPEAKER_AMP_PRESENT BIT(17)
> +#define SOF_NAU8318_SPEAKER_AMP_PRESENT BIT(18)
>
> static unsigned long sof_nau8825_quirk = SOF_NAU8825_SSP_CODEC(0);
>
> @@ -338,6 +339,13 @@ static struct snd_soc_dai_link_component rt1019p_component[] = {
> }
> };
>
> +static struct snd_soc_dai_link_component nau8318_components[] = {
> + {
> + .name = "NVTN2012:00",
Deep sigh...
This ACPI HID is already used to match with the 8815, so this is not
going to work if we confuse two independent drivers...
sound/soc/codecs/nau8315.c:
#ifdef CONFIG_ACPI
static const struct acpi_device_id nau8315_acpi_match[] = {
{ "NVTN2010", 0 },
{ "NVTN2012", 0},
{},
};
MODULE_DEVICE_TABLE(acpi, nau8315_acpi_match);
#endif
How does this identify a NAU8825?
> + .dai_name = "nau8315-hifi",
and again this makes a reference to 8815.
I will stop the review here.
NAK for this v1. Please clarify which codec you are using and make sure
all references are consistent.
> + }
> +};
> +
> static struct snd_soc_dai_link_component dummy_component[] = {
> {
> .name = "snd-soc-dummy",
> @@ -486,6 +494,11 @@ static struct snd_soc_dai_link *sof_card_dai_links_create(struct device *dev,
> max_98360a_dai_link(&links[id]);
> } else if (sof_nau8825_quirk & SOF_RT1015P_SPEAKER_AMP_PRESENT) {
> sof_rt1015p_dai_link(&links[id]);
> + } else if (sof_nau8825_quirk &
> + SOF_NAU8318_SPEAKER_AMP_PRESENT) {
> + links[id].codecs = nau8318_components;
> + links[id].num_codecs = ARRAY_SIZE(nau8318_components);
> + links[id].init = speaker_codec_init;
> } else {
> goto devm_err;
> }
> @@ -657,6 +670,16 @@ static const struct platform_device_id board_ids[] = {
> SOF_BT_OFFLOAD_SSP(2) |
> SOF_SSP_BT_OFFLOAD_PRESENT),
> },
> + {
> + .name = "adl_nau8318_nau8825",
> + .driver_data = (kernel_ulong_t)(SOF_NAU8825_SSP_CODEC(0) |
> + SOF_SPEAKER_AMP_PRESENT |
> + SOF_NAU8318_SPEAKER_AMP_PRESENT |
> + SOF_NAU8825_SSP_AMP(1) |
> + SOF_NAU8825_NUM_HDMIDEV(4) |
> + SOF_BT_OFFLOAD_SSP(2) |
> + SOF_SSP_BT_OFFLOAD_PRESENT),
> + },
> { }
> };
> MODULE_DEVICE_TABLE(platform, board_ids);
> diff --git a/sound/soc/intel/common/soc-acpi-intel-adl-match.c b/sound/soc/intel/common/soc-acpi-intel-adl-match.c
> index 60aee56f94bd..282b9c966ce6 100644
> --- a/sound/soc/intel/common/soc-acpi-intel-adl-match.c
> +++ b/sound/soc/intel/common/soc-acpi-intel-adl-match.c
> @@ -450,6 +450,11 @@ static const struct snd_soc_acpi_codecs adl_lt6911_hdmi = {
> .codecs = {"INTC10B0"}
> };
>
> +static const struct snd_soc_acpi_codecs adl_nau8318_amp = {
> + .num_codecs = 1,
> + .codecs = {"NVTN2012"}
> +};
> +
> struct snd_soc_acpi_mach snd_soc_acpi_intel_adl_machines[] = {
> {
> .comp_ids = &adl_rt5682_rt5682s_hp,
> @@ -507,6 +512,13 @@ struct snd_soc_acpi_mach snd_soc_acpi_intel_adl_machines[] = {
> .quirk_data = &adl_rt1015p_amp,
> .sof_tplg_filename = "sof-adl-rt1015-nau8825.tplg",
> },
> + {
> + .id = "10508825",
> + .drv_name = "adl_nau8318_nau8825",
> + .machine_quirk = snd_soc_acpi_codec_list,
> + .quirk_data = &adl_nau8318_amp,
> + .sof_tplg_filename = "sof-adl-max98360a-nau8825.tplg",
> + },
> {
> .id = "10508825",
> .drv_name = "sof_nau8825",
Powered by blists - more mailing lists