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

["Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>]

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?