On 2022-05-06 3:12 PM, Mark Brown wrote:
On Wed, Apr 27, 2022 at 10:47:12AM -0500, Pierre-Louis Bossart wrote:
On 4/27/22 03:18, Cezary Rojewski wrote:
Add generic ASoC equivalent of ALSA HD-Audio codec. This codec is designed to follow HDA_DEV_LEGACY convention. Driver wrapps existing hda_codec.c handlers to prevent code duplication within the newly added
I am surprised the explanations don't even mention the existence of hdac_hda.c
I thought the series was about adding machine drivers, but this also adds code on the sound/soc/codecs/ side which I didn't see coming.
I am not qualified to review this part of the code, I just wonder about duplication of functionality.
At the very least an explanation on why you decided to NOT use hdac_hda.c would be useful to reviewers and maintainers.
Right, why the duplication here? Can't we fix or extend the existing code to do whatever it's not currently doing which compels reimplementation?
Sorry for the late response, did not realize there is an unanswered comment here.
So, the rough list goes as: - hdac_hda.c hardcodes codec capabilities rather than aligning with what sound/pci/hda/ code does - merges HDMI (i.e. Intel i915 audio component) and HDA DAIs together whereas these are two separate devices - because of above, implements custom search/matching mechanism for PCM/DAI - cont. because of above, its header hosts private data struct, unnecessary complication - follows HDA_DEV_ASOC convention rather than HDA_DEV_LEGACY causing misalignments between sound/pci/hda and sound/soc/ behaviour - has basic PM runtime support and does not survive scenarios where resume/suspend + denylist + rmmod/modprobe are mixed together or invoked in unordered fashion between this module and several others in the audio stack
My suggestion is different: have all HD-Audio ASoC users switch to this implementation when possible and remove the existing code along with skylake-driver.
Regards, Czarek