[PATCH v1] ASoC: Intel: sof_nau8825: add support for nau8825 with amp nau8318
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Fri Dec 9 18:31:24 CET 2022
>> This looks inconsistent with the commit message. There are separate
>> Kconfigs for different codecs.
>>
>> SND_SOC_NAU8315
>> SND_SOC_NAU8825
>>
>> Which is it?
>>
>
> Sorry about confusing you, I think it is better to change the title as
> ASoC: Intel: sof_nau8825: add combination of nau8825 headset codec
> with nau8318 Amp.
Suggested edit:
ASoC: Intel: sof_nau8825: add variant with nau8318 amplifier.
>
> And add some information about Nuvoton chips in the comment message.
> ***
> The nau8315 and nau8318 are both Nuvoton Amp chips. They use the same
> Amp driver nau8315.c. The acpi_device_id for nau8315 is "NVTN2010",
> for nau8318 is "NVTN2012".
That should be added in the commit message please.
>> NAK for this v1. Please clarify which codec you are using and make sure
>> all references are consistent.
>>
>>
>
> I apologize for the unclear comment message, please give me any
> suggestions if needed, and I will send v2 for review.
> thanks.
Ok, makes sense now. Please do include the explanations on 8315/8318
variants, I couldn't figure out what chips were used.
I would also not use the same topology name as a different platform with
another amplifier. I appreciate you trying to reuse when possible, but
it's just better to create a new topology file. That way if you want any
update down the road you can do so, it's more maintainable and risk-free.
More information about the Alsa-devel
mailing list