[PATCH 0/2] ASoC: Intel: avs: Add support for rt5514 codec

[PATCH 0/2] ASoC: Intel: avs: Add support for rt5514 codec

[Amadeusz Sławiński]

There are machines which use codec rt5514 as DMIC, add support for them.

Amadeusz Sławiński (2):
  ASoC: Intel: avs: Add rt5514 machine board
  ASoC: Intel: avs: Add rt5514 machine board

 sound/soc/intel/avs/board_selection.c |   9 ++
 sound/soc/intel/avs/boards/Kconfig    |  10 ++
 sound/soc/intel/avs/boards/Makefile   |   2 +
 sound/soc/intel/avs/boards/rt5514.c   | 187 ++++++++++++++++++++++++++
 4 files changed, 208 insertions(+)
 create mode 100644 sound/soc/intel/avs/boards/rt5514.c


base-commit: 00236a89602f7b67016bd1d9bc63079f2c0f0a8d
-- 
2.34.1

[PATCH 1/2] ASoC: Intel: avs: Add rt5514 machine board

[Amadeusz Sławiński]

To support AVS-rt5514 configuration add machine board connecting AVS
platform component driver with rt5514 codec one.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/avs/boards/Kconfig  |  10 ++
 sound/soc/intel/avs/boards/Makefile |   2 +
 sound/soc/intel/avs/boards/rt5514.c | 187 ++++++++++++++++++++++++++++
 3 files changed, 199 insertions(+)
 create mode 100644 sound/soc/intel/avs/boards/rt5514.c

diff --git a/sound/soc/intel/avs/boards/Kconfig b/sound/soc/intel/avs/boards/Kconfig
index 07353d37ecae..00b0f6c176d6 100644
--- a/sound/soc/intel/avs/boards/Kconfig
+++ b/sound/soc/intel/avs/boards/Kconfig
@@ -125,6 +125,16 @@ config SND_SOC_INTEL_AVS_MACH_RT298
 	   Say Y or m if you have such a device. This is a recommended option.
 	   If unsure select "N".
 
+config SND_SOC_INTEL_AVS_MACH_RT5514
+	tristate "rt5514 in I2S mode"
+	depends on I2C
+	depends on MFD_INTEL_LPSS || COMPILE_TEST
+	select SND_SOC_RT5514
+	help
+	   This adds support for ASoC machine driver with RT5514 I2S audio codec.
+	   Say Y or m if you have such a device. This is a recommended option.
+	   If unsure select "N".
+
 config SND_SOC_INTEL_AVS_MACH_RT5663
 	tristate "rt5663 in I2S mode"
 	depends on I2C
diff --git a/sound/soc/intel/avs/boards/Makefile b/sound/soc/intel/avs/boards/Makefile
index 34347bcd1e7d..0ff21d55be24 100644
--- a/sound/soc/intel/avs/boards/Makefile
+++ b/sound/soc/intel/avs/boards/Makefile
@@ -13,6 +13,7 @@ snd-soc-avs-probe-objs := probe.o
 snd-soc-avs-rt274-objs := rt274.o
 snd-soc-avs-rt286-objs := rt286.o
 snd-soc-avs-rt298-objs := rt298.o
+snd-soc-avs-rt5514-objs := rt5514.o
 snd-soc-avs-rt5663-objs := rt5663.o
 snd-soc-avs-rt5682-objs := rt5682.o
 snd-soc-avs-ssm4567-objs := ssm4567.o
@@ -30,6 +31,7 @@ obj-$(CONFIG_SND_SOC_INTEL_AVS_MACH_PROBE) += snd-soc-avs-probe.o
 obj-$(CONFIG_SND_SOC_INTEL_AVS_MACH_RT274) += snd-soc-avs-rt274.o
 obj-$(CONFIG_SND_SOC_INTEL_AVS_MACH_RT286) += snd-soc-avs-rt286.o
 obj-$(CONFIG_SND_SOC_INTEL_AVS_MACH_RT298) += snd-soc-avs-rt298.o
+obj-$(CONFIG_SND_SOC_INTEL_AVS_MACH_RT5514) += snd-soc-avs-rt5514.o
 obj-$(CONFIG_SND_SOC_INTEL_AVS_MACH_RT5663) += snd-soc-avs-rt5663.o
 obj-$(CONFIG_SND_SOC_INTEL_AVS_MACH_RT5682) += snd-soc-avs-rt5682.o
 obj-$(CONFIG_SND_SOC_INTEL_AVS_MACH_SSM4567) += snd-soc-avs-ssm4567.o
