[alsa-devel] [PATCH v1 5/9] ALSA: hda - split snd_hda_codec_new function

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart at linux.intel.com]
>Sent: Friday, February 23, 2018 10:20 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja at intel.com>; alsa-devel at alsa-
>project.org; broonie at kernel.org; tiwai at suse.de;
>liam.r.girdwood at linux.intel.com
>Cc: Koul, Vinod <vinod.koul at intel.com>; Patches Audio
><patches.audio at intel.com>
>Subject: Re: [PATCH v1 5/9] ALSA: hda - split snd_hda_codec_new function
>
>On 2/23/18 2:12 AM, Rakesh Ughreja wrote:
>> Split snd_hda_codec_new into two separate functions.
>> snd_hda_codec_device_init allocates memory and registers with bus.
>> snd_hda_codec_device_new initialializes the fields and performs
>> snd_device_new. This enables reuse of legacy HDA codec drivers as ASoC
>> codec drivers.
>>
>> In addition mark some functions with EXPORT_SYMBOL_GPL so that it can be
>> called by ASoC codec drivers.
>
>I don't get the commit message.
>You first say that we can now reuse legacy HDaudio codec drivers in an
>ASoC framework, then say that there will be additional ASoC codec
>drivers? Why would we do this, it seems like a contradicting goal?
>

Yes, its misleading, so correcting the line as,

In addition mark some functions with EXPORT_SYMBOL_GPL so that it can be
called by ASoC wrapper around the legacy HDA driver (hdac_hda).

>>
>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja at intel.com>
>> ---
>>   sound/pci/hda/hda_codec.c | 67 +++++++++++++++++++++++++++++++++++--
>----------
>>   sound/pci/hda/hda_codec.h |  2 ++
>>   2 files changed, 52 insertions(+), 17 deletions(-)
>>
>> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
>> index 5bc3a74..c487411 100644
>> --- a/sound/pci/hda/hda_codec.c
>> +++ b/sound/pci/hda/hda_codec.c
>> @@ -856,6 +856,38 @@ static void snd_hda_codec_dev_release(struct device
>*dev)
>>   	kfree(codec);
>>   }
>>
>> +static int snd_hda_codec_device_init(struct hda_bus *bus, struct snd_card
>*card,
>> +			unsigned int codec_addr, struct hda_codec **codecp)
>> +{
>> +	int err;
>> +	char component[31];
>
>looks like magic number?

I kept it as it was in the previous code, but now defined a macro
#define COMPONENT_NAME_SIZE 31

>
>> +	struct hda_codec *codec;
>
>reverse x-mas tree order?

Yes, changed.

Regards,
Rakesh