Make sure we check all component state transitions and report any errors to trace by calling comp_set_state().
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com
comp fixup --- src/audio/component.c | 22 +++++++++++++++++++++- src/audio/dai.c | 12 +++++++++--- src/audio/eq_fir.c | 16 +++++++++++----- src/audio/eq_iir.c | 16 +++++++++++----- src/audio/host.c | 7 ++++++- src/audio/mixer.c | 13 +++++++++---- src/audio/src.c | 7 ++++--- src/audio/tone.c | 14 ++++++++++---- src/audio/volume.c | 20 ++++++++++++++------ src/include/reef/audio/component.h | 2 ++ 10 files changed, 97 insertions(+), 32 deletions(-)
diff --git a/src/audio/component.c b/src/audio/component.c index 80311e3..5815fad 100644 --- a/src/audio/component.c +++ b/src/audio/component.c @@ -142,7 +142,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) break; case COMP_CMD_STOP: if (dev->state == COMP_STATE_ACTIVE) { - dev->state = COMP_STATE_READY; + dev->state = COMP_STATE_PREPARE; } else { trace_comp_error("CEs"); trace_value(dev->state); @@ -159,6 +159,26 @@ int comp_set_state(struct comp_dev *dev, int cmd) ret = -EINVAL; } break; + case COMP_CMD_RESET: + /* reset always succeeds */ + dev->state = COMP_STATE_READY; + if (dev->state == COMP_STATE_ACTIVE || + dev->state == COMP_STATE_PAUSED) { + trace_comp_error("CER"); + trace_value(dev->state); + ret = 0; + } + break; + case COMP_CMD_PREPARE: + if (dev->state == COMP_STATE_PREPARE || + dev->state == COMP_STATE_READY) { + dev->state = COMP_STATE_PREPARE; + } else { + trace_comp_error("CEP"); + trace_value(dev->state); + ret = -EINVAL; + } + break; default: break; } diff --git a/src/audio/dai.c b/src/audio/dai.c index 4f9a082..25049c2 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -396,10 +396,15 @@ static int dai_prepare(struct comp_dev *dev)
trace_dai("pre");
+ ret = comp_set_state(dev, COMP_CMD_PREPARE); + if (ret < 0) + return ret; + dev->position = 0;
if (list_is_empty(&dd->config.elem_list)) { trace_dai_error("wdm"); + comp_set_state(dev, COMP_CMD_RESET); return -EINVAL; }
@@ -412,8 +417,9 @@ static int dai_prepare(struct comp_dev *dev) }
ret = dma_set_config(dd->dma, dd->chan, &dd->config); - if (ret == 0) - dev->state = COMP_STATE_PREPARE; + if (ret < 0) + comp_set_state(dev, COMP_CMD_RESET); + return ret; }
@@ -440,7 +446,7 @@ static int dai_reset(struct comp_dev *dev) dd->last_bytes = 0; dd->wallclock = 0; dev->position = 0; - dev->state = COMP_STATE_READY; + comp_set_state(dev, COMP_CMD_RESET);
return 0; } diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c index 323c702..d1f5072 100644 --- a/src/audio/eq_fir.c +++ b/src/audio/eq_fir.c @@ -468,18 +468,24 @@ static int eq_fir_prepare(struct comp_dev *dev)
trace_eq("EPp");
+ ret = comp_set_state(dev, COMP_CMD_PREPARE); + if (ret < 0) + return ret; + cd->eq_fir_func = eq_fir_s32_default;
/* Initialize EQ */ - if (cd->config == NULL) + if (cd->config == NULL) { + comp_set_state(dev, COMP_CMD_RESET); return -EINVAL; + }
ret = eq_fir_setup(cd->fir, cd->config, dev->params.channels); - if (ret < 0) + if (ret < 0) { + comp_set_state(dev, COMP_CMD_RESET); return ret; + }
- //dev->preload = PLAT_INT_PERIODS; - dev->state = COMP_STATE_PREPARE; return 0; }
@@ -502,7 +508,7 @@ static int eq_fir_reset(struct comp_dev *dev) for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) fir_reset(&cd->fir[i]);
- dev->state = COMP_STATE_INIT; + comp_set_state(dev, COMP_CMD_RESET); return 0; }
diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c index f8e3040..723e5b3 100644 --- a/src/audio/eq_iir.c +++ b/src/audio/eq_iir.c @@ -463,21 +463,27 @@ static int eq_iir_prepare(struct comp_dev *dev)
trace_eq_iir("EPp");
+ ret = comp_set_state(dev, COMP_CMD_PREPARE); + if (ret < 0) + return ret; + cd->eq_iir_func = eq_iir_s32_default;
/* Initialize EQ. Note that if EQ has not received command to * configure the response the EQ prepare returns an error that * interrupts pipeline prepare for downstream. */ - if (cd->config == NULL) + if (cd->config == NULL) { + comp_set_state(dev, COMP_CMD_RESET); return -EINVAL; + }
ret = eq_iir_setup(cd->iir, cd->config, dev->params.channels); - if (ret < 0) + if (ret < 0) { + comp_set_state(dev, COMP_CMD_RESET); return ret; + }
- //dev->preload = PLAT_INT_PERIODS; - dev->state = COMP_STATE_PREPARE; return 0; }
@@ -500,7 +506,7 @@ static int eq_iir_reset(struct comp_dev *dev) for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) iir_reset_df2t(&cd->iir[i]);
- dev->state = COMP_STATE_INIT; + comp_set_state(dev, COMP_CMD_RESET); return 0; }
diff --git a/src/audio/host.c b/src/audio/host.c index 4990f11..14a77d4 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -487,9 +487,14 @@ static int host_prepare(struct comp_dev *dev) { struct host_data *hd = comp_get_drvdata(dev); struct comp_buffer *dma_buffer; + int ret;
trace_host("pre");
+ ret = comp_set_state(dev, COMP_CMD_PREPARE); + if (ret < 0) + return ret; + if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) dma_buffer = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); @@ -505,7 +510,6 @@ static int host_prepare(struct comp_dev *dev) hd->split_remaining = 0; dev->position = 0;
- dev->state = COMP_STATE_PREPARE; return 0; }
@@ -519,6 +523,7 @@ static int host_pointer_reset(struct comp_dev *dev) hd->local_pos = 0; hd->report_pos = 0; dev->position = 0; + comp_set_state(dev, COMP_CMD_RESET);
return 0; } diff --git a/src/audio/mixer.c b/src/audio/mixer.c index 3321684..cf265a7 100644 --- a/src/audio/mixer.c +++ b/src/audio/mixer.c @@ -286,7 +286,7 @@ static int mixer_reset(struct comp_dev *dev) return 1; /* should not reset the downstream components */ }
- dev->state = COMP_STATE_READY; + comp_set_state(dev, COMP_CMD_RESET); return 0; }
@@ -304,13 +304,20 @@ static int mixer_prepare(struct comp_dev *dev) struct list_item * blist; struct comp_buffer *source; int downstream = 0; + int ret;
trace_mixer("pre");
+ /* does mixer already have active source streams ? */ if (dev->state != COMP_STATE_ACTIVE) { + + /* currently inactive so setup mixer */ md->mix_func = mix_n; dev->state = COMP_STATE_PREPARE; - //dev->preload = PLAT_INT_PERIODS; + + ret = comp_set_state(dev, COMP_CMD_PREPARE); + if (ret < 0) + return ret; }
/* check each mixer source state */ @@ -324,8 +331,6 @@ static int mixer_prepare(struct comp_dev *dev) } }
- dev->state = COMP_STATE_PREPARE; - /* prepare downstream */ return downstream; } diff --git a/src/audio/src.c b/src/audio/src.c index 993c76b..4c4bc6e 100644 --- a/src/audio/src.c +++ b/src/audio/src.c @@ -549,8 +549,9 @@ static int src_copy(struct comp_dev *dev)
static int src_prepare(struct comp_dev *dev) { - dev->state = COMP_STATE_PREPARE; - return 0; + trace_src("pre"); + + return comp_set_state(dev, COMP_CMD_PREPARE); }
static int src_preload(struct comp_dev *dev) @@ -569,7 +570,7 @@ static int src_reset(struct comp_dev *dev) for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) src_polyphase_reset(&cd->src[i]);
- dev->state = COMP_STATE_READY; + comp_set_state(dev, COMP_CMD_RESET); return 0; }
diff --git a/src/audio/tone.c b/src/audio/tone.c index 0a91f09..0baf247 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -517,12 +517,17 @@ static int tone_copy(struct comp_dev *dev)
static int tone_prepare(struct comp_dev *dev) { + struct comp_data *cd = comp_get_drvdata(dev); int32_t f; int32_t a; - struct comp_data *cd = comp_get_drvdata(dev); + int ret;
trace_tone("TPp");
+ ret = comp_set_state(dev, COMP_CMD_PREPARE); + if (ret < 0) + return ret; + cd->channels = dev->params.channels; cd->rate = dev->params.rate; tracev_value(cd->channels); @@ -530,10 +535,11 @@ static int tone_prepare(struct comp_dev *dev)
f = tonegen_get_f(&cd->sg); a = tonegen_get_a(&cd->sg); - if (tonegen_init(&cd->sg, cd->rate, f, a) < 0) + if (tonegen_init(&cd->sg, cd->rate, f, a) < 0) { + comp_set_state(dev, COMP_CMD_RESET); return -EINVAL; + }
- dev->state = COMP_STATE_PREPARE; return 0; }
@@ -552,7 +558,7 @@ static int tone_reset(struct comp_dev *dev) /* Initialize with the defaults */ tonegen_reset(&cd->sg);
- dev->state = COMP_STATE_READY; + comp_set_state(dev, COMP_CMD_RESET);
return 0; } diff --git a/src/audio/volume.c b/src/audio/volume.c index 04d1b34..472aab9 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -566,6 +566,10 @@ static int volume_prepare(struct comp_dev *dev)
trace_volume("pre");
+ ret = comp_set_state(dev, COMP_CMD_PREPARE); + if (ret < 0) + return ret; + /* volume components will only ever have 1 source and 1 sink buffer */ sourceb = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); sinkb = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); @@ -617,7 +621,7 @@ static int volume_prepare(struct comp_dev *dev) config->periods_sink); if (ret < 0) { trace_volume_error("vp0"); - return ret; + goto err; }
buffer_reset_pos(sinkb); @@ -627,13 +631,15 @@ static int volume_prepare(struct comp_dev *dev) trace_volume_error("vp1"); trace_value(dev->frames); trace_value(sinkb->sink->frame_bytes); - return -EINVAL; + ret = -EINVAL; + goto err; } if (cd->source_period_bytes == 0) { trace_volume_error("vp2"); trace_value(dev->frames); trace_value(sourceb->source->frame_bytes); - return -EINVAL; + ret = -EINVAL; + goto err; }
/* map the volume function for source and sink buffers */ @@ -653,13 +659,15 @@ static int volume_prepare(struct comp_dev *dev) trace_value(cd->source_format); trace_value(cd->sink_format); trace_value(dev->params.channels); - return -EINVAL; + +err: + comp_set_state(dev, COMP_CMD_RESET); + return ret;
found: for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) vol_sync_host(cd, i);
- dev->state = COMP_STATE_PREPARE; return 0; }
@@ -683,7 +691,7 @@ static int volume_reset(struct comp_dev *dev) { trace_volume("res");
- dev->state = COMP_STATE_READY; + comp_set_state(dev, COMP_CMD_RESET); return 0; }
diff --git a/src/include/reef/audio/component.h b/src/include/reef/audio/component.h index 436a758..db5f35b 100644 --- a/src/include/reef/audio/component.h +++ b/src/include/reef/audio/component.h @@ -78,6 +78,8 @@ #define COMP_CMD_RELEASE 3 /* release paused component stream */ #define COMP_CMD_SUSPEND 5 /* suspend component */ #define COMP_CMD_RESUME 6 /* resume component */ +#define COMP_CMD_RESET 6 /* reset component */ +#define COMP_CMD_PREPARE 7 /* prepare component */
/* * standard component control commands