diff --git a/sound/soc/intel/avs/boards/rt5514.c b/sound/soc/intel/avs/boards/rt5514.c
new file mode 100644
index 000000000000..ad486a52e5e3
--- /dev/null
+++ b/sound/soc/intel/avs/boards/rt5514.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright(c) 2021-2023 Intel Corporation. All rights reserved.
+//
+// Authors: Cezary Rojewski <cezary.rojewski@intel.com>
+//          Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
+//
+
+#include <linux/clk.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-acpi.h>
+#include "../../../codecs/rt5514.h"
+#include "../utils.h"
+
+#define RT5514_CODEC_DAI	"rt5514-aif1"
+
+static const struct snd_soc_dapm_widget card_widgets[] = {
+	SND_SOC_DAPM_MIC("DMIC", NULL),
+};
+
+static const struct snd_soc_dapm_route card_base_routes[] = {
+	/* DMIC */
+	{ "DMIC1L", NULL, "DMIC" },
+	{ "DMIC1R", NULL, "DMIC" },
+	{ "DMIC2L", NULL, "DMIC" },
+	{ "DMIC2R", NULL, "DMIC" },
+};
+
+static int avs_rt5514_codec_init(struct snd_soc_pcm_runtime *runtime)
+{
+	int ret = snd_soc_dapm_ignore_suspend(&runtime->card->dapm, "DMIC");
+
+	if (ret)
+		dev_err(runtime->dev, "DMIC - Ignore suspend failed = %d\n", ret);
+
+	return ret;
+}
+
+static int avs_rt5514_be_fixup(struct snd_soc_pcm_runtime *runtime,
+			       struct snd_pcm_hw_params *params)
+{
+	struct snd_interval *rate, *channels;
+	struct snd_mask *fmt;
+
+	rate = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
+	channels = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	fmt = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT);
+
+	rate->min = rate->max = 48000;
+	channels->min = channels->max = 4;
+
+	snd_mask_none(fmt);
+	snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE);
+
+	return 0;
+}
+
+static int avs_rt5514_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
+	int ret;
+
+	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0xF, 0, 8, 16);
+	if (ret < 0) {
+		dev_err(rtd->dev, "set TDM slot err:%d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, RT5514_SCLK_S_MCLK, 24576000, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		dev_err(rtd->dev, "set sysclk err: %d\n", ret);
+
+	return ret;
+}
+
+static const struct snd_soc_ops avs_rt5514_ops = {
+	.hw_params = avs_rt5514_hw_params,
+};
+
+static int avs_create_dai_link(struct device *dev, const char *platform_name, int ssp_port,
+			       int tdm_slot, struct snd_soc_dai_link **dai_link)
+{
+	struct snd_soc_dai_link_component *platform;
+	struct snd_soc_dai_link *dl;
+
+	dl = devm_kzalloc(dev, sizeof(*dl), GFP_KERNEL);
+	platform = devm_kzalloc(dev, sizeof(*platform), GFP_KERNEL);
+	if (!dl || !platform)
+		return -ENOMEM;
+
+	platform->name = platform_name;
+
+	dl->name = devm_kasprintf(dev, GFP_KERNEL,
+				  AVS_STRING_FMT("SSP", "-Codec", ssp_port, tdm_slot));
+	dl->cpus = devm_kzalloc(dev, sizeof(*dl->cpus), GFP_KERNEL);
+	dl->codecs = devm_kzalloc(dev, sizeof(*dl->codecs), GFP_KERNEL);
+	if (!dl->name || !dl->cpus || !dl->codecs)
+		return -ENOMEM;
+
+	dl->cpus->dai_name = devm_kasprintf(dev, GFP_KERNEL,
+					    AVS_STRING_FMT("SSP", " Pin", ssp_port, tdm_slot));
+	dl->codecs->name = devm_kasprintf(dev, GFP_KERNEL, "i2c-10EC5514:00");
+	dl->codecs->dai_name = devm_kasprintf(dev, GFP_KERNEL, RT5514_CODEC_DAI);
+	if (!dl->cpus->dai_name || !dl->codecs->name || !dl->codecs->dai_name)
+		return -ENOMEM;
+
+	dl->num_cpus = 1;
+	dl->num_codecs = 1;
+	dl->platforms = platform;
+	dl->num_platforms = 1;
+	dl->id = 0;
+	dl->dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS;
+	dl->init = avs_rt5514_codec_init;
+	dl->be_hw_params_fixup = avs_rt5514_be_fixup;
+	dl->nonatomic = 1;
+	dl->no_pcm = 1;
+	dl->dpcm_capture = 1;
+	dl->ops = &avs_rt5514_ops;
+
+	*dai_link = dl;
+
+	return 0;
+}
+
+static int avs_rt5514_probe(struct platform_device *pdev)
+{
+	struct snd_soc_dai_link *dai_link;
+	struct snd_soc_acpi_mach *mach;
+	struct snd_soc_card *card;
+	struct device *dev = &pdev->dev;
+	const char *pname;
+	int ssp_port, tdm_slot, ret;
+
+	mach = dev_get_platdata(dev);
+	pname = mach->mach_params.platform;
+
+	ret = avs_mach_get_ssp_tdm(dev, mach, &ssp_port, &tdm_slot);
+	if (ret)
+		return ret;
+
+	ret = avs_create_dai_link(dev, pname, ssp_port, tdm_slot, &dai_link);
+	if (ret) {
+		dev_err(dev, "Failed to create dai link: %d", ret);
+		return ret;
+	}
+
+	card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL);
+	if (!card)
+		return -ENOMEM;
+
+	card->name = "avs_rt5514";
+	card->dev = dev;
+	card->owner = THIS_MODULE;
+	card->dai_link = dai_link;
+	card->num_links = 1;
+	card->dapm_widgets = card_widgets;
+	card->num_dapm_widgets = ARRAY_SIZE(card_widgets);
+	card->dapm_routes = card_base_routes;
+	card->num_dapm_routes = ARRAY_SIZE(card_base_routes);
+	card->fully_routed = true;
+
+	ret = snd_soc_fixup_dai_links_platform_name(card, pname);
+	if (ret)
+		return ret;
+
+	return devm_snd_soc_register_card(dev, card);
+}
+
+static struct platform_driver avs_rt5514_driver = {
+	.probe = avs_rt5514_probe,
+	.driver = {
+		.name = "avs_rt5514",
+		.pm = &snd_soc_pm_ops,
+	},
+};
+
+module_platform_driver(avs_rt5514_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:avs_rt5514");
-- 
2.34.1

[PATCH 2/2] ASoC: Intel: avs: Add rt5514 machine board

[Amadeusz Sławiński]

In order to support Eve chromebooks add rt5514 configuration to board
lookup table. rt5514 is used for capture in DMIC configuration on SSP 0
and TDM 1.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/intel/avs/board_selection.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sound/soc/intel/avs/board_selection.c b/sound/soc/intel/avs/board_selection.c
index c10fff705496..8e91eece992d 100644
--- a/sound/soc/intel/avs/board_selection.c
+++ b/sound/soc/intel/avs/board_selection.c
@@ -135,6 +135,15 @@ static struct snd_soc_acpi_mach avs_kbl_i2s_machines[] = {
 		},
 		.tplg_filename = "max98927-tplg.bin",
 	},
