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 affects multiple platforms but I have only tested this on
SM8250, SM8450, and SM6350. Testing on other platforms is welcomed.
Thanks,
Mani
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 (16):
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
EDAC/device: Make use of poll_msec value in edac_device_ctl_info
struct
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
qcom: llcc/edac: Fix the base address used for accessing LLCC banks
qcom: llcc/edac: Support polling mode for ECC handling
.../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 | 2 +-
drivers/edac/qcom_edac.c | 63 +++++----
drivers/soc/qcom/llcc-qcom.c | 85 ++++++------
include/linux/soc/qcom/llcc-qcom.h | 6 +-
14 files changed, 243 insertions(+), 95 deletions(-)
--
2.25.1
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 <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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..e1c0d9faf46e 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 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
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 <[email protected]>
Tested-by: Luca Weiss <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/edac/qcom_edac.c | 37 +++++++++++++++++++++++++-----------
drivers/soc/qcom/llcc-qcom.c | 13 ++++++-------
2 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 1d3cc1930a74..cfcdc35b0373 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,22 +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;
+ /* Check if LLCC driver has passed ECC IRQ */
+ ecc_irq = llcc_driv_data->ecc_irq;
+ if (ecc_irq > 0) {
+ /* Use interrupt mode if IRQ is available */
+ edac_op_state = EDAC_OPSTATE_INT;
+ } else {
+ /* Fall back to polling mode otherwise */
+ edac_op_state = EDAC_OPSTATE_POLL;
+ edev_ctl->poll_msec = ECC_POLL_MSEC;
+ edev_ctl->edac_check = llcc_ecc_check;
+ }
+
rc = edac_device_add_device(edev_ctl);
if (rc)
goto out_mem;
platform_set_drvdata(pdev, edev_ctl);
- /* Request for 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,
+ /* Request ECC IRQ if available */
+ if (ecc_irq > 0) {
+ 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)
+ goto out_dev;
+ }
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
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 <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
On Thu, Dec 22, 2022 at 06:46:40PM +0530, Manivannan Sadhasivam wrote:
> 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 affects multiple platforms but I have only tested this on
> SM8250, SM8450, and SM6350. Testing on other platforms is welcomed.
>
> Thanks,
> Mani
Tested-by: Andrew Halaney <[email protected]> # sa8540p-ride
With this series applied:
1. The module loads automatically
2. I get the edac driver setup in interrupt mode
3. unloading/loading the module repeatedly works without issue
Thanks Mani!
>
> 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 (16):
> 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
> EDAC/device: Make use of poll_msec value in edac_device_ctl_info
> struct
> 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
> qcom: llcc/edac: Fix the base address used for accessing LLCC banks
> qcom: llcc/edac: Support polling mode for ECC handling
>
> .../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 | 2 +-
> drivers/edac/qcom_edac.c | 63 +++++----
> drivers/soc/qcom/llcc-qcom.c | 85 ++++++------
> include/linux/soc/qcom/llcc-qcom.h | 6 +-
> 14 files changed, 243 insertions(+), 95 deletions(-)
>
> --
> 2.25.1
>
On Thu, Dec 22, 2022 at 7:17 AM Manivannan Sadhasivam
<[email protected]> wrote:
>
> 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 affects multiple platforms but I have only tested this on
> SM8250, SM8450, and SM6350. Testing on other platforms is welcomed.
>
> Thanks,
> Mani
>
> 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 (16):
> 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
> EDAC/device: Make use of poll_msec value in edac_device_ctl_info
> struct
> 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
> qcom: llcc/edac: Fix the base address used for accessing LLCC banks
> qcom: llcc/edac: Support polling mode for ECC handling
>
> .../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 | 2 +-
> drivers/edac/qcom_edac.c | 63 +++++----
> drivers/soc/qcom/llcc-qcom.c | 85 ++++++------
> include/linux/soc/qcom/llcc-qcom.h | 6 +-
> 14 files changed, 243 insertions(+), 95 deletions(-)
>
> --
> 2.25.1
>
Tested-by: Steev Klimaszewski <[email protected]> # Thinkpad X13s
Like Andrew, tested starting, restarting, and stopping the edac
service multiple times.
edac-ctl --mainboard reports "edac-ctl: mainboard: LENOVO 21BX0015US"
Thanks Mani!
-- steev
On Thu, Dec 22, 2022 at 06:46:40PM +0530, Manivannan Sadhasivam wrote:
> 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 affects multiple platforms but I have only tested this on
> SM8250, SM8450, and SM6350. Testing on other platforms is welcomed.
>
> Thanks,
> Mani
>
> 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 (16):
> 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
> EDAC/device: Make use of poll_msec value in edac_device_ctl_info
> struct
> 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
Can you clarify if these patches needs to be applied in the specific
order, or if the EDAC changes can go in independently of the llcc driver
changes?
Thanks,
Bjorn
> qcom: llcc/edac: Fix the base address used for accessing LLCC banks
> qcom: llcc/edac: Support polling mode for ECC handling
>
> .../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 | 2 +-
> drivers/edac/qcom_edac.c | 63 +++++----
> drivers/soc/qcom/llcc-qcom.c | 85 ++++++------
> include/linux/soc/qcom/llcc-qcom.h | 6 +-
> 14 files changed, 243 insertions(+), 95 deletions(-)
>
> --
> 2.25.1
>
On Tue, Dec 27, 2022 at 10:31:38PM -0600, Bjorn Andersson wrote:
> On Thu, Dec 22, 2022 at 06:46:40PM +0530, Manivannan Sadhasivam wrote:
> > 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 affects multiple platforms but I have only tested this on
> > SM8250, SM8450, and SM6350. Testing on other platforms is welcomed.
> >
> > Thanks,
> > Mani
> >
> > 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 (16):
> > 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
> > EDAC/device: Make use of poll_msec value in edac_device_ctl_info
> > struct
> > 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
>
> Can you clarify if these patches needs to be applied in the specific
> order, or if the EDAC changes can go in independently of the llcc driver
> changes?
>
The EDAC/qcom and EDAC/device patches can go independently of the rest. All
other patches needs to go through qcom tree.
Steev spotted an issue on C630, so I'm going to respin the series one more
time.
Thanks,
Mani
> Thanks,
> Bjorn
>
> > qcom: llcc/edac: Fix the base address used for accessing LLCC banks
> > qcom: llcc/edac: Support polling mode for ECC handling
> >
> > .../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 | 2 +-
> > drivers/edac/qcom_edac.c | 63 +++++----
> > drivers/soc/qcom/llcc-qcom.c | 85 ++++++------
> > include/linux/soc/qcom/llcc-qcom.h | 6 +-
> > 14 files changed, 243 insertions(+), 95 deletions(-)
> >
> > --
> > 2.25.1
> >
--
மணிவண்ணன் சதாசிவம்