Re: [PATCH v5 03/12] ALSA: control: Add init callback for kcontrol

[Takashi Iwai <tiwai@suse.de>]

At Mon, 8 Sep 2014 09:44:21 +0530,
Vinod Koul wrote:
> 
> On Sat, Sep 06, 2014 at 05:56:02PM +0200, Takashi Iwai wrote:
> > At Sat, 6 Sep 2014 15:21:24 +0100,
> > Mark Brown wrote:
> > > 
> > > On Tue, Sep 02, 2014 at 06:05:58PM +0530, Subhransu S. Prusty wrote:
> > > > Some controls need to initialize stuffs like pvt data, so they need a
> > > > callback if the control creation is successful.
> > > 
> > > Adding Takashi - this is ALSA core code so he needs to review it.
> > 
> > This would bloat effectively the data size of all sound drivers, so I
> > can't ack it without more proper reasoning.  That is, please convince
> > me why this change must be taken for the cost of size bloat of all
> > sound drivers.  Can't you do it in the caller side of snd_ctl_add()
> > like many other drivers already do?
> Okay lets step back and see why we need this :)
> 
> In our case after the control creation we need to allocate memory which will
> hold the data for the byte controls. This can be done only after the
> controls are created (by asoc).

Why?  Because you don't need how many bytes to allocate?

> For that we need a callback into driver so that we can allocate the memory.
> Thats why we added .init() method. If you have any other way to do this, we
> are all ears :)

For example, you can embed an init flag into your record and call the
initializer in get/put callback if not called yet.


Takashi

> 
> -- 
> ~Vinod
> 
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > > > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> > > > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > > > ---
> > > >  include/sound/control.h | 3 +++
> > > >  sound/core/control.c    | 7 +++++++
> > > >  2 files changed, 10 insertions(+)
> > > > 
> > > > diff --git a/include/sound/control.h b/include/sound/control.h
> > > > index 0426139..1389f69 100644
> > > > --- a/include/sound/control.h
> > > > +++ b/include/sound/control.h
> > > > @@ -30,6 +30,7 @@ struct snd_kcontrol;
> > > >  typedef int (snd_kcontrol_info_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_info * uinfo);
> > > >  typedef int (snd_kcontrol_get_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_value * ucontrol);
> > > >  typedef int (snd_kcontrol_put_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_value * ucontrol);
> > > > +typedef int (snd_kcontrol_init_t) (struct snd_kcontrol * kcontrol);
> > > >  typedef int (snd_kcontrol_tlv_rw_t)(struct snd_kcontrol *kcontrol,
> > > >  				    int op_flag, /* SNDRV_CTL_TLV_OP_XXX */
> > > >  				    unsigned int size,
> > > > @@ -52,6 +53,7 @@ struct snd_kcontrol_new {
> > > >  	snd_kcontrol_info_t *info;
> > > >  	snd_kcontrol_get_t *get;
> > > >  	snd_kcontrol_put_t *put;
> > > > +	snd_kcontrol_init_t *init;
> > > >  	union {
> > > >  		snd_kcontrol_tlv_rw_t *c;
> > > >  		const unsigned int *p;
> > > > @@ -71,6 +73,7 @@ struct snd_kcontrol {
> > > >  	snd_kcontrol_info_t *info;
> > > >  	snd_kcontrol_get_t *get;
> > > >  	snd_kcontrol_put_t *put;
> > > > +	snd_kcontrol_init_t *init;
> > > >  	union {
> > > >  		snd_kcontrol_tlv_rw_t *c;
> > > >  		const unsigned int *p;
> > > > diff --git a/sound/core/control.c b/sound/core/control.c
> > > > index b961134..9d30663 100644
> > > > --- a/sound/core/control.c
> > > > +++ b/sound/core/control.c
> > > > @@ -256,6 +256,7 @@ struct snd_kcontrol *snd_ctl_new1(const struct snd_kcontrol_new *ncontrol,
> > > >  	kctl.info = ncontrol->info;
> > > >  	kctl.get = ncontrol->get;
> > > >  	kctl.put = ncontrol->put;
> > > > +	kctl.init = ncontrol->init;
> > > >  	kctl.tlv.p = ncontrol->tlv.p;
> > > >  	kctl.private_value = ncontrol->private_value;
> > > >  	kctl.private_data = private_data;
> > > > @@ -362,6 +363,12 @@ int snd_ctl_add(struct snd_card *card, struct snd_kcontrol *kcontrol)
> > > >  		err = -ENOMEM;
> > > >  		goto error;
> > > >  	}
> > > > +	if (kcontrol->init) {
> > > > +		err = kcontrol->init(kcontrol);
> > > > +		if (err < 0)
> > > > +			goto error;
> > > > +	}
> > > > +
> > > >  	list_add_tail(&kcontrol->list, &card->controls);
> > > >  	card->controls_count += kcontrol->count;
> > > >  	kcontrol->id.numid = card->last_numid + 1;
> > > > -- 
> > > > 1.9.0
> > > > 
> > > > 
> > > [2 Digital signature <application/pgp-signature (7bit)>]
> > > 
> 
> -- 
>