From 1fcef985c8bdd542c43da0d87bd9d51980c3859b Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 11 Mar 2021 16:22:51 -0800 Subject: [PATCH 1/6] remoteproc: qcom: wcnss: Fix race with iris probe The remoteproc driver is split between the responsibilities of getting the SoC-internal ARM core up and running and the external RF (aka "Iris") part configured. In order to satisfy the regulator framework's need of a struct device * to look up supplies this was implemented as two different drivers, using of_platform_populate() in the remoteproc part to probe the iris part. Unfortunately it's possible that the iris part probe defers on yet not available regulators and an attempt to start the remoteproc will have to be rejected, until this has been resolved. But there's no useful mechanism of knowing when this would be. Instead replace the of_platform_populate() and the iris probe with a function that rolls its own struct device, with the relevant of_node associated that is enough to acquire regulators and clocks specified in the DT node and that may propagate the EPROBE_DEFER back to the wcnss device's probe. Acked-by: Mathieu Poirier Reported-by: Anibal Limon Reported-by: Loic Poulain Tested-by: Anibal Limon Link: https://lore.kernel.org/r/20210312002251.3273013-1-bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson --- drivers/remoteproc/qcom_wcnss.c | 49 +++-------- drivers/remoteproc/qcom_wcnss.h | 4 +- drivers/remoteproc/qcom_wcnss_iris.c | 120 +++++++++++++++++---------- 3 files changed, 89 insertions(+), 84 deletions(-) diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c index f1cbc6b2edbb..ebadc6c08e11 100644 --- a/drivers/remoteproc/qcom_wcnss.c +++ b/drivers/remoteproc/qcom_wcnss.c @@ -142,18 +142,6 @@ static const struct wcnss_data pronto_v2_data = { .num_vregs = 1, }; -void qcom_wcnss_assign_iris(struct qcom_wcnss *wcnss, - struct qcom_iris *iris, - bool use_48mhz_xo) -{ - mutex_lock(&wcnss->iris_lock); - - wcnss->iris = iris; - wcnss->use_48mhz_xo = use_48mhz_xo; - - mutex_unlock(&wcnss->iris_lock); -} - static int wcnss_load(struct rproc *rproc, const struct firmware *fw) { struct qcom_wcnss *wcnss = (struct qcom_wcnss *)rproc->priv; @@ -639,12 +627,20 @@ static int wcnss_probe(struct platform_device *pdev) goto detach_pds; } + wcnss->iris = qcom_iris_probe(&pdev->dev, &wcnss->use_48mhz_xo); + if (IS_ERR(wcnss->iris)) { + ret = PTR_ERR(wcnss->iris); + goto detach_pds; + } + ret = rproc_add(rproc); if (ret) - goto detach_pds; + goto remove_iris; - return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev); + return 0; +remove_iris: + qcom_iris_remove(wcnss->iris); detach_pds: wcnss_release_pds(wcnss); free_rproc: @@ -657,7 +653,7 @@ static int wcnss_remove(struct platform_device *pdev) { struct qcom_wcnss *wcnss = platform_get_drvdata(pdev); - of_platform_depopulate(&pdev->dev); + qcom_iris_remove(wcnss->iris); rproc_del(wcnss->rproc); @@ -686,28 +682,7 @@ static struct platform_driver wcnss_driver = { }, }; -static int __init wcnss_init(void) -{ - int ret; - - ret = platform_driver_register(&wcnss_driver); - if (ret) - return ret; - - ret = platform_driver_register(&qcom_iris_driver); - if (ret) - platform_driver_unregister(&wcnss_driver); - - return ret; -} -module_init(wcnss_init); - -static void __exit wcnss_exit(void) -{ - platform_driver_unregister(&qcom_iris_driver); - platform_driver_unregister(&wcnss_driver); -} -module_exit(wcnss_exit); +module_platform_driver(wcnss_driver); MODULE_DESCRIPTION("Qualcomm Peripheral Image Loader for Wireless Subsystem"); MODULE_LICENSE("GPL v2"); diff --git a/drivers/remoteproc/qcom_wcnss.h b/drivers/remoteproc/qcom_wcnss.h index 62c8682d0a92..6d01ee6afa7f 100644 --- a/drivers/remoteproc/qcom_wcnss.h +++ b/drivers/remoteproc/qcom_wcnss.h @@ -17,9 +17,9 @@ struct wcnss_vreg_info { bool super_turbo; }; +struct qcom_iris *qcom_iris_probe(struct device *parent, bool *use_48mhz_xo); +void qcom_iris_remove(struct qcom_iris *iris); int qcom_iris_enable(struct qcom_iris *iris); void qcom_iris_disable(struct qcom_iris *iris); -void qcom_wcnss_assign_iris(struct qcom_wcnss *wcnss, struct qcom_iris *iris, bool use_48mhz_xo); - #endif diff --git a/drivers/remoteproc/qcom_wcnss_iris.c b/drivers/remoteproc/qcom_wcnss_iris.c index 169acd305ae3..09720ddddc85 100644 --- a/drivers/remoteproc/qcom_wcnss_iris.c +++ b/drivers/remoteproc/qcom_wcnss_iris.c @@ -17,7 +17,7 @@ #include "qcom_wcnss.h" struct qcom_iris { - struct device *dev; + struct device dev; struct clk *xo_clk; @@ -75,7 +75,7 @@ int qcom_iris_enable(struct qcom_iris *iris) ret = clk_prepare_enable(iris->xo_clk); if (ret) { - dev_err(iris->dev, "failed to enable xo clk\n"); + dev_err(&iris->dev, "failed to enable xo clk\n"); goto disable_regulators; } @@ -93,43 +93,90 @@ void qcom_iris_disable(struct qcom_iris *iris) regulator_bulk_disable(iris->num_vregs, iris->vregs); } -static int qcom_iris_probe(struct platform_device *pdev) +static const struct of_device_id iris_of_match[] = { + { .compatible = "qcom,wcn3620", .data = &wcn3620_data }, + { .compatible = "qcom,wcn3660", .data = &wcn3660_data }, + { .compatible = "qcom,wcn3660b", .data = &wcn3680_data }, + { .compatible = "qcom,wcn3680", .data = &wcn3680_data }, + {} +}; + +static void qcom_iris_release(struct device *dev) { + struct qcom_iris *iris = container_of(dev, struct qcom_iris, dev); + + of_node_put(iris->dev.of_node); + kfree(iris); +} + +struct qcom_iris *qcom_iris_probe(struct device *parent, bool *use_48mhz_xo) +{ + const struct of_device_id *match; const struct iris_data *data; - struct qcom_wcnss *wcnss; + struct device_node *of_node; struct qcom_iris *iris; int ret; int i; - iris = devm_kzalloc(&pdev->dev, sizeof(struct qcom_iris), GFP_KERNEL); - if (!iris) - return -ENOMEM; + of_node = of_get_child_by_name(parent->of_node, "iris"); + if (!of_node) { + dev_err(parent, "No child node \"iris\" found\n"); + return ERR_PTR(-EINVAL); + } - data = of_device_get_match_data(&pdev->dev); - wcnss = dev_get_drvdata(pdev->dev.parent); + iris = kzalloc(sizeof(*iris), GFP_KERNEL); + if (!iris) { + of_node_put(of_node); + return ERR_PTR(-ENOMEM); + } - iris->xo_clk = devm_clk_get(&pdev->dev, "xo"); + device_initialize(&iris->dev); + iris->dev.parent = parent; + iris->dev.release = qcom_iris_release; + iris->dev.of_node = of_node; + + dev_set_name(&iris->dev, "%s.iris", dev_name(parent)); + + ret = device_add(&iris->dev); + if (ret) { + put_device(&iris->dev); + return ERR_PTR(ret); + } + + match = of_match_device(iris_of_match, &iris->dev); + if (!match) { + dev_err(&iris->dev, "no matching compatible for iris\n"); + ret = -EINVAL; + goto err_device_del; + } + + data = match->data; + + iris->xo_clk = devm_clk_get(&iris->dev, "xo"); if (IS_ERR(iris->xo_clk)) { - if (PTR_ERR(iris->xo_clk) != -EPROBE_DEFER) - dev_err(&pdev->dev, "failed to acquire xo clk\n"); - return PTR_ERR(iris->xo_clk); + ret = PTR_ERR(iris->xo_clk); + if (ret != -EPROBE_DEFER) + dev_err(&iris->dev, "failed to acquire xo clk\n"); + goto err_device_del; } iris->num_vregs = data->num_vregs; - iris->vregs = devm_kcalloc(&pdev->dev, + iris->vregs = devm_kcalloc(&iris->dev, iris->num_vregs, sizeof(struct regulator_bulk_data), GFP_KERNEL); - if (!iris->vregs) - return -ENOMEM; + if (!iris->vregs) { + ret = -ENOMEM; + goto err_device_del; + } for (i = 0; i < iris->num_vregs; i++) iris->vregs[i].supply = data->vregs[i].name; - ret = devm_regulator_bulk_get(&pdev->dev, iris->num_vregs, iris->vregs); + ret = devm_regulator_bulk_get(&iris->dev, iris->num_vregs, iris->vregs); if (ret) { - dev_err(&pdev->dev, "failed to get regulators\n"); - return ret; + dev_err(&iris->dev, "failed to get regulators\n"); + goto err_device_del; } for (i = 0; i < iris->num_vregs; i++) { @@ -143,34 +190,17 @@ static int qcom_iris_probe(struct platform_device *pdev) data->vregs[i].load_uA); } - qcom_wcnss_assign_iris(wcnss, iris, data->use_48mhz_xo); + *use_48mhz_xo = data->use_48mhz_xo; - return 0; + return iris; + +err_device_del: + device_del(&iris->dev); + + return ERR_PTR(ret); } -static int qcom_iris_remove(struct platform_device *pdev) +void qcom_iris_remove(struct qcom_iris *iris) { - struct qcom_wcnss *wcnss = dev_get_drvdata(pdev->dev.parent); - - qcom_wcnss_assign_iris(wcnss, NULL, false); - - return 0; + device_del(&iris->dev); } - -static const struct of_device_id iris_of_match[] = { - { .compatible = "qcom,wcn3620", .data = &wcn3620_data }, - { .compatible = "qcom,wcn3660", .data = &wcn3660_data }, - { .compatible = "qcom,wcn3660b", .data = &wcn3680_data }, - { .compatible = "qcom,wcn3680", .data = &wcn3680_data }, - {} -}; -MODULE_DEVICE_TABLE(of, iris_of_match); - -struct platform_driver qcom_iris_driver = { - .probe = qcom_iris_probe, - .remove = qcom_iris_remove, - .driver = { - .name = "qcom-iris", - .of_match_table = iris_of_match, - }, -}; From c080128b6f05cb803d830e6bf2ec0b214435ce38 Mon Sep 17 00:00:00 2001 From: Dong Aisheng Date: Tue, 6 Jul 2021 22:21:55 +0800 Subject: [PATCH 2/6] remoteproc: fix an typo in fw_elf_get_class code comments Drop 'and' which looks like unnecessary. Fixes: 73516a33588c ("remoteproc: Add elf helpers to access elf64 and elf32 fields") Signed-off-by: Dong Aisheng Link: https://lore.kernel.org/r/20210706142156.952794-1-aisheng.dong@nxp.com Signed-off-by: Bjorn Andersson --- drivers/remoteproc/remoteproc_elf_helpers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/remoteproc/remoteproc_elf_helpers.h b/drivers/remoteproc/remoteproc_elf_helpers.h index 26404e68e17a..e6de53a5000c 100644 --- a/drivers/remoteproc/remoteproc_elf_helpers.h +++ b/drivers/remoteproc/remoteproc_elf_helpers.h @@ -15,7 +15,7 @@ * fw_elf_get_class - Get elf class * @fw: the ELF firmware image * - * Note that we use and elf32_hdr to access the class since the start of the + * Note that we use elf32_hdr to access the class since the start of the * struct is the same for both elf class * * Return: elf class of the firmware From 147b589c5f446d602215577835356a96c40a4044 Mon Sep 17 00:00:00 2001 From: Dong Aisheng Date: Tue, 6 Jul 2021 22:21:56 +0800 Subject: [PATCH 3/6] remoteproc: fix kernel doc for struct rproc_ops The load_rsc_table was removed since the commit c1d35c1ab424 ("remoteproc: Rename "load_rsc_table" to "parse_fw"") but got added back again by mistake in the below commit: commit b1a17513a2d6 ("remoteproc: add vendor resources handling"). The patch fixed a small code indent issue which not worth a separate patch. Fixes: b1a17513a2d6 ("remoteproc: add vendor resources handling") Signed-off-by: Dong Aisheng Link: https://lore.kernel.org/r/20210706142156.952794-2-aisheng.dong@nxp.com Signed-off-by: Bjorn Andersson --- include/linux/remoteproc.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index a5b37bc10865..83c09ac36b13 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -369,9 +369,8 @@ enum rsc_handling_status { * @da_to_va: optional platform hook to perform address translations * @parse_fw: parse firmware to extract information (e.g. resource table) * @handle_rsc: optional platform hook to handle vendor resources. Should return - * RSC_HANDLED if resource was handled, RSC_IGNORED if not handled and a - * negative value on error - * @load_rsc_table: load resource table from firmware image + * RSC_HANDLED if resource was handled, RSC_IGNORED if not handled + * and a negative value on error * @find_loaded_rsc_table: find the loaded resource table from firmware image * @get_loaded_rsc_table: get resource table installed in memory * by external entity From 3ad51c1743ebd23ec3b5ebc6195dafe867eaebb1 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Wed, 19 May 2021 18:44:18 -0500 Subject: [PATCH 4/6] remoteproc: use freezable workqueue for crash notifications When a remoteproc has crashed, rproc_report_crash() is called to handle whatever recovery is desired. This can happen at almost any time, often triggered by an interrupt, though it can also be initiated by a write to debugfs file remoteproc/remoteproc*/crash. When a crash is reported, the crash handler worker is scheduled to run (rproc_crash_handler_work()). One thing that worker does is call rproc_trigger_recovery(), which calls rproc_stop(). That calls the ->stop method for any remoteproc subdevices before making the remote processor go offline. The Q6V5 modem remoteproc driver implements an SSR subdevice that notifies registered drivers when the modem changes operational state (prepare, started, stop/crash, unprepared). The IPA driver registers to receive these notifications. With that as context, I'll now describe the problem. There was a situation in which buggy modem firmware led to a modem crash very soon after system (AP) resume had begun. The crash caused a remoteproc SSR crash notification to be sent to the IPA driver. The problem was that, although system resume had begun, it had not yet completed, and the IPA driver was still in a suspended state. This scenario could happen to any driver that registers for these SSR notifications, because they are delivered without knowledge of the (suspend) state of registered recipient drivers. This patch offers a simple fix for this, by having the crash handling worker function run on the system freezable workqueue. This workqueue does not operate if user space is frozen (for suspend). As a result, the SSR subdevice only delivers its crash notification when the system is fully operational (i.e., neither suspended nor in suspend/resume transition). Tested-by: Siddharth Gupta Reviewed-by: Bjorn Andersson Reviewed-by: Mathieu Poirier Signed-off-by: Alex Elder Link: https://lore.kernel.org/r/20210519234418.1196387-2-elder@linaro.org Signed-off-by: Bjorn Andersson --- drivers/remoteproc/remoteproc_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 7de5905d276a..502b6604b757 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -2750,8 +2750,8 @@ void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type) dev_err(&rproc->dev, "crash detected in %s: type %s\n", rproc->name, rproc_crash_to_string(type)); - /* create a new task to handle the error */ - schedule_work(&rproc->crash_handler); + /* Have a worker handle the error; ensure system is not suspended */ + queue_work(system_freezable_wq, &rproc->crash_handler); } EXPORT_SYMBOL(rproc_report_crash); From f35ef8e4ea0a2b2b35a2c7009fc07b6d80a2b2f3 Mon Sep 17 00:00:00 2001 From: Konrad Dybcio Date: Wed, 28 Jul 2021 23:52:11 +0200 Subject: [PATCH 5/6] dt-bindings: remoteproc: qcom: adsp: Add SDM660 ADSP Add a compatible string for SDM660 ADSP. Signed-off-by: Konrad Dybcio Link: https://lore.kernel.org/r/20210728215212.18217-1-konrad.dybcio@somainline.org [bjorn: Use the -pas suffix] Signed-off-by: Bjorn Andersson --- Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml index c597ccced623..0c112f3264a9 100644 --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml @@ -28,6 +28,7 @@ properties: - qcom,sc8180x-adsp-pas - qcom,sc8180x-cdsp-pas - qcom,sc8180x-mpss-pas + - qcom,sdm660-adsp-pas - qcom,sdm845-adsp-pas - qcom,sdm845-cdsp-pas - qcom,sdx55-mpss-pas From a0a77028c85ad1f6f36c3ceea21b30dc43721665 Mon Sep 17 00:00:00 2001 From: Konrad Dybcio Date: Wed, 28 Jul 2021 23:52:12 +0200 Subject: [PATCH 6/6] remoteproc: q6v5_pas: Add sdm660 ADSP PIL compatible This chipset seems to work fine with the "generic" configuration. Signed-off-by: Konrad Dybcio Link: https://lore.kernel.org/r/20210728215212.18217-2-konrad.dybcio@somainline.org [bjorn: Use "-pas" suffix for remoteprocs using TrustZone] Signed-off-by: Bjorn Andersson --- drivers/remoteproc/qcom_q6v5_pas.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index a79bee901e9b..401b1ec90785 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -833,6 +833,7 @@ static const struct of_device_id adsp_of_match[] = { { .compatible = "qcom,sc8180x-adsp-pas", .data = &sm8150_adsp_resource}, { .compatible = "qcom,sc8180x-cdsp-pas", .data = &sm8150_cdsp_resource}, { .compatible = "qcom,sc8180x-mpss-pas", .data = &sc8180x_mpss_resource}, + { .compatible = "qcom,sdm660-adsp-pas", .data = &adsp_resource_init}, { .compatible = "qcom,sdm845-adsp-pas", .data = &adsp_resource_init}, { .compatible = "qcom,sdm845-cdsp-pas", .data = &cdsp_resource_init}, { .compatible = "qcom,sdx55-mpss-pas", .data = &sdx55_mpss_resource},