From 9b5ecfebbb99dd0d98713f92b1e219a7bdcd6e0c Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 1 Mar 2019 16:27:55 +0100 Subject: [PATCH 1/9] ASoC: SOF: (cosmetic) reduce nesting depth Invert a condition and use an early return from a function to reduce its nesting depth. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/ipc.c | 49 ++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index bcd56f5a5fa907..c77358332cf8d6 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -293,33 +293,32 @@ static void ipc_tx_next_msg(struct work_struct *work) spin_lock_irq(&sdev->ipc_lock); /* send message if HW ready and message in TX list */ - if (!list_empty(&ipc->tx_list) && snd_sof_dsp_is_ipc_ready(sdev)) { - /* send first message in TX list */ - msg = list_first_entry(&ipc->tx_list, struct snd_sof_ipc_msg, - list); - ret = snd_sof_dsp_send_msg(sdev, msg); - if (ret < 0) { - - /* - * if ipc tx fails, the msg will be retained in the - * msg_list, so that it can be re-tried until it - * succeeds or times-out. The ipc re-try mechanism in - * SOF currently relies upon tx_kwork being - * rescheduled, which is not guaranteed. This needs - * to be enhanced with a better mechanism. - */ - dev_err_ratelimited(sdev->dev, - "error: ipc tx failed with error %d\n", - ret); - } else { - - /* message sent. move it to the reply list */ - list_move(&msg->list, &ipc->reply_list); - - ipc_log_header(sdev->dev, "ipc tx", msg->header); - } + if (list_empty(&ipc->tx_list) || !snd_sof_dsp_is_ipc_ready(sdev)) + goto unlock; + + /* send first message in TX list */ + msg = list_first_entry(&ipc->tx_list, struct snd_sof_ipc_msg, list); + + ret = snd_sof_dsp_send_msg(sdev, msg); + if (ret < 0) { + /* + * if ipc tx fails, the msg will be retained in the msg_list, so + * that it can be re-tried until it succeeds or times-out. The + * ipc re-try mechanism in SOF currently relies upon tx_kwork + * being rescheduled, which is not guaranteed. This needs to be + * enhanced with a better mechanism. + */ + dev_err_ratelimited(sdev->dev, + "error: ipc tx failed with error %d\n", + ret); + } else { + /* message sent. move it to the reply list */ + list_move(&msg->list, &ipc->reply_list); + + ipc_log_header(sdev->dev, "ipc tx", msg->header); } +unlock: spin_unlock_irq(&sdev->ipc_lock); } From 92ca10f1667142a6d1701bff5f586f8a9c6687c5 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Sun, 3 Mar 2019 17:05:10 +0100 Subject: [PATCH 2/9] ASoC: SOF: remove the RX work Using the RX work isn't required since also without it the serialisation is guaranteed by proocessing received messages directly in the IRQ thread. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/ipc.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index c77358332cf8d6..b7c90e4f808d97 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -37,9 +37,6 @@ struct snd_sof_ipc { struct work_struct tx_kwork; u32 msg_pending; - /* Rx Message work and status */ - struct work_struct rx_kwork; - /* lists */ struct list_head tx_list; struct list_head reply_list; @@ -391,11 +388,8 @@ int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id) EXPORT_SYMBOL(snd_sof_ipc_reply); /* DSP firmware has sent host a message */ -static void ipc_msgs_rx(struct work_struct *work) +void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev) { - struct snd_sof_ipc *ipc = - container_of(work, struct snd_sof_ipc, rx_kwork); - struct snd_sof_dev *sdev = ipc->sdev; struct sof_ipc_cmd_hdr hdr; u32 cmd, type; int err = 0; @@ -454,6 +448,7 @@ static void ipc_msgs_rx(struct work_struct *work) /* tell DSP we are done */ snd_sof_dsp_cmd_done(sdev, SOF_IPC_HOST_REPLY); } +EXPORT_SYMBOL(snd_sof_ipc_msgs_rx); /* schedule work to transmit any IPC in queue */ void snd_sof_ipc_msgs_tx(struct snd_sof_dev *sdev) @@ -463,14 +458,6 @@ void snd_sof_ipc_msgs_tx(struct snd_sof_dev *sdev) } EXPORT_SYMBOL(snd_sof_ipc_msgs_tx); -/* schedule work to handle IPC from DSP */ -void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev) -{ - if (!sdev->disable_ipc_queue) - schedule_work(&sdev->ipc->rx_kwork); -} -EXPORT_SYMBOL(snd_sof_ipc_msgs_rx); - /* * IPC trace mechanism. */ @@ -799,7 +786,6 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev) INIT_LIST_HEAD(&ipc->empty_list); init_waitqueue_head(&ipc->wait_txq); INIT_WORK(&ipc->tx_kwork, ipc_tx_next_msg); - INIT_WORK(&ipc->rx_kwork, ipc_msgs_rx); ipc->sdev = sdev; /* pre-allocate messages */ @@ -836,6 +822,5 @@ void snd_sof_ipc_free(struct snd_sof_dev *sdev) sdev->disable_ipc_queue = true; cancel_work_sync(&sdev->ipc->tx_kwork); - cancel_work_sync(&sdev->ipc->rx_kwork); } EXPORT_SYMBOL(snd_sof_ipc_free); From 6f7c3fea267cb738164e825d35b4091773e36764 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Sun, 3 Mar 2019 17:10:00 +0100 Subject: [PATCH 3/9] ASoC: SOF: removed the unused .wait_txq field The .wait_txq wait queue head is initialised but never used. Remove it. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/ipc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index b7c90e4f808d97..0dfa4d8a48dac4 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -33,7 +33,6 @@ struct snd_sof_ipc { struct snd_sof_dev *sdev; /* TX message work and status */ - wait_queue_head_t wait_txq; struct work_struct tx_kwork; u32 msg_pending; @@ -784,7 +783,6 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev) INIT_LIST_HEAD(&ipc->tx_list); INIT_LIST_HEAD(&ipc->reply_list); INIT_LIST_HEAD(&ipc->empty_list); - init_waitqueue_head(&ipc->wait_txq); INIT_WORK(&ipc->tx_kwork, ipc_tx_next_msg); ipc->sdev = sdev; From 5f811e1f5a54604bc0856d9f68825a2f9005152f Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Mon, 4 Mar 2019 11:00:20 +0100 Subject: [PATCH 4/9] ASoC: SOF: use a mutex for IPC TX serialisation IPC TX is currently fully synchronous, the only entry point to send an IPC message is sof_ipc_tx_message() and it unconditionally waits for a reply to the message being sent. Enclosing the complete function into a mutex lock-unlock block ensures, that IPC registers and structs aren't accessed from multiple contexts. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/ipc.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 0dfa4d8a48dac4..c0a9b901244d04 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -11,6 +11,8 @@ // by platform driver code. // +#include + #include "sof-priv.h" #include "ops.h" @@ -32,6 +34,9 @@ static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_cmd); struct snd_sof_ipc { struct snd_sof_dev *sdev; + /* protects messages and the disable flag */ + struct mutex tx_mutex; + /* TX message work and status */ struct work_struct tx_kwork; u32 msg_pending; @@ -244,14 +249,23 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, { struct snd_sof_dev *sdev = ipc->sdev; struct snd_sof_ipc_msg *msg; + int ret; + /* Serialise IPC TX */ + mutex_lock(&ipc->tx_mutex); + + /* + * The spin-lock is also still needed to protect message objects against + * other atomic contexts. + */ spin_lock_irq(&sdev->ipc_lock); /* get an empty message */ msg = msg_get_empty(ipc); if (!msg) { spin_unlock_irq(&sdev->ipc_lock); - return -EBUSY; + ret = -EBUSY; + goto unlock; } msg->header = header; @@ -273,7 +287,12 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, snd_sof_ipc_msgs_tx(sdev); /* now wait for completion */ - return tx_wait_done(ipc, msg, reply_data); + ret = tx_wait_done(ipc, msg, reply_data); + +unlock: + mutex_unlock(&ipc->tx_mutex); + + return ret; } EXPORT_SYMBOL(sof_ipc_tx_message); @@ -784,6 +803,7 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev) INIT_LIST_HEAD(&ipc->reply_list); INIT_LIST_HEAD(&ipc->empty_list); INIT_WORK(&ipc->tx_kwork, ipc_tx_next_msg); + mutex_init(&ipc->tx_mutex); ipc->sdev = sdev; /* pre-allocate messages */ From fd3c962f23a92fa13eabd02283edddea8d650ac1 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 8 Mar 2019 10:49:47 +0100 Subject: [PATCH 5/9] ASoC: SOF: fix a theoretical race If snd_sof_ipc_reply() is called from an IPC ISR for a DSP response, i.e. after snd_sof_dsp_send_msg() has sent a message out, but before the message is moved to the .reply_list in ipc_tx_next_msg(), it won't find the message to which the received response is related. This race is almost certainly only theoretical, since ipc_tx_next_msg() is holding a spin-lock and has local interrupts disabled while sending the message out. Still, it has to be closed. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/ipc.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index c0a9b901244d04..854cd99a219d07 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -391,9 +391,20 @@ EXPORT_SYMBOL(sof_ipc_drop_all); int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id) { struct snd_sof_ipc_msg *msg; + unsigned long flags; + + /* + * Protect against a theoretical race with ipc_tx_next_msg(): if the DSP + * is fast enough to receive an IPC message, reply to it, and the host + * interrupt processing calls this function on a different core from the + * one, where the sending is taking place, the message might not yet be + * on the .reply_list and sof_ipc_reply_find_msg() will return 0. + */ + spin_lock_irqsave(&sdev->ipc_lock, flags); msg = sof_ipc_reply_find_msg(sdev->ipc, msg_id); if (!msg) { + spin_unlock_irqrestore(&sdev->ipc_lock, flags); dev_err(sdev->dev, "error: can't find message header 0x%x", msg_id); return -EINVAL; @@ -401,6 +412,9 @@ int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id) /* wake up and return the error if we have waiters on this message ? */ sof_ipc_tx_msg_reply_complete(sdev->ipc, msg); + + spin_unlock_irqrestore(&sdev->ipc_lock, flags); + return 0; } EXPORT_SYMBOL(snd_sof_ipc_reply); From 0f9ceb97d2302884431a0325b998ed852dea8f67 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 7 Mar 2019 13:02:33 +0100 Subject: [PATCH 6/9] ASoC: SOF: remove IPC TX scheduling An IPC TX work was used to serialise IPC message transmission. Now that a mutex is used for this, scheduling isn't needed any more. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/ipc.c | 113 +++++++++++++++++---------------------- sound/soc/sof/sof-priv.h | 2 - 2 files changed, 49 insertions(+), 66 deletions(-) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 854cd99a219d07..273b700934b9ce 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -12,6 +12,7 @@ // #include +#include #include "sof-priv.h" #include "ops.h" @@ -36,10 +37,8 @@ struct snd_sof_ipc { /* protects messages and the disable flag */ struct mutex tx_mutex; - - /* TX message work and status */ - struct work_struct tx_kwork; - u32 msg_pending; + /* disables further sending of ipc's */ + bool disable_ipc_tx; /* lists */ struct list_head tx_list; @@ -236,8 +235,34 @@ static int tx_wait_done(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg, snd_sof_dsp_cmd_done(sdev, SOF_IPC_DSP_REPLY); - /* continue to schedule any remaining messages... */ - snd_sof_ipc_msgs_tx(sdev); + return ret; +} + +/* send next IPC message in list */ +static int ipc_tx_next_msg(struct snd_sof_ipc *ipc) +{ + struct snd_sof_dev *sdev = ipc->sdev; + struct snd_sof_ipc_msg *msg; + int ret; + + /* send first message in TX list */ + msg = list_first_entry(&ipc->tx_list, struct snd_sof_ipc_msg, list); + + ret = snd_sof_dsp_send_msg(sdev, msg); + if (ret < 0) { + /* + * if ipc tx fails, the msg will be retained in the msg_list, so + * that it can be re-tried until it succeeds or times-out. + */ + dev_err_ratelimited(sdev->dev, + "error: ipc tx failed with error %d\n", + ret); + } else { + /* message sent. move it to the reply list */ + list_move(&msg->list, &ipc->reply_list); + + ipc_log_header(sdev->dev, "ipc tx", msg->header); + } return ret; } @@ -254,6 +279,11 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, /* Serialise IPC TX */ mutex_lock(&ipc->tx_mutex); + if (ipc->disable_ipc_tx) { + ret = -ENODEV; + goto unlock; + } + /* * The spin-lock is also still needed to protect message objects against * other atomic contexts. @@ -280,14 +310,17 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, /* add message to transmit list */ list_add_tail(&msg->list, &ipc->tx_list); - spin_unlock_irq(&sdev->ipc_lock); - - /* schedule the message if not busy */ + /* send the message if not busy */ if (snd_sof_dsp_is_ipc_ready(sdev)) - snd_sof_ipc_msgs_tx(sdev); + ret = ipc_tx_next_msg(sdev->ipc); + else + ret = -EAGAIN; + + spin_unlock_irq(&sdev->ipc_lock); /* now wait for completion */ - ret = tx_wait_done(ipc, msg, reply_data); + if (!ret) + ret = tx_wait_done(ipc, msg, reply_data); unlock: mutex_unlock(&ipc->tx_mutex); @@ -296,47 +329,6 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, } EXPORT_SYMBOL(sof_ipc_tx_message); -/* send next IPC message in list */ -static void ipc_tx_next_msg(struct work_struct *work) -{ - struct snd_sof_ipc *ipc = - container_of(work, struct snd_sof_ipc, tx_kwork); - struct snd_sof_dev *sdev = ipc->sdev; - struct snd_sof_ipc_msg *msg; - int ret; - - spin_lock_irq(&sdev->ipc_lock); - - /* send message if HW ready and message in TX list */ - if (list_empty(&ipc->tx_list) || !snd_sof_dsp_is_ipc_ready(sdev)) - goto unlock; - - /* send first message in TX list */ - msg = list_first_entry(&ipc->tx_list, struct snd_sof_ipc_msg, list); - - ret = snd_sof_dsp_send_msg(sdev, msg); - if (ret < 0) { - /* - * if ipc tx fails, the msg will be retained in the msg_list, so - * that it can be re-tried until it succeeds or times-out. The - * ipc re-try mechanism in SOF currently relies upon tx_kwork - * being rescheduled, which is not guaranteed. This needs to be - * enhanced with a better mechanism. - */ - dev_err_ratelimited(sdev->dev, - "error: ipc tx failed with error %d\n", - ret); - } else { - /* message sent. move it to the reply list */ - list_move(&msg->list, &ipc->reply_list); - - ipc_log_header(sdev->dev, "ipc tx", msg->header); - } - -unlock: - spin_unlock_irq(&sdev->ipc_lock); -} - /* find original TX message from DSP reply */ static struct snd_sof_ipc_msg *sof_ipc_reply_find_msg(struct snd_sof_ipc *ipc, u32 header) @@ -482,14 +474,6 @@ void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev) } EXPORT_SYMBOL(snd_sof_ipc_msgs_rx); -/* schedule work to transmit any IPC in queue */ -void snd_sof_ipc_msgs_tx(struct snd_sof_dev *sdev) -{ - if (!sdev->disable_ipc_queue) - schedule_work(&sdev->ipc->tx_kwork); -} -EXPORT_SYMBOL(snd_sof_ipc_msgs_tx); - /* * IPC trace mechanism. */ @@ -816,7 +800,6 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev) INIT_LIST_HEAD(&ipc->tx_list); INIT_LIST_HEAD(&ipc->reply_list); INIT_LIST_HEAD(&ipc->empty_list); - INIT_WORK(&ipc->tx_kwork, ipc_tx_next_msg); mutex_init(&ipc->tx_mutex); ipc->sdev = sdev; @@ -850,9 +833,11 @@ EXPORT_SYMBOL(snd_sof_ipc_init); void snd_sof_ipc_free(struct snd_sof_dev *sdev) { - /* disable queueing of ipc's */ - sdev->disable_ipc_queue = true; + struct snd_sof_ipc *ipc = sdev->ipc; - cancel_work_sync(&sdev->ipc->tx_kwork); + /* disable sending of ipc's */ + mutex_lock(&ipc->tx_mutex); + ipc->disable_ipc_tx = true; + mutex_unlock(&ipc->tx_mutex); } EXPORT_SYMBOL(snd_sof_ipc_free); diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 7af3f4db8a2510..09134fa1b2b71b 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -363,7 +363,6 @@ struct snd_sof_dev { u64 irq_status; int ipc_irq; u32 next_comp_id; /* monotonic - reset during S3 */ - u32 disable_ipc_queue; /* disables further queueing of ipc's */ /* memory bases for mmaped DSPs - set by dsp_init() */ void __iomem *bar[SND_SOF_BARS]; /* DSP base address */ @@ -453,7 +452,6 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev); void snd_sof_ipc_free(struct snd_sof_dev *sdev); int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id); void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev); -void snd_sof_ipc_msgs_tx(struct snd_sof_dev *sdev); int snd_sof_ipc_stream_pcm_params(struct snd_sof_dev *sdev, struct sof_ipc_pcm_params *params); int snd_sof_dsp_mailbox_init(struct snd_sof_dev *sdev, u32 dspbox, From 6ad56610dab1fe05d3be8464254514b8f238cb1b Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 8 Mar 2019 13:41:31 +0100 Subject: [PATCH 7/9] ASoC: SOF: remove IPC message lists IPC message lists are currently unused. Only one message is ever used. Remove lists and use just one message. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/ipc.c | 163 +++++++-------------------------------- sound/soc/sof/pm.c | 3 - sound/soc/sof/sof-priv.h | 5 +- 3 files changed, 31 insertions(+), 140 deletions(-) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 273b700934b9ce..ae3be4124cb862 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -22,7 +22,6 @@ * TODO: allow platforms to set size and timeout. */ #define IPC_TIMEOUT_MS 300 -#define IPC_EMPTY_LIST_SIZE 8 static void ipc_trace_message(struct snd_sof_dev *sdev, u32 msg_id); static void ipc_stream_message(struct snd_sof_dev *sdev, u32 msg_cmd); @@ -40,27 +39,9 @@ struct snd_sof_ipc { /* disables further sending of ipc's */ bool disable_ipc_tx; - /* lists */ - struct list_head tx_list; - struct list_head reply_list; - struct list_head empty_list; + struct snd_sof_ipc_msg msg; }; -/* locks held by caller */ -static struct snd_sof_ipc_msg *msg_get_empty(struct snd_sof_ipc *ipc) -{ - struct snd_sof_ipc_msg *msg = NULL; - - /* get first empty message in the list */ - if (!list_empty(&ipc->empty_list)) { - msg = list_first_entry(&ipc->empty_list, struct snd_sof_ipc_msg, - list); - list_del(&msg->list); - } - - return msg; -} - #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_VERBOSE_IPC) static void ipc_log_header(struct device *dev, u8 *text, u32 cmd) { @@ -203,13 +184,6 @@ static int tx_wait_done(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg, ret = wait_event_timeout(msg->waitq, msg->ipc_complete, msecs_to_jiffies(IPC_TIMEOUT_MS)); - /* - * ipc_lock is used to protect ipc message list shared by user - * contexts and a workqueue. There is no need to save interrupt - * status with spin_lock_irqsave. - */ - spin_lock_irq(&sdev->ipc_lock); - if (ret == 0) { dev_err(sdev->dev, "error: ipc timed out for 0x%x size 0x%x\n", hdr->cmd, hdr->size); @@ -228,11 +202,6 @@ static int tx_wait_done(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg, ipc_log_header(sdev->dev, "ipc tx succeeded", hdr->cmd); } - /* return message body to empty list */ - list_move(&msg->list, &ipc->empty_list); - - spin_unlock_irq(&sdev->ipc_lock); - snd_sof_dsp_cmd_done(sdev, SOF_IPC_DSP_REPLY); return ret; @@ -242,27 +211,16 @@ static int tx_wait_done(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg, static int ipc_tx_next_msg(struct snd_sof_ipc *ipc) { struct snd_sof_dev *sdev = ipc->sdev; - struct snd_sof_ipc_msg *msg; - int ret; - - /* send first message in TX list */ - msg = list_first_entry(&ipc->tx_list, struct snd_sof_ipc_msg, list); + struct snd_sof_ipc_msg *msg = &ipc->msg; + int ret = snd_sof_dsp_send_msg(sdev, msg); - ret = snd_sof_dsp_send_msg(sdev, msg); - if (ret < 0) { - /* - * if ipc tx fails, the msg will be retained in the msg_list, so - * that it can be re-tried until it succeeds or times-out. - */ + if (ret < 0) + /* So far IPC TX never fails, consider making the above void */ dev_err_ratelimited(sdev->dev, "error: ipc tx failed with error %d\n", ret); - } else { - /* message sent. move it to the reply list */ - list_move(&msg->list, &ipc->reply_list); - + else ipc_log_header(sdev->dev, "ipc tx", msg->header); - } return ret; } @@ -290,31 +248,26 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, */ spin_lock_irq(&sdev->ipc_lock); - /* get an empty message */ - msg = msg_get_empty(ipc); - if (!msg) { - spin_unlock_irq(&sdev->ipc_lock); - ret = -EBUSY; - goto unlock; - } + /* initialise the message */ + msg = &ipc->msg; msg->header = header; msg->msg_size = msg_bytes; msg->reply_size = reply_bytes; - msg->ipc_complete = false; /* attach any data */ if (msg_bytes) memcpy(msg->msg_data, msg_data, msg_bytes); - /* add message to transmit list */ - list_add_tail(&msg->list, &ipc->tx_list); - /* send the message if not busy */ - if (snd_sof_dsp_is_ipc_ready(sdev)) + if (snd_sof_dsp_is_ipc_ready(sdev)) { ret = ipc_tx_next_msg(sdev->ipc); - else + /* Next reply that we receive will be related to this message */ + if (!ret) + msg->ipc_complete = false; + } else { ret = -EAGAIN; + } spin_unlock_irq(&sdev->ipc_lock); @@ -329,25 +282,6 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, } EXPORT_SYMBOL(sof_ipc_tx_message); -/* find original TX message from DSP reply */ -static struct snd_sof_ipc_msg *sof_ipc_reply_find_msg(struct snd_sof_ipc *ipc, - u32 header) -{ - struct snd_sof_dev *sdev = ipc->sdev; - struct snd_sof_ipc_msg *msg; - - header = SOF_IPC_MESSAGE_ID(header); - - list_for_each_entry(msg, &ipc->reply_list, list) { - if (SOF_IPC_MESSAGE_ID(msg->header) == header) - return msg; - } - - dev_err(sdev->dev, "error: rx list empty but received 0x%x\n", - header); - return NULL; -} - /* mark IPC message as complete - locks held by caller */ static void sof_ipc_tx_msg_reply_complete(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg) @@ -356,33 +290,10 @@ static void sof_ipc_tx_msg_reply_complete(struct snd_sof_ipc *ipc, wake_up(&msg->waitq); } -/* drop all IPC messages in preparation for DSP stall/reset */ -void sof_ipc_drop_all(struct snd_sof_ipc *ipc) -{ - struct snd_sof_dev *sdev = ipc->sdev; - struct snd_sof_ipc_msg *msg, *tmp; - - /* drop all TX and Rx messages before we stall + reset DSP */ - spin_lock_irq(&sdev->ipc_lock); - - list_for_each_entry_safe(msg, tmp, &ipc->tx_list, list) { - list_move(&msg->list, &ipc->empty_list); - dev_err(sdev->dev, "error: dropped msg %d\n", msg->header); - } - - list_for_each_entry_safe(msg, tmp, &ipc->reply_list, list) { - list_move(&msg->list, &ipc->empty_list); - dev_err(sdev->dev, "error: dropped reply %d\n", msg->header); - } - - spin_unlock_irq(&sdev->ipc_lock); -} -EXPORT_SYMBOL(sof_ipc_drop_all); - /* handle reply message from DSP */ int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id) { - struct snd_sof_ipc_msg *msg; + struct snd_sof_ipc_msg *msg = &sdev->ipc->msg; unsigned long flags; /* @@ -390,14 +301,13 @@ int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id) * is fast enough to receive an IPC message, reply to it, and the host * interrupt processing calls this function on a different core from the * one, where the sending is taking place, the message might not yet be - * on the .reply_list and sof_ipc_reply_find_msg() will return 0. + * marked as expecting a reply. */ spin_lock_irqsave(&sdev->ipc_lock, flags); - msg = sof_ipc_reply_find_msg(sdev->ipc, msg_id); - if (!msg) { + if (msg->ipc_complete) { spin_unlock_irqrestore(&sdev->ipc_lock, flags); - dev_err(sdev->dev, "error: can't find message header 0x%x", + dev_err(sdev->dev, "error: no reply expected, received 0x%x", msg_id); return -EINVAL; } @@ -785,7 +695,6 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev) { struct snd_sof_ipc *ipc; struct snd_sof_ipc_msg *msg; - int i; /* check if mandatory ops required for ipc are defined */ if (!sof_ops(sdev)->is_ipc_ready || !sof_ops(sdev)->fw_ready) { @@ -797,35 +706,23 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev) if (!ipc) return NULL; - INIT_LIST_HEAD(&ipc->tx_list); - INIT_LIST_HEAD(&ipc->reply_list); - INIT_LIST_HEAD(&ipc->empty_list); mutex_init(&ipc->tx_mutex); ipc->sdev = sdev; + msg = &ipc->msg; - /* pre-allocate messages */ - dev_dbg(sdev->dev, "pre-allocate %d IPC messages\n", - IPC_EMPTY_LIST_SIZE); - msg = devm_kzalloc(sdev->dev, sizeof(struct snd_sof_ipc_msg) * - IPC_EMPTY_LIST_SIZE, GFP_KERNEL); - if (!msg) - return NULL; + /* Indicate, that we aren't sending a message ATM */ + msg->ipc_complete = true; /* pre-allocate message data */ - for (i = 0; i < IPC_EMPTY_LIST_SIZE; i++) { - msg->msg_data = devm_kzalloc(sdev->dev, PAGE_SIZE, GFP_KERNEL); - if (!msg->msg_data) - return NULL; - - msg->reply_data = devm_kzalloc(sdev->dev, PAGE_SIZE, - GFP_KERNEL); - if (!msg->reply_data) - return NULL; - - init_waitqueue_head(&msg->waitq); - list_add(&msg->list, &ipc->empty_list); - msg++; - } + msg->msg_data = devm_kzalloc(sdev->dev, PAGE_SIZE, GFP_KERNEL); + if (!msg->msg_data) + return NULL; + + msg->reply_data = devm_kzalloc(sdev->dev, PAGE_SIZE, GFP_KERNEL); + if (!msg->reply_data) + return NULL; + + init_waitqueue_head(&msg->waitq); return ipc; } diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 15daad65f18064..75a4e20c16d8a4 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -342,9 +342,6 @@ static int sof_suspend(struct device *dev, bool runtime_suspend) ret); } - /* drop all ipc */ - sof_ipc_drop_all(sdev->ipc); - /* power down all DSP cores */ if (runtime_suspend) ret = snd_sof_dsp_runtime_suspend(sdev, 0); diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 09134fa1b2b71b..5497534ae0136f 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -250,8 +250,6 @@ struct snd_sof_mailbox { /* IPC message descriptor for host <-> DSP IO */ struct snd_sof_ipc_msg { - struct list_head list; - /* message data */ u32 header; void *msg_data; @@ -260,7 +258,7 @@ struct snd_sof_ipc_msg { size_t reply_size; wait_queue_head_t waitq; - u32 ipc_complete; + bool ipc_complete; }; /* PCM stream, mapped to FW component */ @@ -489,7 +487,6 @@ struct snd_sof_pcm *snd_sof_find_spcm_comp(struct snd_sof_dev *sdev, int *direction); struct snd_sof_pcm *snd_sof_find_spcm_pcm_id(struct snd_sof_dev *sdev, unsigned int pcm_id); -void sof_ipc_drop_all(struct snd_sof_ipc *ipc); /* * Stream IPC From b8bf22bbed2da8faf594682c520ef953a879297e Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 8 Mar 2019 15:24:10 +0100 Subject: [PATCH 8/9] ASoC: SOF: protect against buffer overflow The sof_ipc_tx_message() function is called from multiple locations in SOF to transmit IPC messages. It copies data from the supplied location to an internal fixed-size buffer without verifying the size and prepares a response buffer for data from the DSP, if any, also without checking the size of requested response data. This can lead to buffer overflows. Add a check to avoid it. When allocating those buffers only allocate the maximum IPC message size. Also remove type-casting of those buffers, since they are of the "void *" type. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/intel/hda-ipc.c | 2 +- sound/soc/sof/ipc.c | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c index 589264524f08c5..766281f3a55c6e 100644 --- a/sound/soc/sof/intel/hda-ipc.c +++ b/sound/soc/sof/intel/hda-ipc.c @@ -90,7 +90,7 @@ int hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev, int ret = 0; u32 size; - hdr = (struct sof_ipc_cmd_hdr *)msg->msg_data; + hdr = msg->msg_data; if (hdr->cmd == (SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_CTX_SAVE)) { /* * memory windows are powered off before sending IPC reply, diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index ae3be4124cb862..04d64ef8a3ec28 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -177,7 +177,7 @@ static int tx_wait_done(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg, void *reply_data) { struct snd_sof_dev *sdev = ipc->sdev; - struct sof_ipc_cmd_hdr *hdr = (struct sof_ipc_cmd_hdr *)msg->msg_data; + struct sof_ipc_cmd_hdr *hdr = msg->msg_data; int ret; /* wait for DSP IPC completion */ @@ -234,6 +234,10 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, struct snd_sof_ipc_msg *msg; int ret; + if (msg_bytes > SOF_IPC_MSG_MAX_SIZE || + reply_bytes > SOF_IPC_MSG_MAX_SIZE) + return -ENOBUFS; + /* Serialise IPC TX */ mutex_lock(&ipc->tx_mutex); @@ -714,11 +718,13 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev) msg->ipc_complete = true; /* pre-allocate message data */ - msg->msg_data = devm_kzalloc(sdev->dev, PAGE_SIZE, GFP_KERNEL); + msg->msg_data = devm_kzalloc(sdev->dev, SOF_IPC_MSG_MAX_SIZE, + GFP_KERNEL); if (!msg->msg_data) return NULL; - msg->reply_data = devm_kzalloc(sdev->dev, PAGE_SIZE, GFP_KERNEL); + msg->reply_data = devm_kzalloc(sdev->dev, SOF_IPC_MSG_MAX_SIZE, + GFP_KERNEL); if (!msg->reply_data) return NULL; From 20d02b3f53825c94b6ca850326e7313242655963 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 7 Mar 2019 11:45:54 +0100 Subject: [PATCH 9/9] ASoC: SOF: remove the "is_ipc_ready" API .is_ipc_ready() is a platform-provided method, that checks whether an IPC transmission operation is currently in progress. It is used by sof_ipc_tx_message() itself to avoid races. However, now the complete sof_ipc_tx_message() function is protected by a mutex, therefore it cannot be re-entered. This allows us to remove .is_ipc_ready() by implicitly replacing its return code with a "true" value. Signed-off-by: Guennadi Liakhovetski --- sound/soc/sof/hw-spi.c | 9 ------ sound/soc/sof/intel/apl.c | 1 - sound/soc/sof/intel/bdw.c | 12 -------- sound/soc/sof/intel/byt.c | 14 --------- sound/soc/sof/intel/cnl.c | 14 --------- sound/soc/sof/intel/hda-ipc.c | 14 --------- sound/soc/sof/intel/hda.h | 1 - sound/soc/sof/intel/hsw.c | 12 -------- sound/soc/sof/intel/skl.c | 1 - sound/soc/sof/ipc.c | 53 +++++++++++++---------------------- sound/soc/sof/ops.h | 9 ------ sound/soc/sof/sof-priv.h | 1 - 12 files changed, 20 insertions(+), 121 deletions(-) diff --git a/sound/soc/sof/hw-spi.c b/sound/soc/sof/hw-spi.c index 426f276d2c232c..58815c65c9c8e3 100644 --- a/sound/soc/sof/hw-spi.c +++ b/sound/soc/sof/hw-spi.c @@ -181,14 +181,6 @@ static irqreturn_t spi_irq_thread(int irq __maybe_unused, void *context __maybe_ return IRQ_HANDLED; } -static int spi_is_ipc_ready(struct snd_sof_dev *sdev __maybe_unused) -{ - // use local variable to store DSP command state. either DSP is ready - // for new cmd or still processing current cmd. - - return 1; -} - static int spi_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { /* send the message */ @@ -300,7 +292,6 @@ const struct snd_sof_dsp_ops snd_sof_spi_ops = { .send_msg = spi_send_msg, .get_reply = spi_get_reply, .fw_ready = spi_fw_ready, - .is_ipc_ready = spi_is_ipc_ready, .cmd_done = spi_cmd_done, /* debug */ diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c index 3baedbb48d10e1..5b53b149c26301 100644 --- a/sound/soc/sof/intel/apl.c +++ b/sound/soc/sof/intel/apl.c @@ -52,7 +52,6 @@ const struct snd_sof_dsp_ops sof_apl_ops = { .send_msg = hda_dsp_ipc_send_msg, .get_reply = hda_dsp_ipc_get_reply, .fw_ready = hda_dsp_ipc_fw_ready, - .is_ipc_ready = hda_dsp_is_ipc_ready, .cmd_done = hda_dsp_ipc_cmd_done, /* debug */ diff --git a/sound/soc/sof/intel/bdw.c b/sound/soc/sof/intel/bdw.c index 1340ccf6fba845..cc7fc1d4c513ba 100644 --- a/sound/soc/sof/intel/bdw.c +++ b/sound/soc/sof/intel/bdw.c @@ -465,17 +465,6 @@ static int bdw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) * IPC Mailbox IO */ -static int bdw_is_ipc_ready(struct snd_sof_dev *sdev) -{ - u32 val; - - val = snd_sof_dsp_read(sdev, BDW_DSP_BAR, SHIM_IPCX); - if ((val & SHIM_IPCX_BUSY) || (val & SHIM_IPCX_DONE)) - return 0; - - return 1; -} - static int bdw_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { /* send the message */ @@ -677,7 +666,6 @@ const struct snd_sof_dsp_ops sof_bdw_ops = { .send_msg = bdw_send_msg, .get_reply = bdw_get_reply, .fw_ready = bdw_fw_ready, - .is_ipc_ready = bdw_is_ipc_ready, .cmd_done = bdw_cmd_done, /* debug */ diff --git a/sound/soc/sof/intel/byt.c b/sound/soc/sof/intel/byt.c index 74a618c095df56..72eb2a886a2990 100644 --- a/sound/soc/sof/intel/byt.c +++ b/sound/soc/sof/intel/byt.c @@ -360,17 +360,6 @@ static irqreturn_t byt_irq_thread(int irq, void *context) return IRQ_HANDLED; } -static int byt_is_ipc_ready(struct snd_sof_dev *sdev) -{ - u64 ipcx; - - ipcx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCX); - if ((ipcx & SHIM_BYT_IPCX_BUSY) || (ipcx & SHIM_BYT_IPCX_DONE)) - return 0; - - return 1; -} - static int byt_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { u64 cmd = msg->header; @@ -616,7 +605,6 @@ const struct snd_sof_dsp_ops sof_tng_ops = { .send_msg = byt_send_msg, .get_reply = byt_get_reply, .fw_ready = byt_fw_ready, - .is_ipc_ready = byt_is_ipc_ready, .cmd_done = byt_cmd_done, /* debug */ @@ -778,7 +766,6 @@ const struct snd_sof_dsp_ops sof_byt_ops = { .send_msg = byt_send_msg, .get_reply = byt_get_reply, .fw_ready = byt_fw_ready, - .is_ipc_ready = byt_is_ipc_ready, .cmd_done = byt_cmd_done, /* debug */ @@ -835,7 +822,6 @@ const struct snd_sof_dsp_ops sof_cht_ops = { .send_msg = byt_send_msg, .get_reply = byt_get_reply, .fw_ready = byt_fw_ready, - .is_ipc_ready = byt_is_ipc_ready, .cmd_done = byt_cmd_done, /* debug */ diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index 3db59228db6e1a..3fb17f42e22bb7 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -153,19 +153,6 @@ static int cnl_ipc_cmd_done(struct snd_sof_dev *sdev, int dir) return 0; } -static int cnl_is_ipc_ready(struct snd_sof_dev *sdev) -{ - u32 busy, done; - - busy = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR); - done = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDA); - if ((busy & CNL_DSP_REG_HIPCIDR_BUSY) || - (done & CNL_DSP_REG_HIPCIDA_DONE)) - return 0; - - return 1; -} - static int cnl_ipc_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { @@ -208,7 +195,6 @@ const struct snd_sof_dsp_ops sof_cnl_ops = { .send_msg = cnl_ipc_send_msg, .get_reply = hda_dsp_ipc_get_reply, .fw_ready = hda_dsp_ipc_fw_ready, - .is_ipc_ready = cnl_is_ipc_ready, .cmd_done = cnl_ipc_cmd_done, /* debug */ diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c index 766281f3a55c6e..4d978666db5ac6 100644 --- a/sound/soc/sof/intel/hda-ipc.c +++ b/sound/soc/sof/intel/hda-ipc.c @@ -55,20 +55,6 @@ int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev, int dir) return 0; } -int hda_dsp_is_ipc_ready(struct snd_sof_dev *sdev) -{ - u32 busy, done; - - /* is DSP ready for next IPC command */ - busy = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCI); - done = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCIE); - if ((busy & HDA_DSP_REG_HIPCI_BUSY) || - (done & HDA_DSP_REG_HIPCIE_DONE)) - return 0; - - return 1; -} - int hda_dsp_ipc_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { u32 cmd = msg->header; diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index baf1785442bbd2..6e1caf8331ae45 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -465,7 +465,6 @@ int hda_dsp_stream_spib_config(struct snd_sof_dev *sdev, /* * DSP IPC Operations. */ -int hda_dsp_is_ipc_ready(struct snd_sof_dev *sdev); int hda_dsp_ipc_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg); int hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev, diff --git a/sound/soc/sof/intel/hsw.c b/sound/soc/sof/intel/hsw.c index 09c9ab8e0f67bd..b7193d21cebb48 100644 --- a/sound/soc/sof/intel/hsw.c +++ b/sound/soc/sof/intel/hsw.c @@ -466,17 +466,6 @@ static int hsw_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) * IPC Mailbox IO */ -static int hsw_is_ipc_ready(struct snd_sof_dev *sdev) -{ - u32 val; - - val = snd_sof_dsp_read(sdev, HSW_DSP_BAR, SHIM_IPCX); - if ((val & SHIM_IPCX_BUSY) || (val & SHIM_IPCX_DONE)) - return 0; - - return 1; -} - static int hsw_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { /* send the message */ @@ -677,7 +666,6 @@ const struct snd_sof_dsp_ops sof_hsw_ops = { .send_msg = hsw_send_msg, .get_reply = hsw_get_reply, .fw_ready = hsw_fw_ready, - .is_ipc_ready = hsw_is_ipc_ready, .cmd_done = hsw_cmd_done, /* debug */ diff --git a/sound/soc/sof/intel/skl.c b/sound/soc/sof/intel/skl.c index 69816deb3c358c..1918297bae420b 100644 --- a/sound/soc/sof/intel/skl.c +++ b/sound/soc/sof/intel/skl.c @@ -52,7 +52,6 @@ const struct snd_sof_dsp_ops sof_skl_ops = { .send_msg = hda_dsp_ipc_send_msg, .get_reply = hda_dsp_ipc_get_reply, .fw_ready = hda_dsp_ipc_fw_ready, - .is_ipc_ready = hda_dsp_is_ipc_ready, .cmd_done = hda_dsp_ipc_cmd_done, /* debug */ diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 04d64ef8a3ec28..e27cdd8702f93e 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -207,24 +207,6 @@ static int tx_wait_done(struct snd_sof_ipc *ipc, struct snd_sof_ipc_msg *msg, return ret; } -/* send next IPC message in list */ -static int ipc_tx_next_msg(struct snd_sof_ipc *ipc) -{ - struct snd_sof_dev *sdev = ipc->sdev; - struct snd_sof_ipc_msg *msg = &ipc->msg; - int ret = snd_sof_dsp_send_msg(sdev, msg); - - if (ret < 0) - /* So far IPC TX never fails, consider making the above void */ - dev_err_ratelimited(sdev->dev, - "error: ipc tx failed with error %d\n", - ret); - else - ipc_log_header(sdev->dev, "ipc tx", msg->header); - - return ret; -} - /* send IPC message from host to DSP */ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, void *msg_data, size_t msg_bytes, void *reply_data, @@ -263,18 +245,23 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header, if (msg_bytes) memcpy(msg->msg_data, msg_data, msg_bytes); - /* send the message if not busy */ - if (snd_sof_dsp_is_ipc_ready(sdev)) { - ret = ipc_tx_next_msg(sdev->ipc); - /* Next reply that we receive will be related to this message */ - if (!ret) - msg->ipc_complete = false; - } else { - ret = -EAGAIN; - } + ret = snd_sof_dsp_send_msg(sdev, msg); + /* Next reply that we receive will be related to this message */ + if (!ret) + msg->ipc_complete = false; spin_unlock_irq(&sdev->ipc_lock); + if (ret < 0) { + /* So far IPC TX never fails, consider making the above void */ + dev_err_ratelimited(sdev->dev, + "error: ipc tx failed with error %d\n", + ret); + goto unlock; + } + + ipc_log_header(sdev->dev, "ipc tx", msg->header); + /* now wait for completion */ if (!ret) ret = tx_wait_done(ipc, msg, reply_data); @@ -301,11 +288,11 @@ int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id) unsigned long flags; /* - * Protect against a theoretical race with ipc_tx_next_msg(): if the DSP - * is fast enough to receive an IPC message, reply to it, and the host - * interrupt processing calls this function on a different core from the - * one, where the sending is taking place, the message might not yet be - * marked as expecting a reply. + * Protect against a theoretical race with sof_ipc_tx_message(): if the + * DSP is fast enough to receive an IPC message, reply to it, and the + * host interrupt processing calls this function on a different core + * from the one, where the sending is taking place, the message might + * not yet be marked as expecting a reply. */ spin_lock_irqsave(&sdev->ipc_lock, flags); @@ -701,7 +688,7 @@ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev) struct snd_sof_ipc_msg *msg; /* check if mandatory ops required for ipc are defined */ - if (!sof_ops(sdev)->is_ipc_ready || !sof_ops(sdev)->fw_ready) { + if (!sof_ops(sdev)->fw_ready) { dev_err(sdev->dev, "error: ipc mandatory ops not defined\n"); return NULL; } diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index e5a00f573c0608..23b57884c19984 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -267,15 +267,6 @@ static inline int snd_sof_dsp_get_reply(struct snd_sof_dev *sdev, return -ENOTSUPP; } -static inline int snd_sof_dsp_is_ipc_ready(struct snd_sof_dev *sdev) -{ - if (sof_ops(sdev)->is_ipc_ready) - return sof_ops(sdev)->is_ipc_ready(sdev); - - dev_err(sdev->dev, "error: %s not defined\n", __func__); - return -ENOTSUPP; -} - static inline int snd_sof_dsp_cmd_done(struct snd_sof_dev *sdev, int dir) { diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 5497534ae0136f..f5fc283d478f88 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -114,7 +114,6 @@ struct snd_sof_dsp_ops { struct snd_sof_ipc_msg *msg); /* mandatory */ int (*get_reply)(struct snd_sof_dev *sof_dev, struct snd_sof_ipc_msg *msg); /* mandatory */ - int (*is_ipc_ready)(struct snd_sof_dev *sof_dev); /* mandatory */ int (*cmd_done)(struct snd_sof_dev *sof_dev, int dir); /* mandatory */ /* FW loading */