-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, April 20, 2015 4:06 PM To: Jie, Yang Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; tanu.kaskinen@linux.intel.com; Liam Girdwood Subject: Re: [PATCH v6 1/6] ALSA: jack: create jack kcontrols for every jack input
At Mon, 20 Apr 2015 07:52:20 +0000, Jie, Yang wrote:
@@ -82,10 +84,19 @@ struct snd_jack { void (*private_free)(struct snd_jack *); };
+struct snd_jack_kctl {
- struct snd_kcontrol *kctl;
- struct list_head list; /* list of controls belong to the same
jack */
- unsigned int mask_bits; /* one of the corresponding bits of
status change will report to this kctl */
- void *private_data;
- void (*private_free)(struct snd_jack_kctl *jack_kctl); };
You don't have to expose this. The hda_jack.c doesn't actually need these whole bits, as it supposes 1:1 as jack:kctl mapping. You can get rid of hda_jack->kctl since snd_jack_report() does it already, thus the private_data and the destructor can be dropped, too.
The patch 3/6 removes had_jack->kctl, here I added priate destructor for HDA jack to reset hda_jack_tbl jacks->nid and jacks->jack to 0. Seems these are needed for HDA jack logic, otherwise, I can already dropped them. :(
It's not needed, as far as I understand your code.
Try to look from from the top-level POV: the hda driver just needs to register a jack object via snd_jack_new(), reports it, and remove it in destructor. The private_data was needed for tracking the codec-local kctls, but now they are managed in jack object itself, thus we don't have to keep eyes on it. The private_free is for removing the kctl list, and since this is gone, we don't have to take it, either. In other words, we don't have to take care of kctls at all in hda driver side. This would be the ideal situation.
Thanks for explaining it. Finally I make clear that jack private_free will call hda_free_jack_priv() to reset jacks->nid/jack, so I have no worry now. :)
(snip)
snd_kctl_jack_new() itself. At the same time, the code doing the same thing in hda_jack.c can be removed. Then the index argument of
A little concern that there may be a little difference here: hda_jack.c:get_unique_index() check the codec kctl list, but in my patch we check card kctl list, I am afraid that it may influence the exist HDA usage, wo just keep what HDA did before for HDA jack.
Takashi, do you think it's a risk to change from codec kctl list to card kctl list for HDA part?
No, there shouldn't. The id must be unique for the whole card, not specific to codecs. That is, the code looking through snd_ctl_find_id() is even safer than the current version.
OK, then I feel reassured to replace it. :)
thanks,
Takashi