On Mon, May 25, 2015 at 06:22:49PM +0100, Liam Girdwood wrote:
This looks really nice at a high level, obviously it's a lot of code so there might be detail problems but I expect we can fix those up later.
@@ -564,6 +565,7 @@ struct snd_soc_dapm_widget { int num_kcontrols; const struct snd_kcontrol_new *kcontrol_news; struct snd_kcontrol **kcontrols;
struct snd_soc_dobj dobj;
/* widget input and outputs */ struct list_head sources;
This adds the dynamic object (which isn't enormous but isn't tiny) to every widget which could add up a bit. I don't know if it's worth making it a pointer to a dobj and allocating that separately? OTOH it's really routes rather than widgets that are the big cost, and it depends on allocators being friendly and not putting lots of padding round dobjs. Definitely fixable incrementally if it is an issue though.
- /* external kcontrol init - used to set ext funcs + private data */
- int (*control_load)(struct snd_soc_component *,
struct snd_kcontrol_new *, struct snd_soc_tplg_ctl_hdr *);- int (*control_unload)(struct snd_soc_component *,
struct snd_soc_dobj *);
Do we have examples of all these external users yet? Just thinking about what happens if we need a load/unload function but the kernel doesn't know about this particular firmware type - the file didn't say code would be needed and perhaps the code is optional anyway (some optimisation or something). Is that something that was thought about yet, what sort of uses are we expecting?
+/* pass vendor data to component driver for processing */ +static int soc_tplg_vendor_load_(struct soc_tplg *tplg,
- struct snd_soc_tplg_hdr *hdr)
+{
- int ret = 0;
- if (tplg->comp && tplg->ops && tplg->ops->vendor_load)
ret = tplg->ops->vendor_load(tplg->comp, hdr);- if (ret < 0)
dev_err(tplg->dev,"ASoC: vendor load failed at hdr offset %ld/0x%lx for type %d:%d\n",soc_tplg_get_hdr_offset(tplg),soc_tplg_get_hdr_offset(tplg),hdr->type, hdr->vendor_type);- return ret;
+}
For example I wonder if we should warn if we don't have a handler for vendor data that we see?