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 and SM8450. Testing on other platforms is welcomed.
Thanks,
Mani
Manivannan Sadhasivam (12):
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
.../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/qcom_edac.c | 14 +-
drivers/soc/qcom/llcc-qcom.c | 64 +++++----
include/linux/soc/qcom/llcc-qcom.h | 4 +-
13 files changed, 197 insertions(+), 67 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.
Cc: <[email protected]> # 5.17
Fixes: 9ac8999e8d6c ("arm64: dts: qcom: sm8350: Add LLCC node")
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.
On SC7180, there is only one LLCC bank available. So let's just pass that
as "llcc0_base".
Cc: <[email protected]> # 5.6
Fixes: c831fa299996 ("arm64: dts: qcom: sc7180: Add Last level cache controller node")
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.
Cc: <[email protected]> # 5.12
Fixes: 0085a33a25cc ("arm64: dts: qcom: sm8250: Add support for LLCC block")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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
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.
Cc: <[email protected]> # 6.0
Fixes: 152d1faf1e2f ("arm64: dts: qcom: add SC8280XP platform")
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.
Cc: <[email protected]> # 5.18
Fixes: 1dc3e50eb680 ("arm64: dts: qcom: sm8450: Add LLCC/system-cache-controller node")
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 driver has been using a fixed register offset stride for accessing
the 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.
Hence, obtain the base address of each LLCC bank from devicetree and get
rid of the fixed stride.
Cc: <[email protected]> # 4.20
Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")
Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/edac/qcom_edac.c | 14 +++----
drivers/soc/qcom/llcc-qcom.c | 64 ++++++++++++++++++------------
include/linux/soc/qcom/llcc-qcom.h | 4 +-
3 files changed, 44 insertions(+), 38 deletions(-)
diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 97a27e42dd61..70bd39a91b89 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->regmap[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->regmap[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->regmap[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->regmap[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->regmap[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..7264ac9993e0 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
@@ -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,12 +933,46 @@ 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, "llcc0_base");
+ if (IS_ERR(regmap)) {
+ ret = PTR_ERR(regmap);
+ goto err;
+ }
+
+ 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->regmap = devm_kcalloc(dev, num_banks, sizeof(*drv_data->regmap), GFP_KERNEL);
+ if (!drv_data->regmap) {
+ ret = -ENOMEM;
goto err;
}
+ drv_data->regmap[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->regmap[i] = qcom_llcc_init_mmio(pdev, base);
+ if (IS_ERR(drv_data->regmap[i])) {
+ ret = PTR_ERR(drv_data->regmap[i]);
+ kfree(base);
+ goto err;
+ }
+
+ kfree(base);
+ }
+
drv_data->bcast_regmap =
qcom_llcc_init_mmio(pdev, "llcc_broadcast_base");
if (IS_ERR(drv_data->bcast_regmap)) {
@@ -947,8 +980,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
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 +988,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 +995,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..4b8bf585f9ba 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -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 **regmap;
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
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.
Cc: <[email protected]> # 4.20
Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")
Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
Reported-by: Parikshit Pareek <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/edac/qcom_edac.c | 14 +++----
drivers/soc/qcom/llcc-qcom.c | 64 ++++++++++++++++++------------
include/linux/soc/qcom/llcc-qcom.h | 4 +-
3 files changed, 44 insertions(+), 38 deletions(-)
diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 97a27e42dd61..70bd39a91b89 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->regmap[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->regmap[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->regmap[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->regmap[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->regmap[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..7264ac9993e0 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
@@ -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,12 +933,46 @@ 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, "llcc0_base");
+ if (IS_ERR(regmap)) {
+ ret = PTR_ERR(regmap);
+ goto err;
+ }
+
+ 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->regmap = devm_kcalloc(dev, num_banks, sizeof(*drv_data->regmap), GFP_KERNEL);
+ if (!drv_data->regmap) {
+ ret = -ENOMEM;
goto err;
}
+ drv_data->regmap[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->regmap[i] = qcom_llcc_init_mmio(pdev, base);
+ if (IS_ERR(drv_data->regmap[i])) {
+ ret = PTR_ERR(drv_data->regmap[i]);
+ kfree(base);
+ goto err;
+ }
+
+ kfree(base);
+ }
+
drv_data->bcast_regmap =
qcom_llcc_init_mmio(pdev, "llcc_broadcast_base");
if (IS_ERR(drv_data->bcast_regmap)) {
@@ -947,8 +980,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
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 +988,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 +995,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..4b8bf585f9ba 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -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 **regmap;
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
On Wed, Dec 07, 2022 at 07:29:21PM +0530, Manivannan Sadhasivam wrote:
> The LLCC driver has been using a fixed register offset stride for accessing
> the 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.
>
> Hence, obtain the base address of each LLCC bank from devicetree and get
> rid of the fixed stride.
>
> Cc: <[email protected]> # 4.20
> Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")
> Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
Please ignore this patch as this is a duplicate that got sneaked in. I will
remove it in next version.
Thanks,
Mani
> ---
> drivers/edac/qcom_edac.c | 14 +++----
> drivers/soc/qcom/llcc-qcom.c | 64 ++++++++++++++++++------------
> include/linux/soc/qcom/llcc-qcom.h | 4 +-
> 3 files changed, 44 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
> index 97a27e42dd61..70bd39a91b89 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->regmap[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->regmap[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->regmap[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->regmap[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->regmap[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..7264ac9993e0 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
> @@ -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,12 +933,46 @@ 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, "llcc0_base");
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + goto err;
> + }
> +
> + 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->regmap = devm_kcalloc(dev, num_banks, sizeof(*drv_data->regmap), GFP_KERNEL);
> + if (!drv_data->regmap) {
> + ret = -ENOMEM;
> goto err;
> }
>
> + drv_data->regmap[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->regmap[i] = qcom_llcc_init_mmio(pdev, base);
> + if (IS_ERR(drv_data->regmap[i])) {
> + ret = PTR_ERR(drv_data->regmap[i]);
> + kfree(base);
> + goto err;
> + }
> +
> + kfree(base);
> + }
> +
> drv_data->bcast_regmap =
> qcom_llcc_init_mmio(pdev, "llcc_broadcast_base");
> if (IS_ERR(drv_data->bcast_regmap)) {
> @@ -947,8 +980,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> 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 +988,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 +995,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..4b8bf585f9ba 100644
> --- a/include/linux/soc/qcom/llcc-qcom.h
> +++ b/include/linux/soc/qcom/llcc-qcom.h
> @@ -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 **regmap;
> 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
>
--
மணிவண்ணன் சதாசிவம்
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.
Cc: <[email protected]> # 5.4
Fixes: ba0411ddd133 ("arm64: dts: sdm845: Add device node for Last level cache controller")
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
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 <[email protected]>
Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
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 <[email protected]>
- - Sai Prakash Ranjan <[email protected]>
+ - Sai Prakash Ranjan <[email protected]>
description: |
LLCC (Last Level Cache Controller) provides last level of cache memory in SoC,
--
2.25.1
On Wed, Dec 07, 2022 at 07:29:22PM +0530, Manivannan Sadhasivam wrote:
> 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.
>
> Cc: <[email protected]> # 4.20
> Fixes: a3134fb09e0b ("drivers: soc: Add LLCC driver")
> Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
> Reported-by: Parikshit Pareek <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/edac/qcom_edac.c | 14 +++----
> drivers/soc/qcom/llcc-qcom.c | 64 ++++++++++++++++++------------
> include/linux/soc/qcom/llcc-qcom.h | 4 +-
> 3 files changed, 44 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
> index 97a27e42dd61..70bd39a91b89 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->regmap[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->regmap[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->regmap[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->regmap[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->regmap[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..7264ac9993e0 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
> @@ -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,12 +933,46 @@ 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, "llcc0_base");
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + goto err;
> + }
> +
> + cfg = of_device_get_match_data(&pdev->dev);
> +
> + ret = regmap_read(regmap, cfg->reg_offset[LLCC_COMMON_STATUS0],
> + &num_banks);
Don't be afraid of leaving this line unwrapped.
There's a few lines changed above that would be easier to read if
allowed beyond 80 chars as well.
> + if (ret)
> + goto err;
> +
> + num_banks &= LLCC_LB_CNT_MASK;
> + num_banks >>= LLCC_LB_CNT_SHIFT;
> + drv_data->num_banks = num_banks;
> +
> + drv_data->regmap = devm_kcalloc(dev, num_banks, sizeof(*drv_data->regmap), GFP_KERNEL);
> + if (!drv_data->regmap) {
> + ret = -ENOMEM;
> goto err;
> }
>
> + drv_data->regmap[0] = regmap;
> +
> + /* Initialize rest of LLCC bank regmaps */
> + for (i = 1; i < num_banks; i++) {
> + char *base = kasprintf(GFP_KERNEL, "llcc%d_base", i);
As mentioned in the binding, I think you should obtain the regions by
index instead of name.
> +
> + drv_data->regmap[i] = qcom_llcc_init_mmio(pdev, base);
> + if (IS_ERR(drv_data->regmap[i])) {
> + ret = PTR_ERR(drv_data->regmap[i]);
> + kfree(base);
> + goto err;
> + }
> +
> + kfree(base);
> + }
> +
> drv_data->bcast_regmap =
> qcom_llcc_init_mmio(pdev, "llcc_broadcast_base");
> if (IS_ERR(drv_data->bcast_regmap)) {
> @@ -947,8 +980,6 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> 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 +988,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 +995,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..4b8bf585f9ba 100644
> --- a/include/linux/soc/qcom/llcc-qcom.h
> +++ b/include/linux/soc/qcom/llcc-qcom.h
> @@ -129,12 +129,11 @@ struct llcc_edac_reg_offset {
Please also update @regmap description.
> * @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 **regmap;
How about making this plural, to reflect that it's now a set of regmaps?
Regards,
Bjorn
> 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
>
Hi Mani,
On 12/7/2022 7:29 PM, Manivannan Sadhasivam wrote:
> 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 <[email protected]>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> 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 <[email protected]>
> - - Sai Prakash Ranjan <[email protected]>
> + - Sai Prakash Ranjan <[email protected]>
>
Thanks for updating, I believe you can add yourself as well now since
you maintain LLCC driver.
Either way,
Acked-by: Sai Prakash Ranjan <[email protected]>
Thanks,
Sai
Hi Manivannan,
On Wed Dec 7, 2022 at 2:59 PM CET, 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 and SM8450. Testing on other platforms is welcomed.
If you can tell me *how* I can test it, I'd be happy to test the series
on sm6350, like how to trigger the EDAC IRQ.
So far without any extra patches I don't even see the driver probing,
with this in kconfig
+CONFIG_EDAC=y
+CONFIG_EDAC_QCOM=y
I do have /sys/bus/platform/drivers/qcom_llcc_edac at runtime but
nothing in there (except bind, uevent and unbind), and also nothing
interesting in dmesg with "llcc", with edac there's just this message:
[ 0.064800] EDAC MC: Ver: 3.0.0
From what I'm seeing now the edac driver is only registered if the
interrupt is specified but it doesn't seem like sm6350 (=lagoon) has
this irq? Downstream dts is just this:
cache-controller@9200000 {
compatible = "lagoon-llcc-v1";
reg = <0x9200000 0x50000> , <0x9600000 0x50000>;
reg-names = "llcc_base", "llcc_broadcast_base";
cap-based-alloc-and-pwr-collapse;
};
From looking at the downstream code, perhaps it's using the polling mode
there?
/* Request for ecc irq */
ecc_irq = llcc_driv_data->ecc_irq;
if (ecc_irq < 0) {
dev_info(dev, "No ECC IRQ; defaulting to polling mode\n");
Let me know what you think.
Regards
Luca
>
> Thanks,
> Mani
>
> Manivannan Sadhasivam (12):
> 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
>
> .../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/qcom_edac.c | 14 +-
> drivers/soc/qcom/llcc-qcom.c | 64 +++++----
> include/linux/soc/qcom/llcc-qcom.h | 4 +-
> 13 files changed, 197 insertions(+), 67 deletions(-)
>
> --
> 2.25.1
Hi Sai,
On Thu, Dec 08, 2022 at 08:45:48AM +0530, Sai Prakash Ranjan wrote:
> Hi Mani,
>
> On 12/7/2022 7:29 PM, Manivannan Sadhasivam wrote:
> > 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 <[email protected]>
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > 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 <[email protected]>
> > - - Sai Prakash Ranjan <[email protected]>
> > + - Sai Prakash Ranjan <[email protected]>
>
> Thanks for updating, I believe you can add yourself as well now since
> you maintain LLCC driver.
>
I only maintain the EDAC driver, so I'll leave llcc to you :)
> Either way,
>
> Acked-by: Sai Prakash Ranjan <[email protected]>
>
Thanks,
Mani
>
> Thanks,
> Sai
>
--
மணிவண்ணன் சதாசிவம்
Hi Luca,
On Thu, Dec 08, 2022 at 10:16:27AM +0100, Luca Weiss wrote:
> Hi Manivannan,
>
> On Wed Dec 7, 2022 at 2:59 PM CET, 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 and SM8450. Testing on other platforms is welcomed.
>
> If you can tell me *how* I can test it, I'd be happy to test the series
> on sm6350, like how to trigger the EDAC IRQ.
>
I suppose there is no manual way to trigger EDAC IRQ on Qcom platforms.
For testing the series, I manually called the EDAC IRQ handler to verify
that it doesn't crash reading the registers.
> So far without any extra patches I don't even see the driver probing,
> with this in kconfig
>
> +CONFIG_EDAC=y
> +CONFIG_EDAC_QCOM=y
>
> I do have /sys/bus/platform/drivers/qcom_llcc_edac at runtime but
> nothing in there (except bind, uevent and unbind), and also nothing
> interesting in dmesg with "llcc", with edac there's just this message:
>
> [ 0.064800] EDAC MC: Ver: 3.0.0
>
> From what I'm seeing now the edac driver is only registered if the
> interrupt is specified but it doesn't seem like sm6350 (=lagoon) has
> this irq? Downstream dts is just this:
>
Right. The upstream EDAC driver only works in IRQ mode. So you need the
interrupts property in LLCC devicetree node for probing.
> cache-controller@9200000 {
> compatible = "lagoon-llcc-v1";
> reg = <0x9200000 0x50000> , <0x9600000 0x50000>;
> reg-names = "llcc_base", "llcc_broadcast_base";
> cap-based-alloc-and-pwr-collapse;
> };
>
> From looking at the downstream code, perhaps it's using the polling mode
> there?
>
> /* Request for ecc irq */
> ecc_irq = llcc_driv_data->ecc_irq;
> if (ecc_irq < 0) {
> dev_info(dev, "No ECC IRQ; defaulting to polling mode\n");
>
In the next version, I will add polling support so that you can test the
series on your platform without any hacks.
Thanks,
Mani
> Let me know what you think.
>
> Regards
> Luca
>
> >
> > Thanks,
> > Mani
> >
> > Manivannan Sadhasivam (12):
> > 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
> >
> > .../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/qcom_edac.c | 14 +-
> > drivers/soc/qcom/llcc-qcom.c | 64 +++++----
> > include/linux/soc/qcom/llcc-qcom.h | 4 +-
> > 13 files changed, 197 insertions(+), 67 deletions(-)
> >
> > --
> > 2.25.1
>
--
மணிவண்ணன் சதாசிவம்