[Sound-open-firmware] [PATCH] component: enforce checking on component state transitions
Liam Girdwood
liam.r.girdwood at linux.intel.com
Thu Oct 12 23:31:02 CEST 2017
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 at 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
--
2.11.0
More information about the Sound-open-firmware
mailing list