From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH v5 03/12] ALSA: control: Add init callback for kcontrol Date: Mon, 08 Sep 2014 10:04:42 +0200 Message-ID: References: <1409661367-19047-1-git-send-email-subhransu.s.prusty@intel.com> <1409661367-19047-4-git-send-email-subhransu.s.prusty@intel.com> <20140906142124.GG2601@sirena.org.uk> <20140908041421.GI1610@intel.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 051292619E3 for ; Mon, 8 Sep 2014 10:04:45 +0200 (CEST) In-Reply-To: <20140908041421.GI1610@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Vinod Koul Cc: alsa-devel@alsa-project.org, Mark Brown , "Subhransu S. Prusty" , lgirdwood@gmail.com, Lars-Peter Clausen List-Id: alsa-devel@alsa-project.org 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 > > > > Signed-off-by: Vinod Koul > > > > --- > > > > 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 ] > > > > > -- >