[alsa-devel] [PATCH v7 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Jie, Yang]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Tuesday, April 21, 2015 5:53 PM
> To: Jie, Yang
> Cc: broonie at kernel.org; alsa-devel at alsa-project.org; Girdwood, Liam R
> Subject: Re: [PATCH v7 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to
> support embedded kctl
> 
> At Tue, 21 Apr 2015 16:05:40 +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.
> >
> > Signed-off-by: Jie Yang <yang.jie at 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", 5) ? 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);
> >  	if (!kctl)
> >  		return NULL;
> > -	snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
> > -	kctl->id.index = idx;
> > +
> > +	jack_kctl_name_gen(kctl->id.name, name, sizeof(kctl->id.name));
> > +	kctl->id.index = get_available_index(card, name);
> >  	kctl->private_value = 0;
> >  	return kctl;
> >  }
> > diff --git a/sound/core/jack.c b/sound/core/jack.c index
> > 741db7c..b13d0b1 100644
> > --- a/sound/core/jack.c
> > +++ b/sound/core/jack.c
> > @@ -141,7 +141,7 @@ static struct snd_jack_kctl *
> snd_jack_kctl_new(struct snd_card *card, const cha
> >  	struct snd_jack_kctl *jack_kctl;
> >  	int err;
> >
> > -	kctl = snd_kctl_jack_new(name, 0, card);
> > +	kctl = snd_kctl_jack_new(name, card);
> >  	if (!kctl)
> >  		return NULL;
> >
> > diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index
> > e664307..a046e2f 100644
> > --- a/sound/pci/hda/hda_jack.c
> > +++ b/sound/pci/hda/hda_jack.c
> > @@ -402,7 +402,7 @@ static int __snd_hda_jack_add_kctl(struct
> hda_codec *codec, hda_nid_t nid,
> >  		return 0;
> >  	if (jack->kctl)
> >  		return 0; /* already created */
> > -	kctl = snd_kctl_jack_new(name, idx, codec);
> > +	kctl = snd_kctl_jack_new(name, codec);
> >  	if (!kctl)
> >  		return -ENOMEM;
> >  	err = snd_hda_ctl_add(codec, nid, kctl);
> 
> And you can get rid of the same local functions in hda_jack.c.
> Now they are superfluous.

When I grep and found there are 14 other calling to the function snd_hda_ctl_add(),
seems the func is still needed for non-jack controls?

> 
> 
> Takashi