From 274e58cc226c54c849760d9a6ec7be23b221cb12 Mon Sep 17 00:00:00 2001 From: Simon Horman Date: Sun, 8 Sep 2024 21:14:14 +0100 Subject: [PATCH 1/9] MAINTAINERS: Qualcomm SoC: Match reserved-memory bindings commit 8b90269ee6d7 ("MAINTAINERS: Split Qualcomm SoC and linux-arm-msm entries") included an entry for .../bindings/reserved-memory/qcom. However, it appears that this should have been a glob as although there are files that start with that path, no file matches that exact path. Address this by making the entry a glob. Flagged by make htmldocs as: Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/reserved-memory/qcom Cc: Konrad Dybcio Cc: Bjorn Andersson Cc: Rob Herring Cc: Krzysztof Kozlowski Cc: Conor Dooley Cc: linux-arm-msm@vger.kernel.org Cc: devicetree@vger.kernel.org Signed-off-by: Simon Horman Link: https://lore.kernel.org/r/20240908-qcom-glob-v1-1-94a390f36744@kernel.org Signed-off-by: Bjorn Andersson --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index c27f3190737f..2396d053a69f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2865,7 +2865,7 @@ F: Documentation/devicetree/bindings/arm/qcom.yaml F: Documentation/devicetree/bindings/bus/qcom* F: Documentation/devicetree/bindings/cache/qcom,llcc.yaml F: Documentation/devicetree/bindings/firmware/qcom,scm.yaml -F: Documentation/devicetree/bindings/reserved-memory/qcom +F: Documentation/devicetree/bindings/reserved-memory/qcom* F: Documentation/devicetree/bindings/soc/qcom/ F: arch/arm/boot/dts/qcom/ F: arch/arm/configs/qcom_defconfig From e694d2b5c58ba2d1e995d068707c8d966e7f5f2a Mon Sep 17 00:00:00 2001 From: Charles Han Date: Sun, 29 Sep 2024 15:23:49 +0800 Subject: [PATCH 2/9] soc: qcom: Add check devm_kasprintf() returned value devm_kasprintf() can return a NULL pointer on failure but this returned value in qcom_socinfo_probe() is not checked. Signed-off-by: Charles Han Link: https://lore.kernel.org/r/20240929072349.202520-1-hanchunchao@inspur.com Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/socinfo.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c index 24c3971f2ef1..e42d86d5f6f9 100644 --- a/drivers/soc/qcom/socinfo.c +++ b/drivers/soc/qcom/socinfo.c @@ -786,10 +786,16 @@ static int qcom_socinfo_probe(struct platform_device *pdev) qs->attr.revision = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%u.%u", SOCINFO_MAJOR(le32_to_cpu(info->ver)), SOCINFO_MINOR(le32_to_cpu(info->ver))); - if (offsetof(struct socinfo, serial_num) <= item_size) + if (!qs->attr.soc_id || qs->attr.revision) + return -ENOMEM; + + if (offsetof(struct socinfo, serial_num) <= item_size) { qs->attr.serial_number = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%u", le32_to_cpu(info->serial_num)); + if (!qs->attr.serial_number) + return -ENOMEM; + } qs->soc_dev = soc_device_register(&qs->attr); if (IS_ERR(qs->soc_dev)) From d67907154808745b0fae5874edc7b0f78d33991c Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 2 Oct 2024 12:01:21 +0200 Subject: [PATCH 3/9] firmware: qcom: scm: suppress download mode error Stop spamming the logs with errors about missing mechanism for setting the so called download (or dump) mode for users that have not requested that feature to be enabled in the first place. This avoids the follow error being logged on boot as well as on shutdown when the feature it not available and download mode has not been enabled on the kernel command line: qcom_scm firmware:scm: No available mechanism for setting download mode Fixes: 79cb2cb8d89b ("firmware: qcom: scm: Disable SDI and write no dump to dump mode") Fixes: 781d32d1c970 ("firmware: qcom_scm: Clear download bit during reboot") Cc: Mukesh Ojha Cc: stable@vger.kernel.org # 6.4 Signed-off-by: Johan Hovold Reviewed-by: Mukesh Ojha Link: https://lore.kernel.org/r/20241002100122.18809-2-johan+linaro@kernel.org Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom/qcom_scm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index 10986cb11ec0..e2ac595902ed 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -545,7 +545,7 @@ static void qcom_scm_set_download_mode(u32 dload_mode) } else if (__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_SET_DLOAD_MODE)) { ret = __qcom_scm_set_dload_mode(__scm->dev, !!dload_mode); - } else { + } else if (dload_mode) { dev_err(__scm->dev, "No available mechanism for setting download mode\n"); } From ca61d6836e6f4442a77762e1074d2706a2a6e578 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Mon, 30 Sep 2024 10:33:28 +0200 Subject: [PATCH 4/9] firmware: qcom: scm: fix a NULL-pointer dereference Some SCM calls can be invoked with __scm being NULL (the driver may not have been and will not be probed as there's no SCM entry in device-tree). Make sure we don't dereference a NULL pointer. Fixes: 449d0d84bcd8 ("firmware: qcom: scm: smc: switch to using the SCM allocator") Reported-by: Rudraksha Gupta Closes: https://lore.kernel.org/lkml/692cfe9a-8c05-4ce4-813e-82b3f310019a@gmail.com/ Reviewed-by: Konrad Dybcio Tested-by: Rudraksha Gupta Reviewed-by: Dmitry Baryshkov Reviewed-by: Stephan Gerhold Signed-off-by: Bartosz Golaszewski Reviewed-by: Kuldeep Singh Link: https://lore.kernel.org/r/20240930083328.17904-1-brgl@bgdev.pl Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom/qcom_scm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index e2ac595902ed..f47fdf32d2da 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -216,7 +216,7 @@ static DEFINE_SPINLOCK(scm_query_lock); struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void) { - return __scm->mempool; + return __scm ? __scm->mempool : NULL; } static enum qcom_scm_convention __get_convention(void) From 0a97195d2181caced187acd7454464b8e37021d7 Mon Sep 17 00:00:00 2001 From: Rajendra Nayak Date: Tue, 3 Sep 2024 15:45:10 +0530 Subject: [PATCH 5/9] EDAC/qcom: Make irq configuration optional On most modern qualcomm SoCs, the configuration necessary to enable the Tag/Data RAM related irqs being propagated to the SoC irq controller is already done in firmware (in DSF or 'DDR System Firmware') On some like the x1e80100, these registers aren't even accesible to the kernel causing a crash when edac device is probed. Hence, make the irq configuration optional in the driver and mark x1e80100 as the SoC on which this should be avoided. Fixes: af16b00578a7 ("arm64: dts: qcom: Add base X1E80100 dtsi and the QCP dts") Reported-by: Bjorn Andersson Signed-off-by: Rajendra Nayak Reviewed-by: Manivannan Sadhasivam Reviewed-by: Abel Vesa Link: https://lore.kernel.org/r/20240903101510.3452734-1-quic_rjendra@quicinc.com Signed-off-by: Bjorn Andersson --- drivers/edac/qcom_edac.c | 8 +++++--- drivers/soc/qcom/llcc-qcom.c | 3 +++ include/linux/soc/qcom/llcc-qcom.h | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c index d3cd4cc54ace..a9a8ba067007 100644 --- a/drivers/edac/qcom_edac.c +++ b/drivers/edac/qcom_edac.c @@ -342,9 +342,11 @@ static int qcom_llcc_edac_probe(struct platform_device *pdev) int ecc_irq; int rc; - rc = qcom_llcc_core_setup(llcc_driv_data, llcc_driv_data->bcast_regmap); - if (rc) - return rc; + if (!llcc_driv_data->ecc_irq_configured) { + rc = qcom_llcc_core_setup(llcc_driv_data, llcc_driv_data->bcast_regmap); + if (rc) + return rc; + } /* Allocate edac control info */ edev_ctl = edac_device_alloc_ctl_info(0, "qcom-llcc", 1, "bank", diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c index 8fa4ffd3a9b5..28bcc65e91be 100644 --- a/drivers/soc/qcom/llcc-qcom.c +++ b/drivers/soc/qcom/llcc-qcom.c @@ -139,6 +139,7 @@ struct qcom_llcc_config { int size; bool need_llcc_cfg; bool no_edac; + bool irq_configured; }; struct qcom_sct_config { @@ -718,6 +719,7 @@ static const struct qcom_llcc_config x1e80100_cfg[] = { .need_llcc_cfg = true, .reg_offset = llcc_v2_1_reg_offset, .edac_reg_offset = &llcc_v2_1_edac_reg_offset, + .irq_configured = true, }, }; @@ -1345,6 +1347,7 @@ static int qcom_llcc_probe(struct platform_device *pdev) drv_data->cfg = llcc_cfg; drv_data->cfg_size = sz; drv_data->edac_reg_offset = cfg->edac_reg_offset; + drv_data->ecc_irq_configured = cfg->irq_configured; mutex_init(&drv_data->lock); platform_set_drvdata(pdev, drv_data); diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h index 9e9f528b1370..2f20281d4ad4 100644 --- a/include/linux/soc/qcom/llcc-qcom.h +++ b/include/linux/soc/qcom/llcc-qcom.h @@ -125,6 +125,7 @@ struct llcc_edac_reg_offset { * @num_banks: Number of llcc banks * @bitmap: Bit map to track the active slice ids * @ecc_irq: interrupt for llcc cache error detection and reporting + * @ecc_irq_configured: 'True' if firmware has already configured the irq propagation * @version: Indicates the LLCC version */ struct llcc_drv_data { @@ -139,6 +140,7 @@ struct llcc_drv_data { u32 num_banks; unsigned long *bitmap; int ecc_irq; + bool ecc_irq_configured; u32 version; }; From f489f6c6eb26482010470d77bad3901a3de1b166 Mon Sep 17 00:00:00 2001 From: Qingqing Zhou Date: Wed, 23 Oct 2024 00:51:48 +0530 Subject: [PATCH 6/9] firmware: qcom: scm: Return -EOPNOTSUPP for unsupported SHM bridge enabling When enabling SHM bridge, QTEE returns 0 and sets error 4 in result to qcom_scm for unsupported platforms. Currently, tzmem interprets this as an unknown error rather than recognizing it as an unsupported platform. Error log: [ 0.177224] qcom_scm firmware:scm: error (____ptrval____): Failed to enable the TrustZone memory allocator [ 0.177244] qcom_scm firmware:scm: probe with driver qcom_scm failed with error 4 To address this, modify the function call qcom_scm_shm_bridge_enable() to remap result to indicate an unsupported error. This way, tzmem will correctly identify it as an unsupported platform case instead of reporting it as an error. Fixes: 178e19c0df1b ("firmware: qcom: scm: add support for SHM bridge operations") Signed-off-by: Qingqing Zhou Co-developed-by: Kuldeep Singh Signed-off-by: Kuldeep Singh Reviewed-by: Bartosz Golaszewski Reviewed-by: Mukesh Ojha Link: https://lore.kernel.org/r/20241022192148.1626633-1-quic_kuldsing@quicinc.com Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom/qcom_scm.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index f47fdf32d2da..2e4260ba5f79 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -112,6 +112,7 @@ enum qcom_scm_qseecom_tz_cmd_info { }; #define QSEECOM_MAX_APP_NAME_SIZE 64 +#define SHMBRIDGE_RESULT_NOTSUPP 4 /* Each bit configures cold/warm boot address for one of the 4 CPUs */ static const u8 qcom_scm_cpu_cold_bits[QCOM_SCM_BOOT_MAX_CPUS] = { @@ -1361,6 +1362,8 @@ EXPORT_SYMBOL_GPL(qcom_scm_lmh_dcvsh_available); int qcom_scm_shm_bridge_enable(void) { + int ret; + struct qcom_scm_desc desc = { .svc = QCOM_SCM_SVC_MP, .cmd = QCOM_SCM_MP_SHM_BRIDGE_ENABLE, @@ -1373,7 +1376,15 @@ int qcom_scm_shm_bridge_enable(void) QCOM_SCM_MP_SHM_BRIDGE_ENABLE)) return -EOPNOTSUPP; - return qcom_scm_call(__scm->dev, &desc, &res) ?: res.result[0]; + ret = qcom_scm_call(__scm->dev, &desc, &res); + + if (ret) + return ret; + + if (res.result[0] == SHMBRIDGE_RESULT_NOTSUPP) + return -EOPNOTSUPP; + + return res.result[0]; } EXPORT_SYMBOL_GPL(qcom_scm_shm_bridge_enable); From 128fdbf36cddc2a901c4889ba1c89fa9f2643f2c Mon Sep 17 00:00:00 2001 From: Manikanta Mylavarapu Date: Wed, 16 Oct 2024 20:18:52 +0530 Subject: [PATCH 7/9] soc: qcom: socinfo: fix revision check in qcom_socinfo_probe() In success case, the revision holds a non-null pointer. The current logic incorrectly returns an error for a non-null pointer, whereas it should return an error for a null pointer. The socinfo driver for IPQ9574 and IPQ5332 is currently broken, resulting in the following error message qcom-socinfo qcom-socinfo: probe with driver qcom-socinfo failed with error -12 Add a null check for the revision to ensure it returns an error only in failure case (null pointer). Fixes: e694d2b5c58b ("soc: qcom: Add check devm_kasprintf() returned value") Signed-off-by: Manikanta Mylavarapu Reviewed-by: Konrad Dybcio Link: https://lore.kernel.org/r/20241016144852.2888679-1-quic_mmanikan@quicinc.com Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/socinfo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c index e42d86d5f6f9..c3b877832521 100644 --- a/drivers/soc/qcom/socinfo.c +++ b/drivers/soc/qcom/socinfo.c @@ -786,7 +786,7 @@ static int qcom_socinfo_probe(struct platform_device *pdev) qs->attr.revision = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%u.%u", SOCINFO_MAJOR(le32_to_cpu(info->ver)), SOCINFO_MINOR(le32_to_cpu(info->ver))); - if (!qs->attr.soc_id || qs->attr.revision) + if (!qs->attr.soc_id || !qs->attr.revision) return -ENOMEM; if (offsetof(struct socinfo, serial_num) <= item_size) { From a387e73fedd6307c0e194deaa53c42b153ff0bd6 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Wed, 23 Oct 2024 17:24:32 +0000 Subject: [PATCH 8/9] rpmsg: glink: Handle rejected intent request better GLINK operates using pre-allocated buffers, aka intents, where incoming messages are aggregated before being passed up the stack. In the case that no suitable intents have been announced by the receiver, the sender can request an intent to be allocated. The initial implementation of the response to such request dealt with two outcomes; granted allocations, and all other cases being considered -ECANCELLED (likely from "cancelling the operation as the remote is going down"). But on some channels intent allocation is not supported, instead the remote will pre-allocate and announce a fixed number of intents for the sender to use. If for such channels an rpmsg_send() is being invoked before any channels have been announced, an intent request will be issued and as this comes back rejected the call fails with -ECANCELED. Given that this is reported in the same way as the remote being shut down, there's no way for the client to differentiate the two cases. In line with the original GLINK design, change the return value to -EAGAIN for the case where the remote rejects an intent allocation request. It's tempting to handle this case in the GLINK core, as we expect intents to show up in this case. But there's no way to distinguish between this case and a rejection for a too big allocation, nor is it possible to predict if a currently used (and seemingly suitable) intent will be returned for reuse or not. As such, returning the error to the client and allow it to react seems to be the only sensible solution. In addition to this, commit 'c05dfce0b89e ("rpmsg: glink: Wait for intent, not just request ack")' changed the logic such that the code always wait for an intent request response and an intent. This works out in most cases, but in the event that an intent request is rejected and no further intent arrives (e.g. client asks for a too big intent), the code will stall for 10 seconds and then return -ETIMEDOUT; instead of a more suitable error. This change also resulted in intent requests racing with the shutdown of the remote would be exposed to this same problem, unless some intent happens to arrive. A patch for this was developed and posted by Sarannya S [1], and has been incorporated here. To summarize, the intent request can end in 4 ways: - Timeout, no response arrived => return -ETIMEDOUT - Abort TX, the edge is going away => return -ECANCELLED - Intent request was rejected => return -EAGAIN - Intent request was accepted, and an intent arrived => return 0 This patch was developed with input from Sarannya S, Deepak Kumar Singh, and Chris Lew. [1] https://lore.kernel.org/all/20240925072328.1163183-1-quic_deesin@quicinc.com/ Fixes: c05dfce0b89e ("rpmsg: glink: Wait for intent, not just request ack") Cc: stable@vger.kernel.org Tested-by: Johan Hovold Signed-off-by: Bjorn Andersson Reviewed-by: Chris Lew Link: https://lore.kernel.org/r/20241023-pmic-glink-ecancelled-v2-1-ebc268129407@oss.qualcomm.com Signed-off-by: Bjorn Andersson --- drivers/rpmsg/qcom_glink_native.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c index 0b2f29006908..d3af1dfa3c7d 100644 --- a/drivers/rpmsg/qcom_glink_native.c +++ b/drivers/rpmsg/qcom_glink_native.c @@ -1440,14 +1440,18 @@ static int qcom_glink_request_intent(struct qcom_glink *glink, goto unlock; ret = wait_event_timeout(channel->intent_req_wq, - READ_ONCE(channel->intent_req_result) >= 0 && - READ_ONCE(channel->intent_received), + READ_ONCE(channel->intent_req_result) == 0 || + (READ_ONCE(channel->intent_req_result) > 0 && + READ_ONCE(channel->intent_received)) || + glink->abort_tx, 10 * HZ); if (!ret) { dev_err(glink->dev, "intent request timed out\n"); ret = -ETIMEDOUT; + } else if (glink->abort_tx) { + ret = -ECANCELED; } else { - ret = READ_ONCE(channel->intent_req_result) ? 0 : -ECANCELED; + ret = READ_ONCE(channel->intent_req_result) ? 0 : -EAGAIN; } unlock: From f8c879192465d9f328cb0df07208ef077c560bb1 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Wed, 23 Oct 2024 17:24:33 +0000 Subject: [PATCH 9/9] soc: qcom: pmic_glink: Handle GLINK intent allocation rejections Some versions of the pmic_glink firmware does not allow dynamic GLINK intent allocations, attempting to send a message before the firmware has allocated its receive buffers and announced these intent allocations will fail. When this happens something like this showns up in the log: pmic_glink_altmode.pmic_glink_altmode pmic_glink.altmode.0: failed to send altmode request: 0x10 (-125) pmic_glink_altmode.pmic_glink_altmode pmic_glink.altmode.0: failed to request altmode notifications: -125 ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: failed to send UCSI read request: -125 qcom_battmgr.pmic_glink_power_supply pmic_glink.power-supply.0: failed to request power notifications GLINK has been updated to distinguish between the cases where the remote is going down (-ECANCELED) and the intent allocation being rejected (-EAGAIN). Retry the send until intent buffers becomes available, or an actual error occur. To avoid infinitely waiting for the firmware in the event that this misbehaves and no intents arrive, an arbitrary 5 second timeout is used. This patch was developed with input from Chris Lew. Reported-by: Johan Hovold Closes: https://lore.kernel.org/all/Zqet8iInnDhnxkT9@hovoldconsulting.com/#t Cc: stable@vger.kernel.org # rpmsg: glink: Handle rejected intent request better Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK driver") Tested-by: Johan Hovold Reviewed-by: Johan Hovold Signed-off-by: Bjorn Andersson Reviewed-by: Chris Lew Link: https://lore.kernel.org/r/20241023-pmic-glink-ecancelled-v2-2-ebc268129407@oss.qualcomm.com Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/pmic_glink.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c index 9606222993fd..baa4ac6704a9 100644 --- a/drivers/soc/qcom/pmic_glink.c +++ b/drivers/soc/qcom/pmic_glink.c @@ -4,6 +4,7 @@ * Copyright (c) 2022, Linaro Ltd */ #include +#include #include #include #include @@ -13,6 +14,8 @@ #include #include +#define PMIC_GLINK_SEND_TIMEOUT (5 * HZ) + enum { PMIC_GLINK_CLIENT_BATT = 0, PMIC_GLINK_CLIENT_ALTMODE, @@ -112,13 +115,29 @@ EXPORT_SYMBOL_GPL(pmic_glink_client_register); int pmic_glink_send(struct pmic_glink_client *client, void *data, size_t len) { struct pmic_glink *pg = client->pg; + bool timeout_reached = false; + unsigned long start; int ret; mutex_lock(&pg->state_lock); - if (!pg->ept) + if (!pg->ept) { ret = -ECONNRESET; - else - ret = rpmsg_send(pg->ept, data, len); + } else { + start = jiffies; + for (;;) { + ret = rpmsg_send(pg->ept, data, len); + if (ret != -EAGAIN) + break; + + if (timeout_reached) { + ret = -ETIMEDOUT; + break; + } + + usleep_range(1000, 5000); + timeout_reached = time_after(jiffies, start + PMIC_GLINK_SEND_TIMEOUT); + } + } mutex_unlock(&pg->state_lock); return ret;