[Sound-open-firmware] [PATCH 1/3] trace: core: add trace_error_value() and update users
This patch adds a new trace feature to send any error states/values after the trace messages to mbox and modifes users of trace_value() to use trace_error_value().
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- Tested with Minnowboard Turbot with RT5651 Kernel: https://github.com/plbossart/sound.git branch: topic/sof-v4.14 SOF: https://github.com/ranj063/sof.git branch: debug/trace_test SOFT: 1.1-stable --- --- src/audio/buffer.c | 2 +- src/audio/component.c | 14 +++++++------- src/audio/dai.c | 18 +++++++++--------- src/audio/eq_fir.c | 2 +- src/audio/eq_iir.c | 2 +- src/audio/host.c | 6 +++--- src/audio/pipeline.c | 16 ++++++++-------- src/audio/src.c | 14 +++++++------- src/audio/volume.c | 14 +++++++------- src/drivers/dw-dma.c | 12 ++++++------ src/drivers/hda-dma.c | 4 ++-- src/include/reef/audio/component.h | 4 ++-- src/include/reef/trace.h | 6 ++++++ src/include/reef/wait.h | 4 ++-- src/ipc/intel-ipc.c | 24 ++++++++++++------------ src/ipc/ipc.c | 16 ++++++++-------- src/lib/alloc.c | 4 ++-- 17 files changed, 84 insertions(+), 78 deletions(-)
diff --git a/src/audio/buffer.c b/src/audio/buffer.c index 360ad6b..a5d92b2 100644 --- a/src/audio/buffer.c +++ b/src/audio/buffer.c @@ -55,7 +55,7 @@ struct comp_buffer *buffer_new(struct sof_ipc_buffer *desc) /* validate request */ if (desc->size == 0 || desc->size > HEAP_BUFFER_SIZE) { trace_buffer_error("ebg"); - trace_value(desc->size); + trace_error_value(desc->size); return NULL; }
diff --git a/src/audio/component.c b/src/audio/component.c index f27f350..1802278 100644 --- a/src/audio/component.c +++ b/src/audio/component.c @@ -79,7 +79,7 @@ struct comp_dev *comp_new(struct sof_ipc_comp *comp) drv = get_drv(comp->type); if (drv == NULL) { trace_comp_error("eCD"); - trace_value(comp->type); + trace_error_value(comp->type); return NULL; }
@@ -127,7 +127,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_ACTIVE; } else { trace_comp_error("CES"); - trace_value(dev->state); + trace_error_value(dev->state); ret = -EINVAL; } break; @@ -136,7 +136,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_ACTIVE; } else { trace_comp_error("CEr"); - trace_value(dev->state); + trace_error_value(dev->state); ret = -EINVAL; } break; @@ -146,7 +146,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_PREPARE; } else { trace_comp_error("CEs"); - trace_value(dev->state); + trace_error_value(dev->state); ret = -EINVAL; } break; @@ -156,7 +156,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_PAUSED; else { trace_comp_error("CEp"); - trace_value(dev->state); + trace_error_value(dev->state); ret = -EINVAL; } break; @@ -166,7 +166,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) if (dev->state == COMP_STATE_ACTIVE || dev->state == COMP_STATE_PAUSED) { trace_comp_error("CER"); - trace_value(dev->state); + trace_error_value(dev->state); ret = 0; } break; @@ -176,7 +176,7 @@ int comp_set_state(struct comp_dev *dev, int cmd) dev->state = COMP_STATE_PREPARE; } else { trace_comp_error("CEP"); - trace_value(dev->state); + trace_error_value(dev->state); ret = -EINVAL; } break; diff --git a/src/audio/dai.c b/src/audio/dai.c index 061af5e..f1b4a8e 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -278,10 +278,10 @@ static int dai_playback_params(struct comp_dev *dev) err = buffer_set_size(dma_buffer, buffer_size); if (err < 0) { trace_dai_error("ep1"); - trace_value(source_config->periods_sink); - trace_value(dd->period_bytes); - trace_value(buffer_size); - trace_value(dma_buffer->alloc_size); + trace_error_value(source_config->periods_sink); + trace_error_value(dd->period_bytes); + trace_error_value(buffer_size); + trace_error_value(dma_buffer->alloc_size); return err; }
@@ -346,10 +346,10 @@ static int dai_capture_params(struct comp_dev *dev) err = buffer_set_size(dma_buffer, buffer_size); if (err < 0) { trace_dai_error("ec1"); - trace_value(sink_config->periods_sink); - trace_value(dd->period_bytes); - trace_value(buffer_size); - trace_value(dma_buffer->alloc_size); + trace_error_value(sink_config->periods_sink); + trace_error_value(dd->period_bytes); + trace_error_value(buffer_size); + trace_error_value(dma_buffer->alloc_size); return err; }
@@ -576,7 +576,7 @@ static int dai_cmd(struct comp_dev *dev, int cmd, void *data) ret = wait_for_completion_timeout(&dd->complete); if (ret < 0) { trace_dai_error("ed0"); - trace_value(cmd); + trace_error_value(cmd); } break; default: diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c index b6ec58f..52988cd 100644 --- a/src/audio/eq_fir.c +++ b/src/audio/eq_fir.c @@ -386,7 +386,7 @@ static int fir_cmd_set_data(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdat } } else { trace_eq_error("une"); - trace_value(cdata->index); + trace_error_value(cdata->index); return -EINVAL; } break; diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c index 2a1dd59..bbcd66b 100644 --- a/src/audio/eq_iir.c +++ b/src/audio/eq_iir.c @@ -382,7 +382,7 @@ static int iir_cmd_set_data(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdat } } else { trace_eq_iir_error("une"); - trace_value(cdata->index); + trace_error_value(cdata->index); return -EINVAL; } break; diff --git a/src/audio/host.c b/src/audio/host.c index 744ca02..9177d99 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -671,15 +671,15 @@ static int host_params(struct comp_dev *dev) err = buffer_set_size(hd->dma_buffer, buffer_size); if (err < 0) { trace_host_error("eSz"); - trace_value(buffer_size); + trace_error_value(buffer_size); return err; }
/* component buffer size must be divisor of host buffer size */ if (hd->host_size % hd->period_bytes) { trace_comp_error("eHB"); - trace_value(hd->host_size); - trace_value(hd->period_bytes); + trace_error_value(hd->host_size); + trace_error_value(hd->period_bytes); return -EINVAL; }
diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index c7f8aed..1b7f248 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -392,7 +392,7 @@ static int component_op_downstream(struct op_data *op_data, case COMP_OPS_BUFFER: /* handled by other API call */ default: trace_pipe_error("eOi"); - trace_value(op_data->op); + trace_error_value(op_data->op); return -EINVAL; }
@@ -473,7 +473,7 @@ static int component_op_upstream(struct op_data *op_data, case COMP_OPS_BUFFER: /* handled by other API call */ default: trace_pipe_error("eOi"); - trace_value(op_data->op); + trace_error_value(op_data->op); return -EINVAL; }
@@ -647,8 +647,8 @@ int pipeline_cmd(struct pipeline *p, struct comp_dev *host, int cmd,
if (ret < 0) { trace_ipc_error("pc0"); - trace_value(host->comp.id); - trace_value(cmd); + trace_error_value(host->comp.id); + trace_error_value(cmd); }
spin_unlock(&p->lock); @@ -693,7 +693,7 @@ int pipeline_params(struct pipeline *p, struct comp_dev *host,
if (ret < 0) { trace_ipc_error("pp0"); - trace_value(host->comp.id); + trace_error_value(host->comp.id); }
spin_unlock(&p->lock); @@ -723,7 +723,7 @@ int pipeline_reset(struct pipeline *p, struct comp_dev *host)
if (ret < 0) { trace_ipc_error("pr0"); - trace_value(host->comp.id); + trace_error_value(host->comp.id); }
spin_unlock(&p->lock); @@ -774,7 +774,7 @@ static int pipeline_copy_from_upstream(struct comp_dev *start, err = pipeline_copy_from_upstream(start, buffer->source); if (err < 0) { trace_pipe_error("ePU"); - trace_value(current->comp.id); + trace_error_value(current->comp.id); return err; } } @@ -837,7 +837,7 @@ static int pipeline_copy_to_downstream(struct comp_dev *start, err = pipeline_copy_to_downstream(start, buffer->sink); if (err < 0) { trace_pipe_error("ePD"); - trace_value(current->comp.id); + trace_error_value(current->comp.id); return err; } } diff --git a/src/audio/src.c b/src/audio/src.c index cca0cbc..a579fa9 100644 --- a/src/audio/src.c +++ b/src/audio/src.c @@ -645,10 +645,10 @@ static int src_params(struct comp_dev *dev) params->channels, dev->frames, frames_is_for_source); if (err < 0) { trace_src_error("sr1"); - trace_value(source_rate); - trace_value(sink_rate); - trace_value(params->channels); - trace_value(dev->frames); + trace_error_value(source_rate); + trace_error_value(sink_rate); + trace_error_value(params->channels); + trace_error_value(dev->frames); return err; }
@@ -666,7 +666,7 @@ static int src_params(struct comp_dev *dev) delay_lines_size); if (!cd->delay_lines) { trace_src_error("sr3"); - trace_value(delay_lines_size); + trace_error_value(delay_lines_size); return -EINVAL; }
@@ -721,8 +721,8 @@ static int src_params(struct comp_dev *dev) err = buffer_set_size(sink, q * dev->frames * dev->frame_bytes); if (err < 0) { trace_src_error("eSz"); - trace_value(sink->alloc_size); - trace_value(q * dev->frames * dev->frame_bytes); + trace_error_value(sink->alloc_size); + trace_error_value(q * dev->frames * dev->frame_bytes); return err; }
diff --git a/src/audio/volume.c b/src/audio/volume.c index df926c5..1a3f436 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -943,15 +943,15 @@ static int volume_prepare(struct comp_dev *dev) /* validate */ if (cd->sink_period_bytes == 0) { trace_volume_error("vp1"); - trace_value(dev->frames); - trace_value(sinkb->sink->frame_bytes); + trace_error_value(dev->frames); + trace_error_value(sinkb->sink->frame_bytes); ret = -EINVAL; goto err; } if (cd->source_period_bytes == 0) { trace_volume_error("vp2"); - trace_value(dev->frames); - trace_value(sourceb->source->frame_bytes); + trace_error_value(dev->frames); + trace_error_value(sourceb->source->frame_bytes); ret = -EINVAL; goto err; } @@ -970,9 +970,9 @@ static int volume_prepare(struct comp_dev *dev) goto found; } trace_volume_error("vp3"); - trace_value(cd->source_format); - trace_value(cd->sink_format); - trace_value(dev->params.channels); + trace_error_value(cd->source_format); + trace_error_value(cd->sink_format); + trace_error_value(dev->params.channels);
err: comp_set_state(dev, COMP_CMD_RESET); diff --git a/src/drivers/dw-dma.c b/src/drivers/dw-dma.c index 8cb8e31..1a3c7a9 100644 --- a/src/drivers/dw-dma.c +++ b/src/drivers/dw-dma.c @@ -374,9 +374,9 @@ static int dw_dma_start(struct dma *dma, int channel) (dw_read(dma, DW_DMA_CHAN_EN) & (0x1 << channel))) { ret = -EBUSY; trace_dma_error("eS0"); - trace_value(dw_read(dma, DW_DMA_CHAN_EN)); - trace_value(dw_read(dma, DW_CFG_LOW(channel))); - trace_value(p->chan[channel].status); + trace_error_value(dw_read(dma, DW_DMA_CHAN_EN)); + trace_error_value(dw_read(dma, DW_CFG_LOW(channel))); + trace_error_value(p->chan[channel].status); goto out; }
@@ -480,7 +480,7 @@ static int dw_dma_stop(struct dma *dma, int channel) /* is channel stii active ? */ if ((dw_read(dma, DW_DMA_CHAN_EN) & (0x1 << channel))) { trace_dma_error("ea0"); - trace_value(channel); + trace_error_value(channel); }
p->chan[channel].status = COMP_STATE_PREPARE; @@ -912,7 +912,7 @@ static void dw_dma_irq_handler(void *data) status_intr = dw_read(dma, DW_INTR_STATUS); if (!status_intr) { trace_dma_error("eDI"); - trace_value(status_intr); + trace_error_value(status_intr); }
tracev_dma("irq"); @@ -1078,7 +1078,7 @@ static void dw_dma_irq_handler(void *data) status_block_new = dw_read(dma, DW_STATUS_BLOCK); if (status_block_new) { trace_dma_error("eI2"); - trace_value(status_block_new); + trace_error_value(status_block_new); }
for (i = 0; i < DW_MAX_CHAN; i++) { diff --git a/src/drivers/hda-dma.c b/src/drivers/hda-dma.c index caea70e..c00fb2e 100644 --- a/src/drivers/hda-dma.c +++ b/src/drivers/hda-dma.c @@ -192,8 +192,8 @@ static int hda_dma_start(struct dma *dma, int channel) (dgcs & DGCS_GEN)) { ret = -EBUSY; trace_host_error("eS0"); - trace_value(dgcs); - trace_value(p->chan[channel].status); + trace_error_value(dgcs); + trace_error_value(p->chan[channel].status); goto out; }
diff --git a/src/include/reef/audio/component.h b/src/include/reef/audio/component.h index f25ad3d..538d476 100644 --- a/src/include/reef/audio/component.h +++ b/src/include/reef/audio/component.h @@ -250,8 +250,8 @@ static inline int comp_cmd(struct comp_dev *dev, int cmd, void *data) && ((cdata->data->magic != SOF_ABI_MAGIC) || (cdata->data->abi != SOF_ABI_VERSION))) { trace_comp_error("abi"); - trace_value(cdata->data->magic); - trace_value(cdata->data->abi); + trace_error_value(cdata->data->magic); + trace_error_value(cdata->data->abi); return -EINVAL; }
diff --git a/src/include/reef/trace.h b/src/include/reef/trace.h index 2e22b16..7afbd23 100644 --- a/src/include/reef/trace.h +++ b/src/include/reef/trace.h @@ -103,6 +103,7 @@
void _trace_event(uint32_t event); void _trace_error(uint32_t event); +void _trace_error_value(uint32_t event); void _trace_event_atomic(uint32_t event); void _trace_error_atomic(uint32_t event); void trace_off(void); @@ -139,9 +140,14 @@ void trace_init(struct reef * reef); _trace_error_atomic(__c | (__e[0] << 16) | (__e[1] <<8) | __e[2]) #define trace_error_atomic(__c, __e) \ _trace_error_atomic(__c | (__e[0] << 16) | (__e[1] <<8) | __e[2]) +/* write back error value to mbox*/ +#define trace_error_value(x) _trace_error_atomic(x) +#define trace_error_value_atomic(x) _trace_error_atomic(x) #else #define trace_error(__c, __e) #define trace_error_atomic(__c, __e) +#define trace_error_value(x) +#define trace_error_value_atomic(x) #endif
#else diff --git a/src/include/reef/wait.h b/src/include/reef/wait.h index a598d48..e9737ec 100644 --- a/src/include/reef/wait.h +++ b/src/include/reef/wait.h @@ -139,8 +139,8 @@ static inline int wait_for_completion_timeout(completion_t *comp) return 0; } else { /* timeout */ - trace_value(c->timeout); - trace_value(c->complete); + trace_error_value(c->timeout); + trace_error_value(c->complete); return -ETIME; } } diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c index fbe0b6e..5c062be 100644 --- a/src/ipc/intel-ipc.c +++ b/src/ipc/intel-ipc.c @@ -238,14 +238,14 @@ static int ipc_stream_pcm_params(uint32_t stream) pcm_dev = ipc_get_comp(_ipc, pcm_params->comp_id); if (pcm_dev == NULL) { trace_ipc_error("eAC"); - trace_value(pcm_params->comp_id); + trace_error_value(pcm_params->comp_id); return -EINVAL; }
/* sanity check comp */ if (pcm_dev->cd->pipeline == NULL) { trace_ipc_error("eA1"); - trace_value(pcm_params->comp_id); + trace_error_value(pcm_params->comp_id); return -EINVAL; }
@@ -350,7 +350,7 @@ static int ipc_stream_pcm_free(uint32_t header) /* sanity check comp */ if (pcm_dev->cd->pipeline == NULL) { trace_ipc_error("eF1"); - trace_value(free_req->comp_id); + trace_error_value(free_req->comp_id); return -EINVAL; }
@@ -470,7 +470,7 @@ static int ipc_stream_trigger(uint32_t header) cmd, NULL); if (ret < 0) { trace_ipc_error("eRc"); - trace_value(ipc_cmd); + trace_error_value(ipc_cmd); }
return ret; @@ -516,8 +516,8 @@ static int ipc_dai_config(uint32_t header) dai = dai_get(config->type, config->id); if (dai == NULL) { trace_ipc_error("eDi"); - trace_value(config->type); - trace_value(config->id); + trace_error_value(config->type); + trace_error_value(config->id); return -ENODEV; }
@@ -543,7 +543,7 @@ static int ipc_glb_dai_message(uint32_t header) //return ipc_comp_set_value(header, COMP_CMD_LOOPBACK); default: trace_ipc_error("eDc"); - trace_value(header); + trace_error_value(header); return -EINVAL; } } @@ -747,7 +747,7 @@ static int ipc_glb_debug_message(uint32_t header) return ipc_dma_trace_config(header); default: trace_ipc_error("eDc"); - trace_value(header); + trace_error_value(header); return -EINVAL; } } @@ -769,7 +769,7 @@ static int ipc_comp_value(uint32_t header, uint32_t cmd) comp_dev = ipc_get_comp(_ipc, data->comp_id); if (comp_dev == NULL){ trace_ipc_error("eVg"); - trace_value(data->comp_id); + trace_error_value(data->comp_id); return -ENODEV; } @@ -800,7 +800,7 @@ static int ipc_glb_comp_message(uint32_t header) return ipc_comp_value(header, COMP_CMD_GET_DATA); default: trace_ipc_error("eCc"); - trace_value(header); + trace_error_value(header); return -EINVAL; } } @@ -929,7 +929,7 @@ static int ipc_glb_tplg_message(uint32_t header) return ipc_glb_tplg_free(header, ipc_buffer_free); default: trace_ipc_error("eTc"); - trace_value(header); + trace_error_value(header); return -EINVAL; } } @@ -970,7 +970,7 @@ int ipc_cmd(void) return ipc_glb_debug_message(hdr->cmd); default: trace_ipc_error("eGc"); - trace_value(type); + trace_error_value(type); return -EINVAL; } } diff --git a/src/ipc/ipc.c b/src/ipc/ipc.c index 762bbf5..0b52cb2 100644 --- a/src/ipc/ipc.c +++ b/src/ipc/ipc.c @@ -108,7 +108,7 @@ int ipc_comp_new(struct ipc *ipc, struct sof_ipc_comp *comp) icd = ipc_get_comp(ipc, comp->id); if (icd != NULL) { trace_ipc_error("eCe"); - trace_value(comp->id); + trace_error_value(comp->id); return -EINVAL; }
@@ -162,7 +162,7 @@ int ipc_buffer_new(struct ipc *ipc, struct sof_ipc_buffer *desc) ibd = ipc_get_comp(ipc, desc->comp.id); if (ibd != NULL) { trace_ipc_error("eBe"); - trace_value(desc->comp.id); + trace_error_value(desc->comp.id); return -EINVAL; }
@@ -215,14 +215,14 @@ int ipc_comp_connect(struct ipc *ipc, icd_source = ipc_get_comp(ipc, connect->source_id); if (icd_source == NULL) { trace_ipc_error("eCr"); - trace_value(connect->source_id); + trace_error_value(connect->source_id); return -EINVAL; }
icd_sink = ipc_get_comp(ipc, connect->sink_id); if (icd_sink == NULL) { trace_ipc_error("eCn"); - trace_value(connect->sink_id); + trace_error_value(connect->sink_id); return -EINVAL; }
@@ -237,8 +237,8 @@ int ipc_comp_connect(struct ipc *ipc, icd_source->cd, icd_sink->cb); else { trace_ipc_error("eCt"); - trace_value(connect->source_id); - trace_value(connect->sink_id); + trace_error_value(connect->source_id); + trace_error_value(connect->sink_id); return -EINVAL; } } @@ -255,7 +255,7 @@ int ipc_pipeline_new(struct ipc *ipc, ipc_pipe = ipc_get_comp(ipc, pipe_desc->comp_id); if (ipc_pipe != NULL) { trace_ipc_error("ePi"); - trace_value(pipe_desc->comp_id); + trace_error_value(pipe_desc->comp_id); return -EINVAL; }
@@ -263,7 +263,7 @@ int ipc_pipeline_new(struct ipc *ipc, icd = ipc_get_comp(ipc, pipe_desc->sched_id); if (icd == NULL) { trace_ipc_error("ePs"); - trace_value(pipe_desc->sched_id); + trace_error_value(pipe_desc->sched_id); return -EINVAL; } if (icd->type != COMP_TYPE_COMPONENT) { diff --git a/src/lib/alloc.c b/src/lib/alloc.c index 27916e3..4803456 100644 --- a/src/lib/alloc.c +++ b/src/lib/alloc.c @@ -378,8 +378,8 @@ find:
error: trace_mem_error("eMm"); - trace_value(bytes); - trace_value(caps); + trace_error_value(bytes); + trace_error_value(caps); return NULL; }
This patch adds a macro that can be used to enable sending all trace to mbox.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- Tested with Minnowboard Turbot with RT5651 Kernel: https://github.com/plbossart/sound.git branch: topic/sof-v4.14 SOF: https://github.com/ranj063/sof.git branch: debug/trace_test SOFT: 1.1-stable --- --- src/include/reef/trace.h | 12 ++++++++- src/lib/trace.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/src/include/reef/trace.h b/src/include/reef/trace.h index 7afbd23..f3f4dff 100644 --- a/src/include/reef/trace.h +++ b/src/include/reef/trace.h @@ -100,22 +100,32 @@ #define TRACE 1 #define TRACEV 0 #define TRACEE 1 +#define TRACEM 0 /* send all trace messages to mbox */
void _trace_event(uint32_t event); +void _trace_event_mbox(uint32_t event); void _trace_error(uint32_t event); void _trace_error_value(uint32_t event); void _trace_event_atomic(uint32_t event); +void _trace_event_mbox_atomic(uint32_t event); void _trace_error_atomic(uint32_t event); void trace_off(void); void trace_init(struct reef * reef);
#if TRACE
+/* send all trace to mbox */ +#if TRACEM +#define trace_event(__c, __e) \ + _trace_event_mbox(__c | (__e[0] << 16) | (__e[1] << 8) | __e[2]) +#define trace_event_atomic(__c, __e) \ + _trace_event_mbox_atomic(__c | (__e[0] << 16) | (__e[1] << 8) | __e[2]) +#else #define trace_event(__c, __e) \ _trace_event(__c | (__e[0] << 16) | (__e[1] <<8) | __e[2]) #define trace_event_atomic(__c, __e) \ _trace_event_atomic(__c | (__e[0] << 16) | (__e[1] <<8) | __e[2]) - +#endif #define trace_value(x) _trace_event(x) #define trace_value_atomic(x) _trace_event_atomic(x)
diff --git a/src/lib/trace.c b/src/lib/trace.c index de99a4e..bd0d376 100644 --- a/src/lib/trace.c +++ b/src/lib/trace.c @@ -136,6 +136,71 @@ void _trace_event_atomic(uint32_t event) dtrace_event_atomic((const char*)dt, sizeof(uint64_t) * 2); }
+void _trace_event_mbox(uint32_t event) +{ + unsigned long flags; + uint64_t dt[2]; + uint64_t time; + + volatile uint64_t *t; + + if (!trace.enable) + return; + + time = platform_timer_get(platform_timer); + + dt[0] = time; + dt[1] = event; + dtrace_event((const char *)dt, sizeof(uint64_t) * 2); + + /* send event by mail box too. */ + spin_lock_irq(&trace.lock, flags); + + /* write timestamp and event to trace buffer */ + t = (volatile uint64_t *)(MAILBOX_TRACE_BASE + trace.pos); + trace.pos += (sizeof(uint64_t) << 1); + + if (trace.pos > MAILBOX_TRACE_SIZE - sizeof(uint64_t) * 2) + trace.pos = 0; + + spin_unlock_irq(&trace.lock, flags); + + t[0] = time; + t[1] = event; + + /* writeback trace data */ + dcache_writeback_region((void *)t, sizeof(uint64_t) * 2); +} + +void _trace_event_mbox_atomic(uint32_t event) +{ + volatile uint64_t *t; + uint64_t dt[2]; + uint64_t time; + + if (!trace.enable) + return; + + time = platform_timer_get(platform_timer); + + dt[0] = time; + dt[1] = event; + dtrace_event_atomic((const char *)dt, sizeof(uint64_t) * 2); + + /* write timestamp and event to trace buffer */ + t = (volatile uint64_t *)(MAILBOX_TRACE_BASE + trace.pos); + trace.pos += (sizeof(uint64_t) << 1); + + if (trace.pos > MAILBOX_TRACE_SIZE - sizeof(uint64_t) * 2) + trace.pos = 0; + + t[0] = time; + t[1] = event; + + /* writeback trace data */ + dcache_writeback_region((void *)t, sizeof(uint64_t) * 2); +} + void trace_off(void) { trace.enable = 0;
On Sun, 2018-03-25 at 17:34 -0700, Ranjani Sridharan wrote:
May it be unecessary to send it DMA trace local buffer stil if we have selected to send all trace to mbox already?
It may have the same question.
On Sun, 2018-03-25 at 17:34 -0700, Ranjani Sridharan wrote:
This patch adds a macro that can be used to enable sending all trace to mbox.
Does this also keep sending data to DMA trace too ? It needs to send data to both like trace_error()
Liam
On Mon, 2018-03-26 at 06:56 -0700, Ranjani Sridharan wrote:
ok, great, I wan't sure as two new functions had been added. I thought you would have reused the existing functions used for errors ?
Liam
On Mon, 2018-03-26 at 15:01 +0100, Liam Girdwood wrote:
The function used for errors had the name error in them and I thought it would be confusing to use the same function. But I agree there is a lot of repeated code in trace.c. I can work on cleaning it up to reuse code. I will send out a v2 soon.
This patch adds a new trace_flush() feature to trace to flush the last remaining trace messages during panic.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- Tested with Minnowboard Turbot with RT5651 Kernel: https://github.com/plbossart/sound.git branch: topic/sof-v4.14 SOF: https://github.com/ranj063/sof.git branch: debug/trace_test SOFT: 1.1-stable --- --- src/include/reef/dma-trace.h | 1 + src/include/reef/panic.h | 8 +++++++- src/include/reef/trace.h | 1 + src/lib/dma-trace.c | 20 ++++++++++++++++++++ src/lib/trace.c | 11 +++++++++++ src/platform/apollolake/include/platform/platform.h | 3 +++ src/platform/baytrail/include/platform/platform.h | 3 +++ src/platform/cannonlake/include/platform/platform.h | 3 +++ src/platform/haswell/include/platform/platform.h | 3 +++ 9 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/src/include/reef/dma-trace.h b/src/include/reef/dma-trace.h index f17ffe5..bf6e3f3 100644 --- a/src/include/reef/dma-trace.h +++ b/src/include/reef/dma-trace.h @@ -72,6 +72,7 @@ int dma_trace_init_complete(struct dma_trace_data *d); int dma_trace_host_buffer(struct dma_trace_data *d, struct dma_sg_elem *elem, uint32_t host_size); int dma_trace_enable(struct dma_trace_data *d); +void dma_trace_flush(void *t);
void dtrace_event(const char *e, uint32_t size); void dtrace_event_atomic(const char *e, uint32_t length); diff --git a/src/include/reef/panic.h b/src/include/reef/panic.h index e8b4ae2..71df1b8 100644 --- a/src/include/reef/panic.h +++ b/src/include/reef/panic.h @@ -34,6 +34,7 @@ #include <reef/reef.h> #include <reef/mailbox.h> #include <reef/interrupt.h> +#include <reef/trace.h> #include <platform/platform.h> #include <uapi/ipc.h> #include <stdint.h> @@ -59,8 +60,13 @@ static inline void panic_rewind(uint32_t p, uint32_t stack_rewind_frames)
/* TODO: send IPC oops message to host */
- /* panic and loop forever */ + /* panic */ platform_panic(p); + + /* flush last trace messages */ + trace_flush(); + + /* and loop forever */ while (1) {}; }
diff --git a/src/include/reef/trace.h b/src/include/reef/trace.h index f3f4dff..c915be8 100644 --- a/src/include/reef/trace.h +++ b/src/include/reef/trace.h @@ -109,6 +109,7 @@ void _trace_error_value(uint32_t event); void _trace_event_atomic(uint32_t event); void _trace_event_mbox_atomic(uint32_t event); void _trace_error_atomic(uint32_t event); +void trace_flush(void); void trace_off(void); void trace_init(struct reef * reef);
diff --git a/src/lib/dma-trace.c b/src/lib/dma-trace.c index 1ff2bd4..a61f1f2 100644 --- a/src/lib/dma-trace.c +++ b/src/lib/dma-trace.c @@ -36,6 +36,7 @@ #include <arch/cache.h> #include <platform/timer.h> #include <platform/dma.h> +#include <platform/platform.h> #include <reef/lock.h> #include <stdint.h>
@@ -285,6 +286,25 @@ int dma_trace_enable(struct dma_trace_data *d) return 0; }
+void dma_trace_flush(void *t) +{ + struct dma_trace_buf *buffer = &trace_data->dmatb; + uint32_t avail = buffer->avail; + int32_t size; + + /* number of bytes to flush */ + if (avail > DMA_FLUSH_TRACE_SIZE) { + size = DMA_FLUSH_TRACE_SIZE; + memcpy(t, buffer->w_ptr - DMA_FLUSH_TRACE_SIZE, size); + } else { + size = buffer->w_ptr - buffer->r_ptr; + memcpy(t, buffer->r_ptr, size); + } + + /* writeback trace data */ + dcache_writeback_region(t, size); +} + static void dtrace_add_event(const char *e, uint32_t length) { struct dma_trace_buf *buffer = &trace_data->dmatb; diff --git a/src/lib/trace.c b/src/lib/trace.c index bd0d376..b2082dd 100644 --- a/src/lib/trace.c +++ b/src/lib/trace.c @@ -201,6 +201,17 @@ void _trace_event_mbox_atomic(uint32_t event) dcache_writeback_region((void *)t, sizeof(uint64_t) * 2); }
+void trace_flush(void) +{ + volatile uint64_t *t; + + /* get mailbox position */ + t = (volatile uint64_t *)(MAILBOX_TRACE_BASE + trace.pos); + + /* flush dma trace messages */ + dma_trace_flush((void *)t); +} + void trace_off(void) { trace.enable = 0; diff --git a/src/platform/apollolake/include/platform/platform.h b/src/platform/apollolake/include/platform/platform.h index a1dfba2..86172bf 100644 --- a/src/platform/apollolake/include/platform/platform.h +++ b/src/platform/apollolake/include/platform/platform.h @@ -88,6 +88,9 @@ struct reef; /* local buffer size of DMA tracing */ #define DMA_TRACE_LOCAL_SIZE HOST_PAGE_SIZE
+/* trace bytes flushed during panic */ +#define DMA_FLUSH_TRACE_SIZE (MAILBOX_TRACE_SIZE >> 2) + /* the interval of DMA trace copying */ #define DMA_TRACE_PERIOD 500000
diff --git a/src/platform/baytrail/include/platform/platform.h b/src/platform/baytrail/include/platform/platform.h index 9a6967b..f3464f5 100644 --- a/src/platform/baytrail/include/platform/platform.h +++ b/src/platform/baytrail/include/platform/platform.h @@ -82,6 +82,9 @@ struct reef; /* local buffer size of DMA tracing */ #define DMA_TRACE_LOCAL_SIZE HOST_PAGE_SIZE
+/* trace bytes flushed during panic */ +#define DMA_FLUSH_TRACE_SIZE (MAILBOX_TRACE_SIZE >> 2) + /* the interval of DMA trace copying */ #define DMA_TRACE_PERIOD 500000
diff --git a/src/platform/cannonlake/include/platform/platform.h b/src/platform/cannonlake/include/platform/platform.h index b007e8a..764ef76 100644 --- a/src/platform/cannonlake/include/platform/platform.h +++ b/src/platform/cannonlake/include/platform/platform.h @@ -92,6 +92,9 @@ struct reef; /* local buffer size of DMA tracing */ #define DMA_TRACE_LOCAL_SIZE HOST_PAGE_SIZE
+/* trace bytes flushed during panic */ +#define DMA_FLUSH_TRACE_SIZE (MAILBOX_TRACE_SIZE >> 2) + /* the interval of DMA trace copying */ #define DMA_TRACE_PERIOD 500000
diff --git a/src/platform/haswell/include/platform/platform.h b/src/platform/haswell/include/platform/platform.h index a9efe8a..9ffab90 100644 --- a/src/platform/haswell/include/platform/platform.h +++ b/src/platform/haswell/include/platform/platform.h @@ -81,6 +81,9 @@ struct reef; /* local buffer size of DMA tracing */ #define DMA_TRACE_LOCAL_SIZE HOST_PAGE_SIZE
+/* trace bytes flushed during panic */ +#define DMA_FLUSH_TRACE_SIZE (MAILBOX_TRACE_SIZE >> 2) + /* the interval of DMA trace copying */ #define DMA_TRACE_PERIOD 500000
On Sun, 2018-03-25 at 17:34 -0700, Ranjani Sridharan wrote:
If trace_data->dmatb is wrappered and buffer->w_ptr is at the start of DMA trace local buffer, It may cause wrong data sent? We may need check wrapper status of DMA local buffer.
participants (4)
-
Liam Girdwood -
Ranjani Sridharan -
Yan Wang -
yanwang