linux-edac.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks
@ 2023-01-18 15:08 Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value Manivannan Sadhasivam
                   ` (16 more replies)
  0 siblings, 17 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The Qualcomm LLCC/EDAC drivers were using a fixed register stride for
accessing the (Control and Status Regsiters) CSRs of each LLCC bank.
This offset only works for some SoCs like SDM845 for which driver support
was initially added.
    
But the later SoCs use different register stride that vary between the
banks with holes in-between. So it is not possible to use a single register
stride for accessing the CSRs of each bank. By doing so could result in a
crash with the current drivers. So far this crash is not reported since
EDAC_QCOM driver is not enabled in ARM64 defconfig and no one tested the
driver extensively by triggering the EDAC IRQ (that's where each bank
CSRs are accessed).

For fixing this issue, let's obtain the base address of each LLCC bank from
devicetree and get rid of the fixed stride.

This series has been tested on SM8250, SM8450, SM6350, SC8280XP, SA8540P,
and SDM845.

Merging strategy
----------------

All patches should be merged to qcom tree due to LLCC dependency.

Thanks,
Mani

Changes in v6:

* Incorporated comments from Borislav for the EDAC patches and collected
  review tags.

Changes in v5:

* Reduced the size of llcc0 to 0x45000 on SDM845 due to overlapping with BWMON
* Added a patch to disable creation of EDAC platform device on SDM845
* Rebase on top of v6.2-rc1
* Moved the EDAC specific patches to the start so that they can be applied
  independently of LLCC patches

Changes in v4:

* Added a patch that fixes the use-after-free bug in qcom_edac driver

Changes in v3:

* Brought back reg-names property for compatibility (Krzysztof)
* Removed Fixes tag and stable list as backporting the drivers/binding/dts
  patches alone would break (Krzysztof)
* Fixed the uninitialized variable issue (Kbot)
* Added a patch to make use of driver supplied polling interval (Luca)
* Added a patch for module autoloading (Andrew)
* Didn't collect Review tags from Sai as the dts patches were changed.

Changes in v2:

* Removed reg-names property and used index of reg property to parse LLCC
  bank base address (Bjorn)
* Collected Ack from Sai for binding
* Added a new patch for polling mode (Luca)
* Renamed subject of patches targeting SC7180 and SM6350

Manivannan Sadhasivam (17):
  EDAC/device: Respect any driver-supplied workqueue polling value
  EDAC/qcom: Add platform_device_id table for module autoloading
  EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's
    pvt_info
  dt-bindings: arm: msm: Update the maintainers for LLCC
  dt-bindings: arm: msm: Fix register regions used for LLCC banks
  arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks
  arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks
  qcom: llcc/edac: Fix the base address used for accessing LLCC banks
  qcom: llcc/edac: Support polling mode for ECC handling
  soc: qcom: llcc: Do not create EDAC platform device on SDM845

 .../bindings/arm/msm/qcom,llcc.yaml           | 128 ++++++++++++++++--
 arch/arm64/boot/dts/qcom/sc7180.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sc7280.dtsi          |   5 +-
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi        |  10 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |   7 +-
 arch/arm64/boot/dts/qcom/sm6350.dtsi          |   2 +-
 arch/arm64/boot/dts/qcom/sm8150.dtsi          |   7 +-
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |   7 +-
 arch/arm64/boot/dts/qcom/sm8350.dtsi          |   7 +-
 arch/arm64/boot/dts/qcom/sm8450.dtsi          |   7 +-
 drivers/edac/edac_device.c                    |  15 +-
 drivers/edac/qcom_edac.c                      |  76 ++++++-----
 drivers/soc/qcom/llcc-qcom.c                  |  80 ++++++-----
 include/linux/soc/qcom/llcc-qcom.h            |   6 +-
 14 files changed, 253 insertions(+), 106 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 17:46   ` Bjorn Andersson
  2023-01-19 10:55   ` Borislav Petkov
  2023-01-18 15:08 ` [PATCH v6 02/17] EDAC/qcom: Add platform_device_id table for module autoloading Manivannan Sadhasivam
                   ` (15 subsequent siblings)
  16 siblings, 2 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam, stable

The EDAC drivers may optionally pass the poll_msec value. Use that value
if available, else fall back to 1000ms.

  [ bp: Touchups. ]

Fixes: e27e3dac6517 ("drivers/edac: add edac_device class")
Reported-by: Luca Weiss <luca.weiss@fairphone.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
Cc: <stable@vger.kernel.org> # 4.9
Link: https://lore.kernel.org/r/COZYL8MWN97H.MROQ391BGA09@otso
---
 drivers/edac/edac_device.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index 19522c568aa5..a50b7bcfb731 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -34,6 +34,9 @@
 static DEFINE_MUTEX(device_ctls_mutex);
 static LIST_HEAD(edac_device_list);
 
+/* Default workqueue processing interval on this instance, in msecs */
+#define DEFAULT_POLL_INTERVAL 1000
+
 #ifdef CONFIG_EDAC_DEBUG
 static void edac_device_dump_device(struct edac_device_ctl_info *edac_dev)
 {
@@ -336,7 +339,7 @@ static void edac_device_workq_function(struct work_struct *work_req)
 	 * whole one second to save timers firing all over the period
 	 * between integral seconds
 	 */
-	if (edac_dev->poll_msec == 1000)
+	if (edac_dev->poll_msec == DEFAULT_POLL_INTERVAL)
 		edac_queue_work(&edac_dev->work, round_jiffies_relative(edac_dev->delay));
 	else
 		edac_queue_work(&edac_dev->work, edac_dev->delay);
@@ -366,7 +369,7 @@ static void edac_device_workq_setup(struct edac_device_ctl_info *edac_dev,
 	 * timers firing on sub-second basis, while they are happy
 	 * to fire together on the 1 second exactly
 	 */
-	if (edac_dev->poll_msec == 1000)
+	if (edac_dev->poll_msec == DEFAULT_POLL_INTERVAL)
 		edac_queue_work(&edac_dev->work, round_jiffies_relative(edac_dev->delay));
 	else
 		edac_queue_work(&edac_dev->work, edac_dev->delay);
@@ -398,7 +401,7 @@ void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev,
 {
 	unsigned long jiffs = msecs_to_jiffies(value);
 
-	if (value == 1000)
+	if (value == DEFAULT_POLL_INTERVAL)
 		jiffs = round_jiffies_relative(value);
 
 	edac_dev->poll_msec = value;
@@ -443,11 +446,7 @@ int edac_device_add_device(struct edac_device_ctl_info *edac_dev)
 		/* This instance is NOW RUNNING */
 		edac_dev->op_state = OP_RUNNING_POLL;
 
-		/*
-		 * enable workq processing on this instance,
-		 * default = 1000 msec
-		 */
-		edac_device_workq_setup(edac_dev, 1000);
+		edac_device_workq_setup(edac_dev, edac_dev->poll_msec ?: DEFAULT_POLL_INTERVAL);
 	} else {
 		edac_dev->op_state = OP_RUNNING_INTERRUPT;
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 02/17] EDAC/qcom: Add platform_device_id table for module autoloading
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-20 16:10   ` Borislav Petkov
  2023-01-18 15:08 ` [PATCH v6 03/17] EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's pvt_info Manivannan Sadhasivam
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

Add a device ID table so that the driver loads automatically when the
associated platform_device gets registered.

Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
Reported-by: Andrew Halaney <ahalaney@redhat.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/edac/qcom_edac.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 97a27e42dd61..9e77fa84e84f 100644
--- a/drivers/edac/qcom_edac.c
+++ b/drivers/edac/qcom_edac.c
@@ -397,12 +397,19 @@ static int qcom_llcc_edac_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id qcom_llcc_edac_id_table[] = {
+	{ .name = "qcom_llcc_edac" },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, qcom_llcc_edac_id_table);
+
 static struct platform_driver qcom_llcc_edac_driver = {
 	.probe = qcom_llcc_edac_probe,
 	.remove = qcom_llcc_edac_remove,
 	.driver = {
 		.name = "qcom_llcc_edac",
 	},
+	.id_table = qcom_llcc_edac_id_table,
 };
 module_platform_driver(qcom_llcc_edac_driver);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 03/17] EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's pvt_info
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 02/17] EDAC/qcom: Add platform_device_id table for module autoloading Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-20 18:56   ` Borislav Petkov
  2023-01-18 15:08 ` [PATCH v6 04/17] dt-bindings: arm: msm: Update the maintainers for LLCC Manivannan Sadhasivam
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam, stable

The memory for "llcc_driv_data" is allocated by the LLCC driver. But when
it is passed as "pvt_info" to the EDAC core, it will get freed during the
qcom_edac driver release. So when the qcom_edac driver gets probed again,
it will try to use the freed data leading to the use-after-free bug.

Hence, do not pass "llcc_driv_data" as pvt_info but rather reference it
using the "platform_data" in the qcom_edac driver.

Cc: <stable@vger.kernel.org> # 4.20
Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
Reported-by: Steev Klimaszewski <steev@kali.org>
Reviewed-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/edac/qcom_edac.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 9e77fa84e84f..3256254c3722 100644
--- a/drivers/edac/qcom_edac.c
+++ b/drivers/edac/qcom_edac.c
@@ -252,7 +252,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
 static int
 dump_syn_reg(struct edac_device_ctl_info *edev_ctl, int err_type, u32 bank)
 {
-	struct llcc_drv_data *drv = edev_ctl->pvt_info;
+	struct llcc_drv_data *drv = edev_ctl->dev->platform_data;
 	int ret;
 
 	ret = dump_syn_reg_values(drv, bank, err_type);
@@ -289,7 +289,7 @@ static irqreturn_t
 llcc_ecc_irq_handler(int irq, void *edev_ctl)
 {
 	struct edac_device_ctl_info *edac_dev_ctl = edev_ctl;
-	struct llcc_drv_data *drv = edac_dev_ctl->pvt_info;
+	struct llcc_drv_data *drv = edac_dev_ctl->dev->platform_data;
 	irqreturn_t irq_rc = IRQ_NONE;
 	u32 drp_error, trp_error, i;
 	int ret;
@@ -358,7 +358,6 @@ static int qcom_llcc_edac_probe(struct platform_device *pdev)
 	edev_ctl->dev_name = dev_name(dev);
 	edev_ctl->ctl_name = "llcc";
 	edev_ctl->panic_on_ue = LLCC_ERP_PANIC_ON_UE;
-	edev_ctl->pvt_info = llcc_driv_data;
 
 	rc = edac_device_add_device(edev_ctl);
 	if (rc)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 04/17] dt-bindings: arm: msm: Update the maintainers for LLCC
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (2 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 03/17] EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's pvt_info Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 05/17] dt-bindings: arm: msm: Fix register regions used for LLCC banks Manivannan Sadhasivam
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam, Krzysztof Kozlowski

Rishabh Bhatnagar has left Qualcomm, and there is no evidence of him
maintaining with a new identity. So his entry needs to be removed.

Also, Sai Prakash Ranjan's email address should be updated to use
quicinc domain.

Cc: Sai Prakash Ranjan <quic_saipraka@quicinc.com>
Acked-by: Sai Prakash Ranjan <quic_saipraka@quicinc.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
index 38efcad56dbd..d1df49ffcc1b 100644
--- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
@@ -7,8 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: Last Level Cache Controller
 
 maintainers:
-  - Rishabh Bhatnagar <rishabhb@codeaurora.org>
-  - Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
+  - Sai Prakash Ranjan <quic_saipraka@quicinc.com>
 
 description: |
   LLCC (Last Level Cache Controller) provides last level of cache memory in SoC,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 05/17] dt-bindings: arm: msm: Fix register regions used for LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (3 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 04/17] dt-bindings: arm: msm: Update the maintainers for LLCC Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 06/17] arm64: dts: qcom: sdm845: Fix the base addresses of " Manivannan Sadhasivam
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam, Krzysztof Kozlowski

Register regions of the LLCC banks are located at different addresses.
Currently, the binding just lists the LLCC0 base address and tries to
cover all the banks using a single size. This is entirely wrong as there
are other register regions that happen to lie inside the size covered by
the binding such as the memory controller and holes.

So this needs to be fixed by specifying the base address of individual
LLCC banks. This approach will break the existing users of this binding
as the register regions are split and the drivers now cannot use
LLCC0 register region for accessing rest of the banks (which is wrong
anyway).

But considering the fact that the binding was wrong from the day one and
also the device drivers going wrong by the binding, this breakage is
acceptable.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 .../bindings/arm/msm/qcom,llcc.yaml           | 125 ++++++++++++++++--
 1 file changed, 114 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
index d1df49ffcc1b..050e21d4a03e 100644
--- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
@@ -33,14 +33,12 @@ properties:
       - qcom,sm8550-llcc
 
   reg:
-    items:
-      - description: LLCC base register region
-      - description: LLCC broadcast base register region
+    minItems: 2
+    maxItems: 9
 
   reg-names:
-    items:
-      - const: llcc_base
-      - const: llcc_broadcast_base
+    minItems: 2
+    maxItems: 9
 
   interrupts:
     maxItems: 1
@@ -50,15 +48,120 @@ required:
   - reg
   - reg-names
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sc7180-llcc
+              - qcom,sm6350-llcc
+    then:
+      properties:
+        reg:
+          items:
+            - description: LLCC0 base register region
+            - description: LLCC broadcast base register region
+        reg-names:
+          items:
+            - const: llcc0_base
+            - const: llcc_broadcast_base
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sc7280-llcc
+    then:
+      properties:
+        reg:
+          items:
+            - description: LLCC0 base register region
+            - description: LLCC1 base register region
+            - description: LLCC broadcast base register region
+        reg-names:
+          items:
+            - const: llcc0_base
+            - const: llcc1_base
+            - const: llcc_broadcast_base
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sc8180x-llcc
+              - qcom,sc8280xp-llcc
+    then:
+      properties:
+        reg:
+          items:
+            - description: LLCC0 base register region
+            - description: LLCC1 base register region
+            - description: LLCC2 base register region
+            - description: LLCC3 base register region
+            - description: LLCC4 base register region
+            - description: LLCC5 base register region
+            - description: LLCC6 base register region
+            - description: LLCC7 base register region
+            - description: LLCC broadcast base register region
+        reg-names:
+          items:
+            - const: llcc0_base
+            - const: llcc1_base
+            - const: llcc2_base
+            - const: llcc3_base
+            - const: llcc4_base
+            - const: llcc5_base
+            - const: llcc6_base
+            - const: llcc7_base
+            - const: llcc_broadcast_base
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sdm845-llcc
+              - qcom,sm8150-llcc
+              - qcom,sm8250-llcc
+              - qcom,sm8350-llcc
+              - qcom,sm8450-llcc
+    then:
+      properties:
+        reg:
+          items:
+            - description: LLCC0 base register region
+            - description: LLCC1 base register region
+            - description: LLCC2 base register region
+            - description: LLCC3 base register region
+            - description: LLCC broadcast base register region
+        reg-names:
+          items:
+            - const: llcc0_base
+            - const: llcc1_base
+            - const: llcc2_base
+            - const: llcc3_base
+            - const: llcc_broadcast_base
+
 additionalProperties: false
 
 examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
 
-    system-cache-controller@1100000 {
-      compatible = "qcom,sdm845-llcc";
-      reg = <0x1100000 0x200000>, <0x1300000 0x50000> ;
-      reg-names = "llcc_base", "llcc_broadcast_base";
-      interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        system-cache-controller@1100000 {
+            compatible = "qcom,sdm845-llcc";
+            reg = <0 0x01100000 0 0x50000>, <0 0x01180000 0 0x50000>,
+                <0 0x01200000 0 0x50000>, <0 0x01280000 0 0x50000>,
+                <0 0x01300000 0 0x50000>;
+            reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+                "llcc3_base", "llcc_broadcast_base";
+            interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
+        };
     };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 06/17] arm64: dts: qcom: sdm845: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (4 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 05/17] dt-bindings: arm: msm: Fix register regions used for LLCC banks Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 07/17] arm64: dts: qcom: sc7180: " Manivannan Sadhasivam
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

On SDM845, the size of the LLCC bank 0 needs to be reduced to 0x4500 as
there are LLCC BWMON registers located after this range.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 65032b94b46d..4db68d4d78df 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2132,8 +2132,11 @@ uart15: serial@a9c000 {
 
 		llcc: system-cache-controller@1100000 {
 			compatible = "qcom,sdm845-llcc";
-			reg = <0 0x01100000 0 0x31000>, <0 0x01300000 0 0x50000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg = <0 0x01100000 0 0x45000>, <0 0x01180000 0 0x50000>,
+			      <0 0x01200000 0 0x50000>, <0 0x01280000 0 0x50000>,
+			      <0 0x01300000 0 0x50000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+				    "llcc3_base", "llcc_broadcast_base";
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 07/17] arm64: dts: qcom: sc7180: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (5 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 06/17] arm64: dts: qcom: sdm845: Fix the base addresses of " Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 08/17] arm64: dts: qcom: sc7280: " Manivannan Sadhasivam
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

On SC7180, there is only one LLCC bank available. So let's just pass that
as "llcc0_base".

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index f71cf21a8dd8..f861f692c9b1 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2759,7 +2759,7 @@ dc_noc: interconnect@9160000 {
 		system-cache-controller@9200000 {
 			compatible = "qcom,sc7180-llcc";
 			reg = <0 0x09200000 0 0x50000>, <0 0x09600000 0 0x50000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg-names = "llcc0_base", "llcc_broadcast_base";
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 08/17] arm64: dts: qcom: sc7280: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (6 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 07/17] arm64: dts: qcom: sc7180: " Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 09/17] arm64: dts: qcom: sc8280xp: " Manivannan Sadhasivam
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

While at it, let's also fix the size of the llcc_broadcast_base to cover
the whole region.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 0adf13399e64..6c6eb6f4f650 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3579,8 +3579,9 @@ gem_noc: interconnect@9100000 {
 
 		system-cache-controller@9200000 {
 			compatible = "qcom,sc7280-llcc";
-			reg = <0 0x09200000 0 0xd0000>, <0 0x09600000 0 0x50000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg = <0 0x09200000 0 0x58000>, <0 0x09280000 0 0x58000>,
+			      <0 0x09600000 0 0x58000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc_broadcast_base";
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 09/17] arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (7 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 08/17] arm64: dts: qcom: sc7280: " Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 10/17] arm64: dts: qcom: sm8150: " Manivannan Sadhasivam
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 109c9d2b684d..0510a5d510e7 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1856,8 +1856,14 @@ opp-6 {
 
 		system-cache-controller@9200000 {
 			compatible = "qcom,sc8280xp-llcc";
-			reg = <0 0x09200000 0 0x58000>, <0 0x09600000 0 0x58000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg = <0 0x09200000 0 0x58000>, <0 0x09280000 0 0x58000>,
+			      <0 0x09300000 0 0x58000>, <0 0x09380000 0 0x58000>,
+			      <0 0x09400000 0 0x58000>, <0 0x09480000 0 0x58000>,
+			      <0 0x09500000 0 0x58000>, <0 0x09580000 0 0x58000>,
+			      <0 0x09600000 0 0x58000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+				    "llcc3_base", "llcc4_base", "llcc5_base",
+				    "llcc6_base", "llcc7_base",  "llcc_broadcast_base";
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 10/17] arm64: dts: qcom: sm8150: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (8 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 09/17] arm64: dts: qcom: sc8280xp: " Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 11/17] arm64: dts: qcom: sm8250: " Manivannan Sadhasivam
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index a0c57fb798d3..7fd2291b2638 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -1762,8 +1762,11 @@ mmss_noc: interconnect@1740000 {
 
 		system-cache-controller@9200000 {
 			compatible = "qcom,sm8150-llcc";
-			reg = <0 0x09200000 0 0x200000>, <0 0x09600000 0 0x50000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg = <0 0x09200000 0 0x50000>, <0 0x09280000 0 0x50000>,
+			      <0 0x09300000 0 0x50000>, <0 0x09380000 0 0x50000>,
+			      <0 0x09600000 0 0x50000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+				    "llcc3_base", "llcc_broadcast_base";
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 11/17] arm64: dts: qcom: sm8250: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (9 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 10/17] arm64: dts: qcom: sm8150: " Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:08 ` [PATCH v6 12/17] arm64: dts: qcom: sm8350: " Manivannan Sadhasivam
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index dab5579946f3..d1b65fb3f3f3 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3545,8 +3545,11 @@ usb_1_dwc3: usb@a600000 {
 
 		system-cache-controller@9200000 {
 			compatible = "qcom,sm8250-llcc";
-			reg = <0 0x09200000 0 0x1d0000>, <0 0x09600000 0 0x50000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg = <0 0x09200000 0 0x50000>, <0 0x09280000 0 0x50000>,
+			      <0 0x09300000 0 0x50000>, <0 0x09380000 0 0x50000>,
+			      <0 0x09600000 0 0x50000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+				    "llcc3_base", "llcc_broadcast_base";
 		};
 
 		usb_2: usb@a8f8800 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 12/17] arm64: dts: qcom: sm8350: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (10 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 11/17] arm64: dts: qcom: sm8250: " Manivannan Sadhasivam
