-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, April 22, 2015 10:09 PM To: Jie, Yang Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; tanu.kaskinen@linux.intel.com Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl
At Wed, 22 Apr 2015 13:56:25 +0000, Jie, Yang wrote:
-----Original Message----- From: Jie, Yang Sent: Wednesday, April 22, 2015 9:23 PM To: 'Takashi Iwai' Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; tanu.kaskinen@linux.intel.com Subject: RE: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, April 22, 2015 8:58 PM To: Jie, Yang Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; tanu.kaskinen@linux.intel.com Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl
At Wed, 22 Apr 2015 12:40:43 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, April 22, 2015 7:33 PM To: Jie, Yang Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; tanu.kaskinen@linux.intel.com Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl
At Wed, 22 Apr 2015 14:03:48 +0800, Jie Yang wrote: > > Move available index get part into snd_kctl_jack_new(), also > add kctl name regenerating func to remove redundant " Jack" > which is passed in wrongly in some cases.
The subject doesn't match with this description well.
How about changing them as below:
ALSA: Jack: handle jack embedded kcontrol creating within ctljack
In ctljack.c, add function get_available_index() to allocate index for new kcontrol, and jack_kctl_name_gen() to remove
redundant "
Jack"
which is passed in wrongly in some cases.
> Signed-off-by: Jie Yang yang.jie@intel.com > --- > include/sound/control.h | 2 +- > sound/core/ctljack.c | 39
+++++++++++++++++++++++++++++++++++-
> sound/core/jack.c | 2 +- > sound/pci/hda/hda_jack.c | 2 +- > 4 files changed, 38 insertions(+), 7 deletions(-) > > diff --git a/include/sound/control.h > b/include/sound/control.h index > 75f3054..58751a0 100644 > --- a/include/sound/control.h > +++ b/include/sound/control.h > @@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct > snd_kcontrol *kctl, bool hook_only); > * Helper functions for jack-detection controls > */ > struct snd_kcontrol * > -snd_kctl_jack_new(const char *name, int idx, void > *private_data); > +snd_kctl_jack_new(const char *name, struct snd_card *card); > void snd_kctl_jack_report(struct snd_card *card, > struct snd_kcontrol *kctl, bool status); > > diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c > index > e4b38fb..b631996 100644 > --- a/sound/core/ctljack.c > +++ b/sound/core/ctljack.c > @@ -31,15 +31,46 @@ static struct snd_kcontrol_new > jack_detect_kctl
= {
> .get = jack_detect_kctl_get, }; > > +static int get_available_index(struct snd_card *card, const > +char > +*name) { > + struct snd_ctl_elem_id sid; > + > + memset(&sid, 0, sizeof(sid)); > + > + sid.index = 0; > + sid.iface = SNDRV_CTL_ELEM_IFACE_CARD; > + strlcpy(sid.name, name, sizeof(sid.name)); > + > + while (snd_ctl_find_id(card, &sid)) > + sid.index++; > + > + return sid.index; > +} > + > +static void jack_kctl_name_gen(char *name, const char > +*src_name, int > +size) { > + size_t count = strlen(src_name); > + bool need_cat = true; > + > + /* remove redundant " Jack" from src_name */ > + if (count >= 5) > + need_cat = strncmp(&src_name[count - 5], " Jack",
- ? true :
> +false; > + > + snprintf(name, size, need_cat ? "%s Jack" : "%s", > +src_name); > + > +} > + > struct snd_kcontrol * > -snd_kctl_jack_new(const char *name, int idx, void > *private_data) > +snd_kctl_jack_new(const char *name, struct snd_card *card) > { > struct snd_kcontrol *kctl; > - kctl = snd_ctl_new1(&jack_detect_kctl, private_data); > + > + kctl = snd_ctl_new1(&jack_detect_kctl, card);
Why changing the private data? Did it really work...?
Hi, Takashi, I recalled that why I need change it to 'card' here: we need it in the new added function get_available_index().
when I look more into snd_ctl_new1() and the private_data usage in hda_jack.c(private_data is not really used there), so, I prefer to keep this change, and change the calling in hda_jack.c as below:
- kctl = snd_kctl_jack_new(name, codec);
- kctl = snd_kctl_jack_new(name, codec->bus->card);
This should fix both issues.
Rather change the function to take both card and private_data arguments temporarily. Later on, you can drop private_data argument once when the code is detached from hda.
OK, will do like that.
Takashi