ipc: improve error reporting and return values

Improve IPC error reporting and values to host by tracing all failures
and making sure errors are returned when detected.

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>

src/ipc/byt-ipc.c

@@ -78,6 +78,7 @@ static void do_notify(void)
 
 out:
 	spin_unlock_irq(&_ipc->lock, flags);
+
 	/* clear DONE bit - tell Host we have completed */
 	shim_write(SHIM_IPCDH, shim_read(SHIM_IPCDH) & ~SHIM_IPCDH_DONE);
 
@@ -121,30 +122,48 @@ static void irq_handler(void *arg)
 void ipc_platform_do_cmd(struct ipc *ipc)
 {
 	struct intel_ipc_data *iipc = ipc_get_drvdata(ipc);
-	uint32_t ipcxh;//, status;
+	struct sof_ipc_reply reply;
+	uint32_t ipcxh;
+	int32_t err;
 
 	trace_ipc("Cmd");
-	//trace_value(_ipc->host_msg);
 
-	/* TODO: handle error with reply data in mailbox */
-	ipc_cmd();
+	/* clear old mailbox return values */
+	reply.hdr.cmd = SOF_IPC_GLB_REPLY;
+	reply.hdr.size = sizeof(reply);
+	reply.error = 0;
+	mailbox_outbox_write(0, &reply, sizeof(reply));
+
+	/* perform command and return any error */
+	err = ipc_cmd();
+	if (err < 0) {
+		/* read component values from the inbox */
+		reply.error = err;
+
+		/* write error back to outbox */
+		mailbox_outbox_write(0, &reply, sizeof(reply));
+	}
 	ipc->host_pending = 0;
-	trace_ipc("CmD");
+
 	/* clear BUSY bit and set DONE bit - accept new messages */
 	ipcxh = shim_read(SHIM_IPCXH);
 	ipcxh &= ~SHIM_IPCXH_BUSY;
-	ipcxh |= SHIM_IPCXH_DONE;// | status;
+	ipcxh |= SHIM_IPCXH_DONE;
 	shim_write(SHIM_IPCXH, ipcxh);
 
+	/* unmask busy interrupt */
+	shim_write(SHIM_IMRD, shim_read(SHIM_IMRD) & ~SHIM_IMRD_BUSY);
+
 	// TODO: signal audio work to enter D3 in normal context
 	/* are we about to enter D3 ? */
 	if (iipc->pm_prepare_D3) {
-		while (1)
+		while (1) {
+			trace_ipc("pme");
 			wait_for_interrupt(0);
+		}
 	}
 
-	/* unmask busy interrupt */
-	shim_write(SHIM_IMRD, shim_read(SHIM_IMRD) & ~SHIM_IMRD_BUSY);
+	trace_ipc("CmD");
 }
 
 void ipc_platform_send_msg(struct ipc *ipc)

src/ipc/intel-ipc.c

@@ -255,7 +255,9 @@ static int ipc_stream_pcm_params(uint32_t stream)
 	return 0;
 
 error:
-	pipeline_reset(pcm_dev->cd->pipeline, pcm_dev->cd);
+	err = pipeline_reset(pcm_dev->cd->pipeline, pcm_dev->cd);
+	if (err < 0)
+		trace_ipc_error("eA!");
 	return -EINVAL;
 }
 
@@ -269,11 +271,14 @@ static int ipc_stream_pcm_free(uint32_t header)
 
 	/* get the pcm_dev */
 	pcm_dev = ipc_get_comp(_ipc, free_req->comp_id);
-	if (pcm_dev == NULL)
-		return ENODEV;
+	if (pcm_dev == NULL) {
+		trace_ipc_error("eFr");
+		return -ENODEV;
+	}
 
 	/* reset the pipeline */
-	pipeline_reset(pcm_dev->cd->pipeline, pcm_dev->cd);
+	return pipeline_reset(pcm_dev->cd->pipeline, pcm_dev->cd);
+}
 
 /* get stream position */
 static int ipc_stream_position(uint32_t header)
@@ -326,7 +331,7 @@ static int ipc_stream_trigger(uint32_t header)
 	uint32_t cmd = COMP_CMD_RELEASE;
 	struct sof_ipc_stream *stream  = _ipc->comp_data;
 	uint32_t ipc_cmd = (header & SOF_CMD_TYPE_MASK) >> SOF_CMD_TYPE_SHIFT;
-	int err;
+	int ret;
 
 	trace_ipc("tri");
 
@@ -334,7 +339,7 @@ static int ipc_stream_trigger(uint32_t header)
 	pcm_dev = ipc_get_comp(_ipc, stream->comp_id);
 	if (pcm_dev == NULL) {
 		trace_ipc_error("eRg");
-		goto error;
+		return -ENODEV;
 	}
 
 	switch (ipc_cmd) {
@@ -359,15 +364,13 @@ static int ipc_stream_trigger(uint32_t header)
 	}
 
 	/* trigger the component */
-	err = pipeline_cmd(pcm_dev->cd->pipeline, pcm_dev->cd,
+	ret = pipeline_cmd(pcm_dev->cd->pipeline, pcm_dev->cd,
 			cmd, NULL);
-	if (err < 0) {
+	if (ret < 0) {
 		trace_ipc_error("eRc");
-		goto error;
 	}
 
-error:
-	return 0;
+	return ret;
 }
 
 static int ipc_glb_stream_message(uint32_t header)
@@ -532,7 +535,7 @@ static int ipc_glb_pm_message(uint32_t header)
 	case iCS(SOF_IPC_PM_CLK_GET):
 	case iCS(SOF_IPC_PM_CLK_REQ):
 	default:
-		return -ENOMEM;
+		return -EINVAL;
 	}
 }
 
@@ -545,12 +548,14 @@ static int ipc_comp_set_value(uint32_t header, uint32_t cmd)
 	struct ipc_comp_dev *stream_dev;
 	struct sof_ipc_ctrl_values *values = _ipc->comp_data;
 
-	//trace_ipc("VoS");
+	trace_ipc("VoS");
 
 	/* get the component */
 	stream_dev = ipc_get_comp(_ipc, values->comp_id);
-	if (stream_dev == NULL)
+	if (stream_dev == NULL) {
+		trace_ipc_error("eVs");
 		return -ENODEV;
+	}
 
 	/* set component values */
 	return comp_cmd(stream_dev->cd, cmd, values);
@@ -566,13 +571,17 @@ static int ipc_comp_get_value(uint32_t header, uint32_t cmd)
 
 	/* get the component */
 	stream_dev = ipc_get_comp(_ipc, values->comp_id);
-	if (stream_dev == NULL)
+	if (stream_dev == NULL){
+		trace_ipc_error("eVg");
 		return -ENODEV;
+	}
 	
 	/* get component values */
 	ret = comp_cmd(stream_dev->cd, COMP_CMD_VOLUME, values);
-	if (ret < 0)
+	if (ret < 0) {
+		trace_ipc_error("eVG");
 		return ret;
+	}
 
 	/* write component values to the outbox */
 	mailbox_outbox_write(values, 0, sizeof(*values));
@@ -618,8 +627,10 @@ static int ipc_glb_tplg_comp_new(uint32_t header)
 
 	/* register component */
 	ret = ipc_comp_new(_ipc, comp);
-	if (ret < 0)
+	if (ret < 0) {
+		trace_ipc_error("etc");
 		return ret;
+	}
 
 	/* write component values to the outbox */
 	mailbox_outbox_write(0, &reply, sizeof(reply));