@ 2023-01-18 15:08 ` Manivannan Sadhasivam
  2023-01-18 15:09 ` [PATCH v6 13/17] arm64: dts: qcom: sm8450: " Manivannan Sadhasivam
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:08 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index 245dce24ec59..836732d16635 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -2513,8 +2513,11 @@ gem_noc: interconnect@9100000 {
 
 		system-cache-controller@9200000 {
 			compatible = "qcom,sm8350-llcc";
-			reg = <0 0x09200000 0 0x1d0000>, <0 0x09600000 0 0x50000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg = <0 0x09200000 0 0x58000>, <0 0x09280000 0 0x58000>,
+			      <0 0x09300000 0 0x58000>, <0 0x09380000 0 0x58000>,
+			      <0 0x09600000 0 0x58000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+				    "llcc3_base", "llcc_broadcast_base";
 		};
 
 		usb_1: usb@a6f8800 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 13/17] arm64: dts: qcom: sm8450: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (11 preceding siblings ...)
  2023-01-18 15:08 ` [PATCH v6 12/17] arm64: dts: qcom: sm8350: " Manivannan Sadhasivam
@ 2023-01-18 15:09 ` Manivannan Sadhasivam
  2023-01-18 15:09 ` [PATCH v6 14/17] arm64: dts: qcom: sm6350: " Manivannan Sadhasivam
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:09 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 570475040d95..12549a2912c6 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -3640,8 +3640,11 @@ gem_noc: interconnect@19100000 {
 
 		system-cache-controller@19200000 {
 			compatible = "qcom,sm8450-llcc";
-			reg = <0 0x19200000 0 0x580000>, <0 0x19a00000 0 0x80000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg = <0 0x19200000 0 0x80000>, <0 0x19600000 0 0x80000>,
+			      <0 0x19300000 0 0x80000>, <0 0x19700000 0 0x80000>,
+			      <0 0x19a00000 0 0x80000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+				    "llcc3_base", "llcc_broadcast_base";
 			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 14/17] arm64: dts: qcom: sm6350: Fix the base addresses of LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (12 preceding siblings ...)
  2023-01-18 15:09 ` [PATCH v6 13/17] arm64: dts: qcom: sm8450: " Manivannan Sadhasivam