+	{
+		.id = "10EC5514",
+		.drv_name = "avs_rt5514",
+		.mach_params = {
+			.i2s_link_mask = AVS_SSP(0),
+		},
+		.pdata = (unsigned long[]){ 0x2, 0, 0, 0, 0, 0 }, /* SSP0 TDMs */
+		.tplg_filename = "rt5514-tplg.bin",
+	},
 	{
 		.id = "10EC5663",
 		.drv_name = "avs_rt5663",
-- 
2.34.1

Re: [PATCH 0/2] ASoC: Intel: avs: Add support for rt5514 codec

[Mark Brown]

On Fri, 27 Oct 2023 13:05:35 +0200, Amadeusz Sławiński wrote:
> There are machines which use codec rt5514 as DMIC, add support for them.
> 
> Amadeusz Sławiński (2):
>   ASoC: Intel: avs: Add rt5514 machine board
>   ASoC: Intel: avs: Add rt5514 machine board
> 
>  sound/soc/intel/avs/board_selection.c |   9 ++
>  sound/soc/intel/avs/boards/Kconfig    |  10 ++
>  sound/soc/intel/avs/boards/Makefile   |   2 +
>  sound/soc/intel/avs/boards/rt5514.c   | 187 ++++++++++++++++++++++++++
>  4 files changed, 208 insertions(+)
>  create mode 100644 sound/soc/intel/avs/boards/rt5514.c
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: Intel: avs: Add rt5514 machine board
      commit: d933333694a7e63b0893d23f65b104430a1d6cf6
[2/2] ASoC: Intel: avs: Add rt5514 machine board
      commit: d933333694a7e63b0893d23f65b104430a1d6cf6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH 1/2] ASoC: Intel: avs: Add rt5514 machine board

