From 1ac19ad799f880af58b9a8a4321334f6f6fc72e6 Mon Sep 17 00:00:00 2001 From: Srinivas Kandagatla Date: Fri, 8 Feb 2019 17:55:10 +0000 Subject: qcom: apr: Make apr callbacks in non-atomic context APR communication with DSP is not atomic in nature. Its request-response type. Trying to pretend that these are atomic and invoking apr client callbacks directly under atomic/irq context has endless issues with soundcard. It makes more sense to convert these to nonatomic calls. This also coverts all the dais to be nonatomic. All the callbacks are now invoked as part of rx work queue. Signed-off-by: Srinivas Kandagatla Reviewed-by: Bjorn Andersson Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/apr.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 5 deletions(-) (limited to 'drivers/soc/qcom/apr.c') diff --git a/drivers/soc/qcom/apr.c b/drivers/soc/qcom/apr.c index 74f8b9607daa..039e3aa6f5e0 100644 --- a/drivers/soc/qcom/apr.c +++ b/drivers/soc/qcom/apr.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -17,8 +18,18 @@ struct apr { struct rpmsg_endpoint *ch; struct device *dev; spinlock_t svcs_lock; + spinlock_t rx_lock; struct idr svcs_idr; int dest_domain_id; + struct workqueue_struct *rxwq; + struct work_struct rx_work; + struct list_head rx_list; +}; + +struct apr_rx_buf { + struct list_head node; + int len; + uint8_t buf[]; }; /** @@ -62,11 +73,7 @@ static int apr_callback(struct rpmsg_device *rpdev, void *buf, int len, void *priv, u32 addr) { struct apr *apr = dev_get_drvdata(&rpdev->dev); - uint16_t hdr_size, msg_type, ver, svc_id; - struct apr_device *svc = NULL; - struct apr_driver *adrv = NULL; - struct apr_resp_pkt resp; - struct apr_hdr *hdr; + struct apr_rx_buf *abuf; unsigned long flags; if (len <= APR_HDR_SIZE) { @@ -75,6 +82,34 @@ static int apr_callback(struct rpmsg_device *rpdev, void *buf, return -EINVAL; } + abuf = kzalloc(sizeof(*abuf) + len, GFP_ATOMIC); + if (!abuf) + return -ENOMEM; + + abuf->len = len; + memcpy(abuf->buf, buf, len); + + spin_lock_irqsave(&apr->rx_lock, flags); + list_add_tail(&abuf->node, &apr->rx_list); + spin_unlock_irqrestore(&apr->rx_lock, flags); + + queue_work(apr->rxwq, &apr->rx_work); + + return 0; +} + + +static int apr_do_rx_callback(struct apr *apr, struct apr_rx_buf *abuf) +{ + uint16_t hdr_size, msg_type, ver, svc_id; + struct apr_device *svc = NULL; + struct apr_driver *adrv = NULL; + struct apr_resp_pkt resp; + struct apr_hdr *hdr; + unsigned long flags; + void *buf = abuf->buf; + int len = abuf->len; + hdr = buf; ver = APR_HDR_FIELD_VER(hdr->hdr_field); if (ver > APR_PKT_VER + 1) @@ -132,6 +167,23 @@ static int apr_callback(struct rpmsg_device *rpdev, void *buf, return 0; } +static void apr_rxwq(struct work_struct *work) +{ + struct apr *apr = container_of(work, struct apr, rx_work); + struct apr_rx_buf *abuf, *b; + unsigned long flags; + + if (!list_empty(&apr->rx_list)) { + list_for_each_entry_safe(abuf, b, &apr->rx_list, node) { + apr_do_rx_callback(apr, abuf); + spin_lock_irqsave(&apr->rx_lock, flags); + list_del(&abuf->node); + spin_unlock_irqrestore(&apr->rx_lock, flags); + kfree(abuf); + } + } +} + static int apr_device_match(struct device *dev, struct device_driver *drv) { struct apr_device *adev = to_apr_device(dev); @@ -285,6 +337,14 @@ static int apr_probe(struct rpmsg_device *rpdev) dev_set_drvdata(dev, apr); apr->ch = rpdev->ept; apr->dev = dev; + apr->rxwq = create_singlethread_workqueue("qcom_apr_rx"); + if (!apr->rxwq) { + dev_err(apr->dev, "Failed to start Rx WQ\n"); + return -ENOMEM; + } + INIT_WORK(&apr->rx_work, apr_rxwq); + INIT_LIST_HEAD(&apr->rx_list); + spin_lock_init(&apr->rx_lock); spin_lock_init(&apr->svcs_lock); idr_init(&apr->svcs_idr); of_register_apr_devices(dev); @@ -303,7 +363,11 @@ static int apr_remove_device(struct device *dev, void *null) static void apr_remove(struct rpmsg_device *rpdev) { + struct apr *apr = dev_get_drvdata(&rpdev->dev); + device_for_each_child(&rpdev->dev, NULL, apr_remove_device); + flush_workqueue(apr->rxwq); + destroy_workqueue(apr->rxwq); } /* -- cgit v1.2.3-59-g8ed1b From 70d22b78d3235303555c921246e3c1ec37b0a29c Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 23 May 2019 08:01:53 -0700 Subject: soc: qcom: apr: Don't use reg for domain id The reg property represents the address and size on the bus that a device lives, but for APR the parent is a rpmsg bus, which does not have numerical addresses. Simply defining #address/#size-cells to 1 and 0, respectively, to silence the compiler is not an appropriate solution. Replace the use of "reg" with an APR specific property. Reviewed-by: Srinivas Kandagatla Reviewed-by: Rob Herring Signed-off-by: Bjorn Andersson --- Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt | 6 +++--- drivers/soc/qcom/apr.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/soc/qcom/apr.c') diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt index bcc612cc7423..db501269f47b 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt @@ -9,7 +9,7 @@ used for audio/voice services on the QDSP. Value type: Definition: must be "qcom,apr-v", example "qcom,apr-v2" -- reg +- qcom,apr-domain Usage: required Value type: Definition: Destination processor ID. @@ -49,9 +49,9 @@ by the individual bindings for the specific service The following example represents a QDSP based sound card on a MSM8996 device which uses apr as communication between Apps and QDSP. - apr@4 { + apr { compatible = "qcom,apr-v2"; - reg = ; + qcom,apr-domain = ; q6core@3 { compatible = "qcom,q6core"; diff --git a/drivers/soc/qcom/apr.c b/drivers/soc/qcom/apr.c index 039e3aa6f5e0..4fcc32420c47 100644 --- a/drivers/soc/qcom/apr.c +++ b/drivers/soc/qcom/apr.c @@ -328,7 +328,7 @@ static int apr_probe(struct rpmsg_device *rpdev) if (!apr) return -ENOMEM; - ret = of_property_read_u32(dev->of_node, "reg", &apr->dest_domain_id); + ret = of_property_read_u32(dev->of_node, "qcom,apr-domain", &apr->dest_domain_id); if (ret) { dev_err(dev, "APR Domain ID not specified in DT\n"); return ret; -- cgit v1.2.3-59-g8ed1b