On Mon, Aug 17, 2015 at 10:56:36PM +0530, Vinod Koul wrote:
+static int skl_tplg_bind_unbind_pipes(struct skl_module_cfg *src_module,
- struct skl_module_cfg *sink_module, struct skl_sst *ctx, bool bind)
+{
- int ret;
- if (!bind) {
ret = skl_stop_pipe(ctx, src_module->pipe);if (ret < 0)return ret;ret = skl_unbind_modules(ctx, src_module, sink_module);- } else {
ret = skl_bind_modules(ctx, src_module, sink_module);- }
- return ret;
+}
Can we *please* stop having these functions which optionally do several different things with nothing at all shared between the different paths? It just adds a layer of indentation in the function and makes everything harder to read (including at the call site - how does the reader know if a given call will bind or unbind?).
I'm sure I've raised this before.
+static bool skl_tplg_is_pipe_mem_available(struct skl *skl,
struct skl_module_cfg *mconfig)
I'm not seeing any users of this function (unlike the mcps checker).
- if (skl->resource.mem + mconfig->pipe->memory_pages > skl->resource.max_mem) {
dev_err(ctx->dev, "exceeds ppl memory available=%d > mem=%d\n",skl->resource.max_mem, skl->resource.mem);return false;- }
I'm not sure how the user is going to be able to tell what exceeded the maximum memory here.
+static bool skl_tplg_is_pipe_mcps_available(struct skl *skl,
struct skl_module_cfg *mconfig)
This looks an awful lot like the memory check. Can we make a struct or ideally array for the constraints and then have a single function which checks them all?
- list_for_each_entry(p, &w->sinks, list_source) {
if ((p->sink->priv == NULL)&& (!is_skl_dsp_widget_type(w)))continue;if ((p->sink->priv != NULL) && (p->connect)&& (is_skl_dsp_widget_type(p->sink))) {
This is harder to read with the extra brackets.