[Krzysztof Kozlowski]

On 27/10/2023 13:05, Amadeusz Sławiński wrote:
> To support AVS-rt5514 configuration add machine board connecting AVS
> platform component driver with rt5514 codec one.
> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> ---
>  sound/soc/intel/avs/boards/Kconfig  |  10 ++
>  sound/soc/intel/avs/boards/Makefile |   2 +
>  sound/soc/intel/avs/boards/rt5514.c | 187 ++++++++++++++++++++++++++++
>  3 files changed, 199 insertions(+)
>  create mode 100644 sound/soc/intel/avs/boards/rt5514.c

...

> +
> +static struct platform_driver avs_rt5514_driver = {
> +	.probe = avs_rt5514_probe,
> +	.driver = {
> +		.name = "avs_rt5514",
> +		.pm = &snd_soc_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(avs_rt5514_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:avs_rt5514");

You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong.

Best regards,
Krzysztof

Re: [PATCH 1/2] ASoC: Intel: avs: Add rt5514 machine board

[Amadeusz Sławiński]

On 10/28/2023 10:46 AM, Krzysztof Kozlowski wrote:
> On 27/10/2023 13:05, Amadeusz Sławiński wrote:
>> To support AVS-rt5514 configuration add machine board connecting AVS
>> platform component driver with rt5514 codec one.
>>
>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>> ---
>>   sound/soc/intel/avs/boards/Kconfig  |  10 ++
>>   sound/soc/intel/avs/boards/Makefile |   2 +
>>   sound/soc/intel/avs/boards/rt5514.c | 187 ++++++++++++++++++++++++++++
>>   3 files changed, 199 insertions(+)
>>   create mode 100644 sound/soc/intel/avs/boards/rt5514.c
> 
> ...
> 
>> +
>> +static struct platform_driver avs_rt5514_driver = {
>> +	.probe = avs_rt5514_probe,
>> +	.driver = {
>> +		.name = "avs_rt5514",
>> +		.pm = &snd_soc_pm_ops,
>> +	},
>> +};
>> +
>> +module_platform_driver(avs_rt5514_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:avs_rt5514");
> 
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong.
> 

In theory yes, in practice it is a bit more complicated, as we use the 
driver alias in sound/soc/intel/avs/board_selection.c in 
snd_soc_acpi_mach and they should match.

For example for rt286, there is:
# modinfo 
/lib/modules/6.4.0-rc3+/kernel/sound/soc/intel/avs/boards/snd-soc-avs-rt286.ko 
| grep 286
filename: 
/lib/modules/6.4.0-rc3+/kernel/sound/soc/intel/avs/boards/snd-soc-avs-rt286.ko
alias:          platform:avs_rt286
name:           snd_soc_avs_rt286
as you can see platform_driver::driver::name is not matching the driver 
name.

I've did quick test with removing alias and changing snd_soc_acpi_mach 
definition for one board and it didn't load.

Now that you pointed it out I also lean towards trying to remove 
MODULE_ALIAS() from board drivers, but it will probably require some 
more investigation if we really want to do it and implementing it properly.

Re: [PATCH 1/2] ASoC: Intel: avs: Add rt5514 machine board

[Krzysztof Kozlowski]

On 30/10/2023 12:50, Amadeusz Sławiński wrote:
> On 10/28/2023 10:46 AM, Krzysztof Kozlowski wrote:
>> On 27/10/2023 13:05, Amadeusz Sławiński wrote:
>>> To support AVS-rt5514 configuration add machine board connecting AVS
>>> platform component driver with rt5514 codec one.
>>>
>>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>>> ---
>>>   sound/soc/intel/avs/boards/Kconfig  |  10 ++
>>>   sound/soc/intel/avs/boards/Makefile |   2 +
>>>   sound/soc/intel/avs/boards/rt5514.c | 187 ++++++++++++++++++++++++++++
>>>   3 files changed, 199 insertions(+)
>>>   create mode 100644 sound/soc/intel/avs/boards/rt5514.c
>>
>> ...
>>
>>> +
>>> +static struct platform_driver avs_rt5514_driver = {
>>> +	.probe = avs_rt5514_probe,
>>> +	.driver = {
>>> +		.name = "avs_rt5514",
>>> +		.pm = &snd_soc_pm_ops,
>>> +	},
>>> +};
>>> +
>>> +module_platform_driver(avs_rt5514_driver);
>>> +
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_ALIAS("platform:avs_rt5514");
>>
>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>> usually it means your device ID table is wrong.
>>
> 
> In theory yes, in practice it is a bit more complicated, as we use the 
> driver alias in sound/soc/intel/avs/board_selection.c in 
> snd_soc_acpi_mach and they should match.
> 
> For example for rt286, there is:
> # modinfo 
> /lib/modules/6.4.0-rc3+/kernel/sound/soc/intel/avs/boards/snd-soc-avs-rt286.ko 
> | grep 286
> filename: 
> /lib/modules/6.4.0-rc3+/kernel/sound/soc/intel/avs/boards/snd-soc-avs-rt286.ko
> alias:          platform:avs_rt286
> name:           snd_soc_avs_rt286
> as you can see platform_driver::driver::name is not matching the driver 
> name.
> 
> I've did quick test with removing alias and changing snd_soc_acpi_mach 
> definition for one board and it didn't load.

Sorry, but why do you talk about platform name? We talk about ID table!

> 
> Now that you pointed it out I also lean towards trying to remove 
> MODULE_ALIAS() from board drivers, but it will probably require some 
> more investigation if we really want to do it and implementing it properly.

Ehm? We have been there. I've been dropping these useless aliases as
well. You miss DEVICE_TABLE and proper ID entries, not adding aliases.



Best regards,
Krzysztof

Re: [PATCH 1/2] ASoC: Intel: avs: Add rt5514 machine board

[Amadeusz Sławiński]

On 10/30/2023 12:59 PM, Krzysztof Kozlowski wrote:
> On 30/10/2023 12:50, Amadeusz Sławiński wrote:
>> On 10/28/2023 10:46 AM, Krzysztof Kozlowski wrote:
>>> On 27/10/2023 13:05, Amadeusz Sławiński wrote:
>>>> To support AVS-rt5514 configuration add machine board connecting AVS
>>>> platform component driver with rt5514 codec one.
>>>>
>>>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>>>> ---
>>>>    sound/soc/intel/avs/boards/Kconfig  |  10 ++
>>>>    sound/soc/intel/avs/boards/Makefile |   2 +
>>>>    sound/soc/intel/avs/boards/rt5514.c | 187 ++++++++++++++++++++++++++++
>>>>    3 files changed, 199 insertions(+)
>>>>    create mode 100644 sound/soc/intel/avs/boards/rt5514.c
>>>
>>> ...
>>>
>>>> +
>>>> +static struct platform_driver avs_rt5514_driver = {
>>>> +	.probe = avs_rt5514_probe,
>>>> +	.driver = {
>>>> +		.name = "avs_rt5514",
>>>> +		.pm = &snd_soc_pm_ops,
>>>> +	},
>>>> +};
>>>> +
>>>> +module_platform_driver(avs_rt5514_driver);
>>>> +
>>>> +MODULE_LICENSE("GPL");
>>>> +MODULE_ALIAS("platform:avs_rt5514");
>>>
>>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>>> usually it means your device ID table is wrong.
>>>
>>
>> In theory yes, in practice it is a bit more complicated, as we use the
>> driver alias in sound/soc/intel/avs/board_selection.c in
>> snd_soc_acpi_mach and they should match.
>>
>> For example for rt286, there is:
>> # modinfo
>> /lib/modules/6.4.0-rc3+/kernel/sound/soc/intel/avs/boards/snd-soc-avs-rt286.ko
>> | grep 286
>> filename:
>> /lib/modules/6.4.0-rc3+/kernel/sound/soc/intel/avs/boards/snd-soc-avs-rt286.ko
>> alias:          platform:avs_rt286
>> name:           snd_soc_avs_rt286
>> as you can see platform_driver::driver::name is not matching the driver
>> name.
>>
>> I've did quick test with removing alias and changing snd_soc_acpi_mach
>> definition for one board and it didn't load.
> 
> Sorry, but why do you talk about platform name? We talk about ID table!
> 
>>
>> Now that you pointed it out I also lean towards trying to remove
>> MODULE_ALIAS() from board drivers, but it will probably require some
>> more investigation if we really want to do it and implementing it properly.
> 
> Ehm? We have been there. I've been dropping these useless aliases as
> well. You miss DEVICE_TABLE and proper ID entries, not adding aliases.
> 

Ah, DEVICE_TABLE, that's what I was missing, so I just to confirm, 
something like:

diff --git a/sound/soc/intel/avs/boards/rt274.c 
b/sound/soc/intel/avs/boards/rt274.c
index dd613aa15e80..659e10b1dcad 100644
--- a/sound/soc/intel/avs/boards/rt274.c
+++ b/sound/soc/intel/avs/boards/rt274.c
@@ -257,6 +257,14 @@ static int avs_rt274_probe(struct platform_device 
*pdev)
         return devm_snd_soc_register_card(dev, card);
  }

+static const struct platform_device_id avs_rt274_driver_ids[] = {
+        {
+                .name           = "avs_rt274",
+        },
+        {},
+};
+MODULE_DEVICE_TABLE(platform, avs_rt274_driver_ids);
+
  static struct platform_driver avs_rt274_driver = {
         .probe = avs_rt274_probe,
         .driver = {
@@ -268,4 +276,3 @@ static struct platform_driver avs_rt274_driver = {
  module_platform_driver(avs_rt274_driver);

  MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:avs_rt274");

for all the boards?

Re: [PATCH 1/2] ASoC: Intel: avs: Add rt5514 machine board

[Krzysztof Kozlowski]

On 30/10/2023 13:15, Amadeusz Sławiński wrote:
>>>>> +static struct platform_driver avs_rt5514_driver = {
>>>>> +	.probe = avs_rt5514_probe,
>>>>> +	.driver = {
>>>>> +		.name = "avs_rt5514",
>>>>> +		.pm = &snd_soc_pm_ops,
>>>>> +	},
>>>>> +};
>>>>> +
>>>>> +module_platform_driver(avs_rt5514_driver);
>>>>> +
>>>>> +MODULE_LICENSE("GPL");
>>>>> +MODULE_ALIAS("platform:avs_rt5514");
>>>>
>>>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>>>> usually it means your device ID table is wrong.
>>>>
>>>
>>> In theory yes, in practice it is a bit more complicated, as we use the
>>> driver alias in sound/soc/intel/avs/board_selection.c in
>>> snd_soc_acpi_mach and they should match.
>>>
>>> For example for rt286, there is:
>>> # modinfo
>>> /lib/modules/6.4.0-rc3+/kernel/sound/soc/intel/avs/boards/snd-soc-avs-rt286.ko
>>> | grep 286
>>> filename:
>>> /lib/modules/6.4.0-rc3+/kernel/sound/soc/intel/avs/boards/snd-soc-avs-rt286.ko
>>> alias:          platform:avs_rt286
>>> name:           snd_soc_avs_rt286
>>> as you can see platform_driver::driver::name is not matching the driver
>>> name.
>>>
>>> I've did quick test with removing alias and changing snd_soc_acpi_mach
>>> definition for one board and it didn't load.
>>
>> Sorry, but why do you talk about platform name? We talk about ID table!
>>
>>>
>>> Now that you pointed it out I also lean towards trying to remove
>>> MODULE_ALIAS() from board drivers, but it will probably require some
>>> more investigation if we really want to do it and implementing it properly.
>>
>> Ehm? We have been there. I've been dropping these useless aliases as
>> well. You miss DEVICE_TABLE and proper ID entries, not adding aliases.
>>
> 
> Ah, DEVICE_TABLE, that's what I was missing, so I just to confirm, 
> something like:
> 
> diff --git a/sound/soc/intel/avs/boards/rt274.c 
> b/sound/soc/intel/avs/boards/rt274.c
> index dd613aa15e80..659e10b1dcad 100644
> --- a/sound/soc/intel/avs/boards/rt274.c
> +++ b/sound/soc/intel/avs/boards/rt274.c
> @@ -257,6 +257,14 @@ static int avs_rt274_probe(struct platform_device 
> *pdev)
>          return devm_snd_soc_register_card(dev, card);
>   }
> 
> +static const struct platform_device_id avs_rt274_driver_ids[] = {
> +        {
> +                .name           = "avs_rt274",
> +        },
> +        {},
> +};
> +MODULE_DEVICE_TABLE(platform, avs_rt274_driver_ids);

Yes, this is the basic method of probing all devices and auto-loading
modules. I wrote at beginning that your ID table is wrong, but I was not
aware that it is missing completely.

Best regards,
Krzysztof