On Fri, Aug 14, 2015 at 11:03:13PM +0100, Mark Brown wrote:
On Sat, Aug 08, 2015 at 01:06:20AM +0530, Subhransu S. Prusty wrote:
- struct skl_pipeline *ppl;
- pipe = devm_kzalloc(dev, sizeof(*pipe), GFP_KERNEL);
There's lots of random spaces in this code so far and some further down too.
sorry about that, will fix
- struct skl_dfw_module *dfw_config = (struct skl_dfw_module *) tplg_w->priv.data;
- skl_tplg_dump_widget_info(bus->dev, dfw_config, tplg_w);
Consider implementing debugfs for this...
yes that a good idea, will drop this here.
+/* This will be read from topology manifest, currently defined here */ +#define SKL_MAX_MCPS 30000000 +#define SKL_FW_MAX_MEM 1000000
Oh dear, this sounds like we need another ABI update to add these manifests?
Not yet. So we will add this and few other stuff in topology manifest vendor data for Intel. So Topology Kernel ABI will not get modified with this. Yes we need to start adding these data sets to Kernel ABI, but I am planning to add that, one shot at the end of SKL cycle so that we don't have to modify ABI defines.
- dev_dbg(bus->dev, "In%s req firmware topology bin\n", __func__);
Those "In%s" are going to come out as the function name prefixed with In. What's that for? It's just going to make the logs harder to read as far as I can tell :(
Will fix this
- const struct firmware *fw;
- ret = request_firmware(&fw, "dfw_sst.bin", bus->dev);
- if (fw == NULL) {
dev_err(bus->dev, "config firmware request failed with %d\n", ret);return ret;- }
We're ignoring the return value (which is what we should be paying attention to here) and checking to see if fw is NULL but fw wasn't initialized :(
Yes, will fix
- /* Index is for each config load */
- ret = snd_soc_tplg_component_load(&platform->component, &skl_tplg_ops, fw, 0);
Which index?
The last arg of snd_soc_tplg_component_load() is id which is set as tplg.req_index = id;
So the comment tries to explain how last 0 index is added. We have only one load so we will be always 0 index
Thanks