@ 2023-01-18 15:09 ` Manivannan Sadhasivam
  2023-01-18 15:09 ` [PATCH v6 15/17] qcom: llcc/edac: Fix the base address used for accessing " Manivannan Sadhasivam
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:09 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

On SM6350, there is only one LLCC bank available. So let's just pass that
as "llcc0_base".

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm6350.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
index 43324bf291c3..c7701f5e4af6 100644
--- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
@@ -1174,7 +1174,7 @@ dc_noc: interconnect@9160000 {
 		system-cache-controller@9200000 {
 			compatible = "qcom,sm6350-llcc";
 			reg = <0 0x09200000 0 0x50000>, <0 0x09600000 0 0x50000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg-names = "llcc0_base", "llcc_broadcast_base";
 		};
 
 		gem_noc: interconnect@9680000 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 15/17] qcom: llcc/edac: Fix the base address used for accessing LLCC banks
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (13 preceding siblings ...)
  2023-01-18 15:09 ` [PATCH v6 14/17] arm64: dts: qcom: sm6350: " Manivannan Sadhasivam
@ 2023-01-18 15:09 ` Manivannan Sadhasivam
  2023-01-18 15:09 ` [PATCH v6 16/17] qcom: llcc/edac: Support polling mode for ECC handling Manivannan Sadhasivam
  2023-01-18 15:09 ` [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845 Manivannan Sadhasivam
  16 siblings, 0 replies; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:09 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

The Qualcomm LLCC/EDAC drivers were using a fixed register stride for
accessing the (Control and Status Registers) CSRs of each LLCC bank.
This stride only works for some SoCs like SDM845 for which driver
support was initially added.

But the later SoCs use different register stride that vary between the
banks with holes in-between. So it is not possible to use a single register
stride for accessing the CSRs of each bank. By doing so could result in a
crash.

For fixing this issue, let's obtain the base address of each LLCC bank from
devicetree and get rid of the fixed stride. This also means, there is no
need to rely on reg-names property and the base addresses can be obtained
using the index.

First index is LLCC bank 0 and last index is LLCC broadcast. If the SoC
supports more than one bank, then those need to be defined in devicetree
for index from 1..N-1.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com>
Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
Reviewed-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/edac/qcom_edac.c           | 14 +++---
 drivers/soc/qcom/llcc-qcom.c       | 72 +++++++++++++++++-------------
 include/linux/soc/qcom/llcc-qcom.h |  6 +--
 3 files changed, 48 insertions(+), 44 deletions(-)

diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 3256254c3722..1d3cc1930a74 100644
--- a/drivers/edac/qcom_edac.c
+++ b/drivers/edac/qcom_edac.c
@@ -213,7 +213,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
 
 	for (i = 0; i < reg_data.reg_cnt; i++) {
 		synd_reg = reg_data.synd_reg + (i * 4);
-		ret = regmap_read(drv->regmap, drv->offsets[bank] + synd_reg,
+		ret = regmap_read(drv->regmaps[bank], synd_reg,
 				  &synd_val);
 		if (ret)
 			goto clear;
@@ -222,8 +222,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
 			    reg_data.name, i, synd_val);
 	}
 
-	ret = regmap_read(drv->regmap,
-			  drv->offsets[bank] + reg_data.count_status_reg,
+	ret = regmap_read(drv->regmaps[bank], reg_data.count_status_reg,
 			  &err_cnt);
 	if (ret)
 		goto clear;
@@ -233,8 +232,7 @@ dump_syn_reg_values(struct llcc_drv_data *drv, u32 bank, int err_type)
 	edac_printk(KERN_CRIT, EDAC_LLCC, "%s: Error count: 0x%4x\n",
 		    reg_data.name, err_cnt);
 
-	ret = regmap_read(drv->regmap,
-			  drv->offsets[bank] + reg_data.ways_status_reg,
+	ret = regmap_read(drv->regmaps[bank], reg_data.ways_status_reg,
 			  &err_ways);
 	if (ret)
 		goto clear;
@@ -296,8 +294,7 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
 
 	/* Iterate over the banks and look for Tag RAM or Data RAM errors */
 	for (i = 0; i < drv->num_banks; i++) {
-		ret = regmap_read(drv->regmap,
-				  drv->offsets[i] + DRP_INTERRUPT_STATUS,
+		ret = regmap_read(drv->regmaps[i], DRP_INTERRUPT_STATUS,
 				  &drp_error);
 
 		if (!ret && (drp_error & SB_ECC_ERROR)) {
@@ -312,8 +309,7 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
 		if (!ret)
 			irq_rc = IRQ_HANDLED;
 
-		ret = regmap_read(drv->regmap,
-				  drv->offsets[i] + TRP_INTERRUPT_0_STATUS,
+		ret = regmap_read(drv->regmaps[i], TRP_INTERRUPT_0_STATUS,
 				  &trp_error);
 
 		if (!ret && (trp_error & SB_ECC_ERROR)) {
diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 23ce2f78c4ed..72f3f2a9aaa0 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -62,8 +62,6 @@
 #define LLCC_TRP_WRSC_CACHEABLE_EN    0x21f2c
 #define LLCC_TRP_ALGO_CFG8	      0x21f30
 
-#define BANK_OFFSET_STRIDE	      0x80000
-
 #define LLCC_VERSION_2_0_0_0          0x02000000
 #define LLCC_VERSION_2_1_0_0          0x02010000
 #define LLCC_VERSION_4_1_0_0          0x04010000
@@ -898,8 +896,8 @@ static int qcom_llcc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
-		const char *name)
+static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev, u8 index,
+					  const char *name)
 {
 	void __iomem *base;
 	struct regmap_config llcc_regmap_config = {
@@ -909,7 +907,7 @@ static struct regmap *qcom_llcc_init_mmio(struct platform_device *pdev,
 		.fast_io = true,
 	};
 
-	base = devm_platform_ioremap_resource_byname(pdev, name);
+	base = devm_platform_ioremap_resource(pdev, index);
 	if (IS_ERR(base))
 		return ERR_CAST(base);
 
@@ -927,6 +925,7 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 	const struct llcc_slice_config *llcc_cfg;
 	u32 sz;
 	u32 version;
+	struct regmap *regmap;
 
 	drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
 	if (!drv_data) {
@@ -934,21 +933,51 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	drv_data->regmap = qcom_llcc_init_mmio(pdev, "llcc_base");
-	if (IS_ERR(drv_data->regmap)) {
-		ret = PTR_ERR(drv_data->regmap);
+	/* Initialize the first LLCC bank regmap */
+	regmap = qcom_llcc_init_mmio(pdev, 0, "llcc0_base");
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
 		goto err;
 	}
 
-	drv_data->bcast_regmap =
-		qcom_llcc_init_mmio(pdev, "llcc_broadcast_base");
+	cfg = of_device_get_match_data(&pdev->dev);
+
+	ret = regmap_read(regmap, cfg->reg_offset[LLCC_COMMON_STATUS0], &num_banks);
+	if (ret)
+		goto err;
+
+	num_banks &= LLCC_LB_CNT_MASK;
+	num_banks >>= LLCC_LB_CNT_SHIFT;
+	drv_data->num_banks = num_banks;
+
+	drv_data->regmaps = devm_kcalloc(dev, num_banks, sizeof(*drv_data->regmaps), GFP_KERNEL);
+	if (!drv_data->regmaps) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	drv_data->regmaps[0] = regmap;
+
+	/* Initialize rest of LLCC bank regmaps */
+	for (i = 1; i < num_banks; i++) {
+		char *base = kasprintf(GFP_KERNEL, "llcc%d_base", i);
+
+		drv_data->regmaps[i] = qcom_llcc_init_mmio(pdev, i, base);
+		if (IS_ERR(drv_data->regmaps[i])) {
+			ret = PTR_ERR(drv_data->regmaps[i]);
+			kfree(base);
+			goto err;
+		}
+
+		kfree(base);
+	}
+
+	drv_data->bcast_regmap = qcom_llcc_init_mmio(pdev, i, "llcc_broadcast_base");
 	if (IS_ERR(drv_data->bcast_regmap)) {
 		ret = PTR_ERR(drv_data->bcast_regmap);
 		goto err;
 	}
 
-	cfg = of_device_get_match_data(&pdev->dev);
-
 	/* Extract version of the IP */
 	ret = regmap_read(drv_data->bcast_regmap, cfg->reg_offset[LLCC_COMMON_HW_INFO],
 			  &version);
@@ -957,15 +986,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 
 	drv_data->version = version;
 
-	ret = regmap_read(drv_data->regmap, cfg->reg_offset[LLCC_COMMON_STATUS0],
-			  &num_banks);
-	if (ret)
-		goto err;
-
-	num_banks &= LLCC_LB_CNT_MASK;
-	num_banks >>= LLCC_LB_CNT_SHIFT;
-	drv_data->num_banks = num_banks;
-
 	llcc_cfg = cfg->sct_data;
 	sz = cfg->size;
 
@@ -973,16 +993,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 		if (llcc_cfg[i].slice_id > drv_data->max_slices)
 			drv_data->max_slices = llcc_cfg[i].slice_id;
 
-	drv_data->offsets = devm_kcalloc(dev, num_banks, sizeof(u32),
-							GFP_KERNEL);
-	if (!drv_data->offsets) {
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	for (i = 0; i < num_banks; i++)
-		drv_data->offsets[i] = i * BANK_OFFSET_STRIDE;
-
 	drv_data->bitmap = devm_bitmap_zalloc(dev, drv_data->max_slices,
 					      GFP_KERNEL);
 	if (!drv_data->bitmap) {
diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
index ad1fd718169d..423220e66026 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -120,7 +120,7 @@ struct llcc_edac_reg_offset {
 
 /**
  * struct llcc_drv_data - Data associated with the llcc driver
- * @regmap: regmap associated with the llcc device
+ * @regmaps: regmaps associated with the llcc device
  * @bcast_regmap: regmap associated with llcc broadcast offset
  * @cfg: pointer to the data structure for slice configuration
  * @edac_reg_offset: Offset of the LLCC EDAC registers
@@ -129,12 +129,11 @@ struct llcc_edac_reg_offset {
  * @max_slices: max slices as read from device tree
  * @num_banks: Number of llcc banks
  * @bitmap: Bit map to track the active slice ids
- * @offsets: Pointer to the bank offsets array
  * @ecc_irq: interrupt for llcc cache error detection and reporting
  * @version: Indicates the LLCC version
  */
 struct llcc_drv_data {
-	struct regmap *regmap;
+	struct regmap **regmaps;
 	struct regmap *bcast_regmap;
 	const struct llcc_slice_config *cfg;
 	const struct llcc_edac_reg_offset *edac_reg_offset;
@@ -143,7 +142,6 @@ struct llcc_drv_data {
 	u32 max_slices;
 	u32 num_banks;
 	unsigned long *bitmap;
-	u32 *offsets;
 	int ecc_irq;
 	u32 version;
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 16/17] qcom: llcc/edac: Support polling mode for ECC handling
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (14 preceding siblings ...)
  2023-01-18 15:09 ` [PATCH v6 15/17] qcom: llcc/edac: Fix the base address used for accessing " Manivannan Sadhasivam
@ 2023-01-18 15:09 ` Manivannan Sadhasivam
  2023-01-20 19:04   ` Borislav Petkov
  2023-01-18 15:09 ` [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845 Manivannan Sadhasivam
  16 siblings, 1 reply; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:09 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam

Not all Qcom platforms support IRQ mode for ECC handling. For those
platforms, the current EDAC driver will not be probed due to missing ECC
IRQ in devicetree.

So add support for polling mode so that the EDAC driver can be used on all
Qcom platforms supporting LLCC.

The polling delay of 5000ms is chosen based on Qcom downstream/vendor
driver.

Reported-by: Luca Weiss <luca.weiss@fairphone.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com>
Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/edac/qcom_edac.c     | 50 +++++++++++++++++++++---------------
 drivers/soc/qcom/llcc-qcom.c | 13 +++++-----
 2 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 1d3cc1930a74..265e0fb39bc7 100644
--- a/drivers/edac/qcom_edac.c
+++ b/drivers/edac/qcom_edac.c
@@ -76,6 +76,8 @@
 #define DRP0_INTERRUPT_ENABLE           BIT(6)
 #define SB_DB_DRP_INTERRUPT_ENABLE      0x3
 
+#define ECC_POLL_MSEC			5000
+
 enum {
 	LLCC_DRAM_CE = 0,
 	LLCC_DRAM_UE,
@@ -283,8 +285,7 @@ dump_syn_reg(struct edac_device_ctl_info *edev_ctl, int err_type, u32 bank)
 	return ret;
 }
 
-static irqreturn_t
-llcc_ecc_irq_handler(int irq, void *edev_ctl)
+static irqreturn_t llcc_ecc_irq_handler(int irq, void *edev_ctl)
 {
 	struct edac_device_ctl_info *edac_dev_ctl = edev_ctl;
 	struct llcc_drv_data *drv = edac_dev_ctl->dev->platform_data;
@@ -328,6 +329,11 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
 	return irq_rc;
 }
 
+static void llcc_ecc_check(struct edac_device_ctl_info *edev_ctl)
+{
+	llcc_ecc_irq_handler(0, edev_ctl);
+}
+
 static int qcom_llcc_edac_probe(struct platform_device *pdev)
 {
 	struct llcc_drv_data *llcc_driv_data = pdev->dev.platform_data;
@@ -355,29 +361,31 @@ static int qcom_llcc_edac_probe(struct platform_device *pdev)
 	edev_ctl->ctl_name = "llcc";
 	edev_ctl->panic_on_ue = LLCC_ERP_PANIC_ON_UE;
 
-	rc = edac_device_add_device(edev_ctl);
-	if (rc)
-		goto out_mem;
-
-	platform_set_drvdata(pdev, edev_ctl);
-
-	/* Request for ecc irq */
+	/* Check if LLCC driver has passed ECC IRQ */
 	ecc_irq = llcc_driv_data->ecc_irq;
-	if (ecc_irq < 0) {
-		rc = -ENODEV;
-		goto out_dev;
-	}
-	rc = devm_request_irq(dev, ecc_irq, llcc_ecc_irq_handler,
+	if (ecc_irq > 0) {
+		/* Use interrupt mode if IRQ is available */
+		rc = devm_request_irq(dev, ecc_irq, llcc_ecc_irq_handler,
 			      IRQF_TRIGGER_HIGH, "llcc_ecc", edev_ctl);
-	if (rc)
-		goto out_dev;
+		if (!rc) {
+			edac_op_state = EDAC_OPSTATE_INT;
+			goto irq_done;
+		}
+	}
 
-	return rc;
+	/* Fall back to polling mode otherwise */
+	edev_ctl->poll_msec = ECC_POLL_MSEC;
+	edev_ctl->edac_check = llcc_ecc_check;
+	edac_op_state = EDAC_OPSTATE_POLL;
 
-out_dev:
-	edac_device_del_device(edev_ctl->dev);
-out_mem:
-	edac_device_free_ctl_info(edev_ctl);
+irq_done:
+	rc = edac_device_add_device(edev_ctl);
+	if (rc) {
+		edac_device_free_ctl_info(edev_ctl);
+		return rc;
+	}
+
+	platform_set_drvdata(pdev, edev_ctl);
 
 	return rc;
 }
diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 72f3f2a9aaa0..7b7c5a38bac6 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -1011,13 +1011,12 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 		goto err;
 
 	drv_data->ecc_irq = platform_get_irq_optional(pdev, 0);
-	if (drv_data->ecc_irq >= 0) {
-		llcc_edac = platform_device_register_data(&pdev->dev,
-						"qcom_llcc_edac", -1, drv_data,
-						sizeof(*drv_data));
-		if (IS_ERR(llcc_edac))
-			dev_err(dev, "Failed to register llcc edac driver\n");
-	}
+
+	llcc_edac = platform_device_register_data(&pdev->dev,
+					"qcom_llcc_edac", -1, drv_data,
+					sizeof(*drv_data));
+	if (IS_ERR(llcc_edac))
+		dev_err(dev, "Failed to register llcc edac driver\n");
 
 	return 0;
 err:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845
  2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
                   ` (15 preceding siblings ...)
  2023-01-18 15:09 ` [PATCH v6 16/17] qcom: llcc/edac: Support polling mode for ECC handling Manivannan Sadhasivam
@ 2023-01-18 15:09 ` Manivannan Sadhasivam
  2023-01-18 15:37   ` Krzysztof Kozlowski
  16 siblings, 1 reply; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:09 UTC (permalink / raw)
  To: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, Manivannan Sadhasivam, stable

The platforms based on SDM845 SoC locks the access to EDAC registers in the
bootloader. So probing the EDAC driver will result in a crash. Hence,
disable the creation of EDAC platform device on all SDM845 devices.

The issue has been observed on Lenovo Yoga C630 and DB845c.

Cc: <stable@vger.kernel.org> # 5.10
Reported-by: Steev Klimaszewski <steev@kali.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/soc/qcom/llcc-qcom.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 7b7c5a38bac6..8d840702df50 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -1012,11 +1012,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
 
 	drv_data->ecc_irq = platform_get_irq_optional(pdev, 0);
 
-	llcc_edac = platform_device_register_data(&pdev->dev,
-					"qcom_llcc_edac", -1, drv_data,
-					sizeof(*drv_data));
-	if (IS_ERR(llcc_edac))
-		dev_err(dev, "Failed to register llcc edac driver\n");
+	/*
+	 * The platforms based on SDM845 SoC locks the access to EDAC registers
+	 * in bootloader. So probing the EDAC driver will result in a crash.
+	 * Hence, disable the creation of EDAC platform device on SDM845.
+	 */
+	if (!of_device_is_compatible(dev->of_node, "qcom,sdm845-llcc")) {
+		llcc_edac = platform_device_register_data(&pdev->dev,
+						"qcom_llcc_edac", -1, drv_data,
+						sizeof(*drv_data));
+		if (IS_ERR(llcc_edac))
+			dev_err(dev, "Failed to register llcc edac driver\n");
+	}
 
 	return 0;
 err:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845
  2023-01-18 15:09 ` [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845 Manivannan Sadhasivam
@ 2023-01-18 15:37   ` Krzysztof Kozlowski
  2023-01-18 15:59     ` Manivannan Sadhasivam
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-18 15:37 UTC (permalink / raw)
  To: Manivannan Sadhasivam, andersson, robh+dt,
	krzysztof.kozlowski+dt, bp, tony.luck
  Cc: quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, stable

On 18/01/2023 16:09, Manivannan Sadhasivam wrote:
> The platforms based on SDM845 SoC locks the access to EDAC registers in the
> bootloader. So probing the EDAC driver will result in a crash. Hence,
> disable the creation of EDAC platform device on all SDM845 devices.
> 
> The issue has been observed on Lenovo Yoga C630 and DB845c.
> 
> Cc: <stable@vger.kernel.org> # 5.10
> Reported-by: Steev Klimaszewski <steev@kali.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/soc/qcom/llcc-qcom.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 7b7c5a38bac6..8d840702df50 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -1012,11 +1012,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>  
>  	drv_data->ecc_irq = platform_get_irq_optional(pdev, 0);
>  
> -	llcc_edac = platform_device_register_data(&pdev->dev,
> -					"qcom_llcc_edac", -1, drv_data,
> -					sizeof(*drv_data));
> -	if (IS_ERR(llcc_edac))
> -		dev_err(dev, "Failed to register llcc edac driver\n");
> +	/*
> +	 * The platforms based on SDM845 SoC locks the access to EDAC registers
> +	 * in bootloader. So probing the EDAC driver will result in a crash.
> +	 * Hence, disable the creation of EDAC platform device on SDM845.
> +	 */
> +	if (!of_device_is_compatible(dev->of_node, "qcom,sdm845-llcc")) {

Don't spread of_device_is_compatible() in driver code. You have driver
data for this.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845
  2023-01-18 15:37   ` Krzysztof Kozlowski
@ 2023-01-18 15:59     ` Manivannan Sadhasivam
  2023-01-18 16:05       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 15:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck,
	quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, stable

On Wed, Jan 18, 2023 at 04:37:29PM +0100, Krzysztof Kozlowski wrote:
> On 18/01/2023 16:09, Manivannan Sadhasivam wrote:
> > The platforms based on SDM845 SoC locks the access to EDAC registers in the
> > bootloader. So probing the EDAC driver will result in a crash. Hence,
> > disable the creation of EDAC platform device on all SDM845 devices.
> > 
> > The issue has been observed on Lenovo Yoga C630 and DB845c.
> > 
> > Cc: <stable@vger.kernel.org> # 5.10
> > Reported-by: Steev Klimaszewski <steev@kali.org>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/soc/qcom/llcc-qcom.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> > index 7b7c5a38bac6..8d840702df50 100644
> > --- a/drivers/soc/qcom/llcc-qcom.c
> > +++ b/drivers/soc/qcom/llcc-qcom.c
> > @@ -1012,11 +1012,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> >  
> >  	drv_data->ecc_irq = platform_get_irq_optional(pdev, 0);
> >  
> > -	llcc_edac = platform_device_register_data(&pdev->dev,
> > -					"qcom_llcc_edac", -1, drv_data,
> > -					sizeof(*drv_data));
> > -	if (IS_ERR(llcc_edac))
> > -		dev_err(dev, "Failed to register llcc edac driver\n");
> > +	/*
> > +	 * The platforms based on SDM845 SoC locks the access to EDAC registers
> > +	 * in bootloader. So probing the EDAC driver will result in a crash.
> > +	 * Hence, disable the creation of EDAC platform device on SDM845.
> > +	 */
> > +	if (!of_device_is_compatible(dev->of_node, "qcom,sdm845-llcc")) {
> 
> Don't spread of_device_is_compatible() in driver code. You have driver
> data for this.
> 

Yeah, but there is no ID to in the driver data to identify an SoC. I could add
one but is that really worth doing so? Is using of_device_is_compatible() in
drivers discouraged nowadays?

Thanks,
Mani

> Best regards,
> Krzysztof
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845
  2023-01-18 15:59     ` Manivannan Sadhasivam
@ 2023-01-18 16:05       ` Krzysztof Kozlowski
  2023-01-18 16:26         ` Manivannan Sadhasivam
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-18 16:05 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck,
	quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, stable

On 18/01/2023 16:59, Manivannan Sadhasivam wrote:
> On Wed, Jan 18, 2023 at 04:37:29PM +0100, Krzysztof Kozlowski wrote:
>> On 18/01/2023 16:09, Manivannan Sadhasivam wrote:
>>> The platforms based on SDM845 SoC locks the access to EDAC registers in the
>>> bootloader. So probing the EDAC driver will result in a crash. Hence,
>>> disable the creation of EDAC platform device on all SDM845 devices.
>>>
>>> The issue has been observed on Lenovo Yoga C630 and DB845c.
>>>
>>> Cc: <stable@vger.kernel.org> # 5.10
>>> Reported-by: Steev Klimaszewski <steev@kali.org>
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>> ---
>>>  drivers/soc/qcom/llcc-qcom.c | 17 ++++++++++++-----
>>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
>>> index 7b7c5a38bac6..8d840702df50 100644
>>> --- a/drivers/soc/qcom/llcc-qcom.c
>>> +++ b/drivers/soc/qcom/llcc-qcom.c
>>> @@ -1012,11 +1012,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>>>  
>>>  	drv_data->ecc_irq = platform_get_irq_optional(pdev, 0);
>>>  
>>> -	llcc_edac = platform_device_register_data(&pdev->dev,
>>> -					"qcom_llcc_edac", -1, drv_data,
>>> -					sizeof(*drv_data));
>>> -	if (IS_ERR(llcc_edac))
>>> -		dev_err(dev, "Failed to register llcc edac driver\n");
>>> +	/*
>>> +	 * The platforms based on SDM845 SoC locks the access to EDAC registers
>>> +	 * in bootloader. So probing the EDAC driver will result in a crash.
>>> +	 * Hence, disable the creation of EDAC platform device on SDM845.
>>> +	 */
>>> +	if (!of_device_is_compatible(dev->of_node, "qcom,sdm845-llcc")) {
>>
>> Don't spread of_device_is_compatible() in driver code. You have driver
>> data for this.
>>
> 
> Yeah, but there is no ID to in the driver data to identify an SoC. 

What do you mean there is no? You use exactly the same compatible as the
one in driver data.


> I could add
> one but is that really worth doing so? Is using of_device_is_compatible() in
> drivers discouraged nowadays?

Because it spreads variant matching all over. It does not scale. drv
data fields are the way or better quirks/flags.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845
  2023-01-18 16:05       ` Krzysztof Kozlowski
@ 2023-01-18 16:26         ` Manivannan Sadhasivam
  2023-01-18 17:06           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: Manivannan Sadhasivam @ 2023-01-18 16:26 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck,
	quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, stable

On Wed, Jan 18, 2023 at 05:05:28PM +0100, Krzysztof Kozlowski wrote:
> On 18/01/2023 16:59, Manivannan Sadhasivam wrote:
> > On Wed, Jan 18, 2023 at 04:37:29PM +0100, Krzysztof Kozlowski wrote:
> >> On 18/01/2023 16:09, Manivannan Sadhasivam wrote:
> >>> The platforms based on SDM845 SoC locks the access to EDAC registers in the
> >>> bootloader. So probing the EDAC driver will result in a crash. Hence,
> >>> disable the creation of EDAC platform device on all SDM845 devices.
> >>>
> >>> The issue has been observed on Lenovo Yoga C630 and DB845c.
> >>>
> >>> Cc: <stable@vger.kernel.org> # 5.10
> >>> Reported-by: Steev Klimaszewski <steev@kali.org>
> >>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >>> ---
> >>>  drivers/soc/qcom/llcc-qcom.c | 17 ++++++++++++-----
> >>>  1 file changed, 12 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> >>> index 7b7c5a38bac6..8d840702df50 100644
> >>> --- a/drivers/soc/qcom/llcc-qcom.c
> >>> +++ b/drivers/soc/qcom/llcc-qcom.c
> >>> @@ -1012,11 +1012,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> >>>  
> >>>  	drv_data->ecc_irq = platform_get_irq_optional(pdev, 0);
> >>>  
> >>> -	llcc_edac = platform_device_register_data(&pdev->dev,
> >>> -					"qcom_llcc_edac", -1, drv_data,
> >>> -					sizeof(*drv_data));
> >>> -	if (IS_ERR(llcc_edac))
> >>> -		dev_err(dev, "Failed to register llcc edac driver\n");
> >>> +	/*
> >>> +	 * The platforms based on SDM845 SoC locks the access to EDAC registers
> >>> +	 * in bootloader. So probing the EDAC driver will result in a crash.
> >>> +	 * Hence, disable the creation of EDAC platform device on SDM845.
> >>> +	 */
> >>> +	if (!of_device_is_compatible(dev->of_node, "qcom,sdm845-llcc")) {
> >>
> >> Don't spread of_device_is_compatible() in driver code. You have driver
> >> data for this.
> >>
> > 
> > Yeah, but there is no ID to in the driver data to identify an SoC. 
> 
> What do you mean there is no? You use exactly the same compatible as the
> one in driver data.
> 

Right, but I was saying that there is no unique field to identify an SoC.

> 
> > I could add
> > one but is that really worth doing so? Is using of_device_is_compatible() in
> > drivers discouraged nowadays?
> 
> Because it spreads variant matching all over. It does not scale. drv
> data fields are the way or better quirks/flags.
> 

The driver quirk/flags are usually beneficial if it applies to multiple
platforms, otherwise they are a bit overkill IMO just like in this case.

One can argue that this matching could spread to other SoCs in the future, but
I don't think that could happen for this case.

Thanks,
Mani

> Best regards,
> Krzysztof
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845
  2023-01-18 16:26         ` Manivannan Sadhasivam
@ 2023-01-18 17:06           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-18 17:06 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, bp, tony.luck,
	quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, stable

On 18/01/2023 17:26, Manivannan Sadhasivam wrote:
> On Wed, Jan 18, 2023 at 05:05:28PM +0100, Krzysztof Kozlowski wrote:
>> On 18/01/2023 16:59, Manivannan Sadhasivam wrote:
>>> On Wed, Jan 18, 2023 at 04:37:29PM +0100, Krzysztof Kozlowski wrote:
>>>> On 18/01/2023 16:09, Manivannan Sadhasivam wrote:
>>>>> The platforms based on SDM845 SoC locks the access to EDAC registers in the
>>>>> bootloader. So probing the EDAC driver will result in a crash. Hence,
>>>>> disable the creation of EDAC platform device on all SDM845 devices.
>>>>>
>>>>> The issue has been observed on Lenovo Yoga C630 and DB845c.
>>>>>
>>>>> Cc: <stable@vger.kernel.org> # 5.10
>>>>> Reported-by: Steev Klimaszewski <steev@kali.org>
>>>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>>> ---
>>>>>  drivers/soc/qcom/llcc-qcom.c | 17 ++++++++++++-----
>>>>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
>>>>> index 7b7c5a38bac6..8d840702df50 100644
>>>>> --- a/drivers/soc/qcom/llcc-qcom.c
>>>>> +++ b/drivers/soc/qcom/llcc-qcom.c
>>>>> @@ -1012,11 +1012,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>>>>>  
>>>>>  	drv_data->ecc_irq = platform_get_irq_optional(pdev, 0);
>>>>>  
>>>>> -	llcc_edac = platform_device_register_data(&pdev->dev,
>>>>> -					"qcom_llcc_edac", -1, drv_data,
>>>>> -					sizeof(*drv_data));
>>>>> -	if (IS_ERR(llcc_edac))
>>>>> -		dev_err(dev, "Failed to register llcc edac driver\n");
>>>>> +	/*
>>>>> +	 * The platforms based on SDM845 SoC locks the access to EDAC registers
>>>>> +	 * in bootloader. So probing the EDAC driver will result in a crash.
>>>>> +	 * Hence, disable the creation of EDAC platform device on SDM845.
>>>>> +	 */
>>>>> +	if (!of_device_is_compatible(dev->of_node, "qcom,sdm845-llcc")) {
>>>>
>>>> Don't spread of_device_is_compatible() in driver code. You have driver
>>>> data for this.
>>>>
>>>
>>> Yeah, but there is no ID to in the driver data to identify an SoC. 
>>
>> What do you mean there is no? You use exactly the same compatible as the
>> one in driver data.
>>
> 
> Right, but I was saying that there is no unique field to identify an SoC.
> 
>>
>>> I could add
>>> one but is that really worth doing so? Is using of_device_is_compatible() in
>>> drivers discouraged nowadays?
>>
>> Because it spreads variant matching all over. It does not scale. drv
>> data fields are the way or better quirks/flags.
>>
> 
> The driver quirk/flags are usually beneficial if it applies to multiple
> platforms, otherwise they are a bit overkill IMO just like in this case.
> 
> One can argue that this matching could spread to other SoCs in the future, but
> I don't think that could happen for this case.

That's the argument for every flag/quirk/field. Driver already uses it -
see need_llcc_cfg being set for only one (!!!) variant. Now you add
orthogonal field just as of_device_is_compatible(). No, that's why we
have driver data and as I said - it is already used.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value
  2023-01-18 15:08 ` [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value Manivannan Sadhasivam
@ 2023-01-18 17:46   ` Bjorn Andersson
  2023-01-18 19:05     ` Borislav Petkov
  2023-01-19 10:55   ` Borislav Petkov
  1 sibling, 1 reply; 29+ messages in thread
From: Bjorn Andersson @ 2023-01-18 17:46 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: robh+dt, krzysztof.kozlowski+dt, bp, tony.luck, quic_saipraka,
	konrad.dybcio, linux-arm-msm, linux-kernel, james.morse, mchehab,
	rric, linux-edac, quic_ppareek, luca.weiss, ahalaney, steev,
	stable

On Wed, Jan 18, 2023 at 08:38:48PM +0530, Manivannan Sadhasivam wrote:
> The EDAC drivers may optionally pass the poll_msec value. Use that value
> if available, else fall back to 1000ms.
> 
>   [ bp: Touchups. ]
> 
> Fixes: e27e3dac6517 ("drivers/edac: add edac_device class")
> Reported-by: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Your S-o-b should be the last one to indicate that you are the  one
certifying the origin of this patch.

> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>

If the two of you wrote the patch, please add a Co-developed-by.

Thanks,
Bjorn

> Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
> Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
> Cc: <stable@vger.kernel.org> # 4.9
> Link: https://lore.kernel.org/r/COZYL8MWN97H.MROQ391BGA09@otso
> ---
>  drivers/edac/edac_device.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
> index 19522c568aa5..a50b7bcfb731 100644
> --- a/drivers/edac/edac_device.c
> +++ b/drivers/edac/edac_device.c
> @@ -34,6 +34,9 @@
>  static DEFINE_MUTEX(device_ctls_mutex);
>  static LIST_HEAD(edac_device_list);
>  
> +/* Default workqueue processing interval on this instance, in msecs */
> +#define DEFAULT_POLL_INTERVAL 1000
> +
>  #ifdef CONFIG_EDAC_DEBUG
>  static void edac_device_dump_device(struct edac_device_ctl_info *edac_dev)
>  {
> @@ -336,7 +339,7 @@ static void edac_device_workq_function(struct work_struct *work_req)
>  	 * whole one second to save timers firing all over the period
>  	 * between integral seconds
>  	 */
> -	if (edac_dev->poll_msec == 1000)
> +	if (edac_dev->poll_msec == DEFAULT_POLL_INTERVAL)
>  		edac_queue_work(&edac_dev->work, round_jiffies_relative(edac_dev->delay));
>  	else
>  		edac_queue_work(&edac_dev->work, edac_dev->delay);
> @@ -366,7 +369,7 @@ static void edac_device_workq_setup(struct edac_device_ctl_info *edac_dev,
>  	 * timers firing on sub-second basis, while they are happy
>  	 * to fire together on the 1 second exactly
>  	 */
> -	if (edac_dev->poll_msec == 1000)
> +	if (edac_dev->poll_msec == DEFAULT_POLL_INTERVAL)
>  		edac_queue_work(&edac_dev->work, round_jiffies_relative(edac_dev->delay));
>  	else
>  		edac_queue_work(&edac_dev->work, edac_dev->delay);
> @@ -398,7 +401,7 @@ void edac_device_reset_delay_period(struct edac_device_ctl_info *edac_dev,
>  {
>  	unsigned long jiffs = msecs_to_jiffies(value);
>  
> -	if (value == 1000)
> +	if (value == DEFAULT_POLL_INTERVAL)
>  		jiffs = round_jiffies_relative(value);
>  
>  	edac_dev->poll_msec = value;
> @@ -443,11 +446,7 @@ int edac_device_add_device(struct edac_device_ctl_info *edac_dev)
>  		/* This instance is NOW RUNNING */
>  		edac_dev->op_state = OP_RUNNING_POLL;
>  
> -		/*
> -		 * enable workq processing on this instance,
> -		 * default = 1000 msec
> -		 */
> -		edac_device_workq_setup(edac_dev, 1000);
> +		edac_device_workq_setup(edac_dev, edac_dev->poll_msec ?: DEFAULT_POLL_INTERVAL);
>  	} else {
>  		edac_dev->op_state = OP_RUNNING_INTERRUPT;
>  	}
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value
  2023-01-18 17:46   ` Bjorn Andersson
@ 2023-01-18 19:05     ` Borislav Petkov
  0 siblings, 0 replies; 29+ messages in thread
From: Borislav Petkov @ 2023-01-18 19:05 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Manivannan Sadhasivam, robh+dt, krzysztof.kozlowski+dt,
	tony.luck, quic_saipraka, konrad.dybcio, linux-arm-msm,
	linux-kernel, james.morse, mchehab, rric, linux-edac,
	quic_ppareek, luca.weiss, ahalaney, steev, stable

On Wed, Jan 18, 2023 at 11:46:25AM -0600, Bjorn Andersson wrote:
> On Wed, Jan 18, 2023 at 08:38:48PM +0530, Manivannan Sadhasivam wrote:
> > The EDAC drivers may optionally pass the poll_msec value. Use that value
> > if available, else fall back to 1000ms.
> > 
> >   [ bp: Touchups. ]
> > 
> > Fixes: e27e3dac6517 ("drivers/edac: add edac_device class")
> > Reported-by: Luca Weiss <luca.weiss@fairphone.com>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Your S-o-b should be the last one to indicate that you are the  one
> certifying the origin of this patch.

I took his and massaged it a bit.

I'll fix up the order properly when applying.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value
  2023-01-18 15:08 ` [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value Manivannan Sadhasivam
  2023-01-18 17:46   ` Bjorn Andersson
@ 2023-01-19 10:55   ` Borislav Petkov
  1 sibling, 0 replies; 29+ messages in thread
From: Borislav Petkov @ 2023-01-19 10:55 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, tony.luck,
	quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, stable

On Wed, Jan 18, 2023 at 08:38:48PM +0530, Manivannan Sadhasivam wrote:
> The EDAC drivers may optionally pass the poll_msec value. Use that value
> if available, else fall back to 1000ms.
> 
>   [ bp: Touchups. ]
> 
> Fixes: e27e3dac6517 ("drivers/edac: add edac_device class")
> Reported-by: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
> Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
> Cc: <stable@vger.kernel.org> # 4.9
> Link: https://lore.kernel.org/r/COZYL8MWN97H.MROQ391BGA09@otso
> ---
>  drivers/edac/edac_device.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 02/17] EDAC/qcom: Add platform_device_id table for module autoloading
  2023-01-18 15:08 ` [PATCH v6 02/17] EDAC/qcom: Add platform_device_id table for module autoloading Manivannan Sadhasivam
@ 2023-01-20 16:10   ` Borislav Petkov
  0 siblings, 0 replies; 29+ messages in thread
From: Borislav Petkov @ 2023-01-20 16:10 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, tony.luck,
	quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev

On Wed, Jan 18, 2023 at 08:38:49PM +0530, Manivannan Sadhasivam wrote:
> Add a device ID table so that the driver loads automatically when the
> associated platform_device gets registered.
> 
> Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
> Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
> Reported-by: Andrew Halaney <ahalaney@redhat.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/edac/qcom_edac.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Applied, thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 03/17] EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's pvt_info
  2023-01-18 15:08 ` [PATCH v6 03/17] EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's pvt_info Manivannan Sadhasivam
@ 2023-01-20 18:56   ` Borislav Petkov
  0 siblings, 0 replies; 29+ messages in thread
From: Borislav Petkov @ 2023-01-20 18:56 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, tony.luck,
	quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev, stable

On Wed, Jan 18, 2023 at 08:38:50PM +0530, Manivannan Sadhasivam wrote:
> The memory for "llcc_driv_data" is allocated by the LLCC driver. But when
> it is passed as "pvt_info" to the EDAC core, it will get freed during the
> qcom_edac driver release. So when the qcom_edac driver gets probed again,
> it will try to use the freed data leading to the use-after-free bug.
> 
> Hence, do not pass "llcc_driv_data" as pvt_info but rather reference it
> using the "platform_data" in the qcom_edac driver.
> 
> Cc: <stable@vger.kernel.org> # 4.20
> Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
> Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
> Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
> Reported-by: Steev Klimaszewski <steev@kali.org>
> Reviewed-by: Borislav Petkov (AMD) <bp@alien8.de>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/edac/qcom_edac.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 16/17] qcom: llcc/edac: Support polling mode for ECC handling
  2023-01-18 15:09 ` [PATCH v6 16/17] qcom: llcc/edac: Support polling mode for ECC handling Manivannan Sadhasivam
@ 2023-01-20 19:04   ` Borislav Petkov
  0 siblings, 0 replies; 29+ messages in thread
From: Borislav Petkov @ 2023-01-20 19:04 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: andersson, robh+dt, krzysztof.kozlowski+dt, tony.luck,
	quic_saipraka, konrad.dybcio, linux-arm-msm, linux-kernel,
	james.morse, mchehab, rric, linux-edac, quic_ppareek, luca.weiss,
	ahalaney, steev

On Wed, Jan 18, 2023 at 08:39:03PM +0530, Manivannan Sadhasivam wrote:
> Not all Qcom platforms support IRQ mode for ECC handling. For those
> platforms, the current EDAC driver will not be probed due to missing ECC
> IRQ in devicetree.
> 
> So add support for polling mode so that the EDAC driver can be used on all
> Qcom platforms supporting LLCC.
> 
> The polling delay of 5000ms is chosen based on Qcom downstream/vendor
> driver.
> 
> Reported-by: Luca Weiss <luca.weiss@fairphone.com>
> Tested-by: Luca Weiss <luca.weiss@fairphone.com>
> Tested-by: Steev Klimaszewski <steev@kali.org> # Thinkpad X13s
> Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/edac/qcom_edac.c     | 50 +++++++++++++++++++++---------------
>  drivers/soc/qcom/llcc-qcom.c | 13 +++++-----
>  2 files changed, 35 insertions(+), 28 deletions(-)

Reviewed-by: Borislav Petkov (AMD) <bp@alien8.de>

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2023-01-20 19:04 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-18 15:08 [PATCH v6 00/17] Qcom: LLCC/EDAC: Fix base address used for LLCC banks Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 01/17] EDAC/device: Respect any driver-supplied workqueue polling value Manivannan Sadhasivam
2023-01-18 17:46   ` Bjorn Andersson
2023-01-18 19:05     ` Borislav Petkov
2023-01-19 10:55   ` Borislav Petkov
2023-01-18 15:08 ` [PATCH v6 02/17] EDAC/qcom: Add platform_device_id table for module autoloading Manivannan Sadhasivam
2023-01-20 16:10   ` Borislav Petkov
2023-01-18 15:08 ` [PATCH v6 03/17] EDAC/qcom: Do not pass llcc_driv_data as edac_device_ctl_info's pvt_info Manivannan Sadhasivam
2023-01-20 18:56   ` Borislav Petkov
2023-01-18 15:08 ` [PATCH v6 04/17] dt-bindings: arm: msm: Update the maintainers for LLCC Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 05/17] dt-bindings: arm: msm: Fix register regions used for LLCC banks Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 06/17] arm64: dts: qcom: sdm845: Fix the base addresses of " Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 07/17] arm64: dts: qcom: sc7180: " Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 08/17] arm64: dts: qcom: sc7280: " Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 09/17] arm64: dts: qcom: sc8280xp: " Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 10/17] arm64: dts: qcom: sm8150: " Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 11/17] arm64: dts: qcom: sm8250: " Manivannan Sadhasivam
2023-01-18 15:08 ` [PATCH v6 12/17] arm64: dts: qcom: sm8350: " Manivannan Sadhasivam
2023-01-18 15:09 ` [PATCH v6 13/17] arm64: dts: qcom: sm8450: " Manivannan Sadhasivam
2023-01-18 15:09 ` [PATCH v6 14/17] arm64: dts: qcom: sm6350: " Manivannan Sadhasivam
2023-01-18 15:09 ` [PATCH v6 15/17] qcom: llcc/edac: Fix the base address used for accessing " Manivannan Sadhasivam
2023-01-18 15:09 ` [PATCH v6 16/17] qcom: llcc/edac: Support polling mode for ECC handling Manivannan Sadhasivam
2023-01-20 19:04   ` Borislav Petkov
2023-01-18 15:09 ` [PATCH v6 17/17] soc: qcom: llcc: Do not create EDAC platform device on SDM845 Manivannan Sadhasivam
2023-01-18 15:37   ` Krzysztof Kozlowski
2023-01-18 15:59     ` Manivannan Sadhasivam
2023-01-18 16:05       ` Krzysztof Kozlowski
2023-01-18 16:26         ` Manivannan Sadhasivam
2023-01-18 17:06           ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).