[alsa-devel] [PATCH v6 1/6] ALSA: jack: create jack kcontrols for every jack input

[Takashi Iwai]

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.

(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.


thanks,

Takashi