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
