2023-03-13 12:41:24

by Komal Bajaj

[permalink] [raw]
Subject: [PATCH v2 0/5] soc: qcom: llcc: Add support for QDU1000/QRU1000

This patchset refactor LLCC driver and adds LLCC support for the
Qualcomm QDU1000 and QRU1000 SoCs. Since QDU1000/QRU1000 supports
multi channel DDR, add support for multi channel DDR configuration
in LLCC.

Changes in v2:
- Rebased on top of 6.3-rc2

Komal Bajaj (5):
soc: qcom: llcc: Refactor llcc driver to support multiple
configuration
dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC
dt-bindings: arm: msm: Add LLCC compatible for QDU1000/QRU1000
soc: qcom: Add LLCC support for multi channel DDR
soc: qcom: llcc: Add QDU1000 and QRU1000 LLCC support

.../bindings/arm/msm/qcom,llcc.yaml | 10 +
drivers/soc/qcom/llcc-qcom.c | 308 +++++++++++++-----
include/linux/soc/qcom/llcc-qcom.h | 4 +-
3 files changed, 240 insertions(+), 82 deletions(-)

--
2.39.1



2023-03-13 12:41:30

by Komal Bajaj

[permalink] [raw]
Subject: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC

Add description for additional nodes needed to support
mulitple channel DDR configurations in LLCC.

Signed-off-by: Komal Bajaj <[email protected]>
---
Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
index 38efcad56dbd..9a4a76caf490 100644
--- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
@@ -37,15 +37,24 @@ properties:
items:
- description: LLCC base register region
- description: LLCC broadcast base register region
+ - description: Feature register to decide which LLCC configuration
+ to use, this is optional

reg-names:
items:
- const: llcc_base
- const: llcc_broadcast_base
+ - const: multi_channel_register

interrupts:
maxItems: 1

+ multi-ch-bit-off:
+ items:
+ - description: Specifies the offset in bits into the multi_channel_register
+ and the number of bits used to decide which LLCC configuration
+ to use
+
required:
- compatible
- reg
--
2.39.1


2023-03-13 12:41:28

by Komal Bajaj

[permalink] [raw]
Subject: [PATCH v2 1/5] soc: qcom: llcc: Refactor llcc driver to support multiple configuration

Refactor driver to support multiple configuration for llcc on a target.

Signed-off-by: Komal Bajaj <[email protected]>
---
drivers/soc/qcom/llcc-qcom.c | 191 ++++++++++++++++++++---------------
1 file changed, 112 insertions(+), 79 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 23ce2f78c4ed..00699a0c047e 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -416,92 +416,125 @@ static const u32 llcc_v2_1_reg_offset[] = {
[LLCC_COMMON_STATUS0] = 0x0003400c,
};

-static const struct qcom_llcc_config sc7180_cfg = {
- .sct_data = sc7180_data,
- .size = ARRAY_SIZE(sc7180_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sc7180_cfg[] = {
+ {
+ .sct_data = sc7180_data,
+ .size = ARRAY_SIZE(sc7180_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sc7280_cfg = {
- .sct_data = sc7280_data,
- .size = ARRAY_SIZE(sc7280_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sc7280_cfg[] = {
+ {
+ .sct_data = sc7280_data,
+ .size = ARRAY_SIZE(sc7280_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sc8180x_cfg = {
- .sct_data = sc8180x_data,
- .size = ARRAY_SIZE(sc8180x_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sc8180x_cfg[] = {
+ {
+ .sct_data = sc8180x_data,
+ .size = ARRAY_SIZE(sc8180x_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sc8280xp_cfg = {
- .sct_data = sc8280xp_data,
- .size = ARRAY_SIZE(sc8280xp_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sc8280xp_cfg[] = {
+ {
+ .sct_data = sc8280xp_data,
+ .size = ARRAY_SIZE(sc8280xp_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sdm845_cfg = {
- .sct_data = sdm845_data,
- .size = ARRAY_SIZE(sdm845_data),
- .need_llcc_cfg = false,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sdm845_cfg[] = {
+ {
+ .sct_data = sdm845_data,
+ .size = ARRAY_SIZE(sdm845_data),
+ .need_llcc_cfg = false,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sm6350_cfg = {
- .sct_data = sm6350_data,
- .size = ARRAY_SIZE(sm6350_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sm6350_cfg[] = {
+ {
+ .sct_data = sm6350_data,
+ .size = ARRAY_SIZE(sm6350_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sm8150_cfg = {
- .sct_data = sm8150_data,
- .size = ARRAY_SIZE(sm8150_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sm8150_cfg[] = {
+ {
+ .sct_data = sm8150_data,
+ .size = ARRAY_SIZE(sm8150_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sm8250_cfg = {
- .sct_data = sm8250_data,
- .size = ARRAY_SIZE(sm8250_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sm8250_cfg[] = {
+ {
+ .sct_data = sm8250_data,
+ .size = ARRAY_SIZE(sm8250_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sm8350_cfg = {
- .sct_data = sm8350_data,
- .size = ARRAY_SIZE(sm8350_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v1_reg_offset,
- .edac_reg_offset = &llcc_v1_edac_reg_offset,
+static const struct qcom_llcc_config sm8350_cfg[] = {
+ {
+ .sct_data = sm8350_data,
+ .size = ARRAY_SIZE(sm8350_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v1_reg_offset,
+ .edac_reg_offset = &llcc_v1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sm8450_cfg = {
- .sct_data = sm8450_data,
- .size = ARRAY_SIZE(sm8450_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v2_1_reg_offset,
- .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+static const struct qcom_llcc_config sm8450_cfg[] = {
+ {
+ .sct_data = sm8450_data,
+ .size = ARRAY_SIZE(sm8450_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v2_1_reg_offset,
+ .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+ },
+ { },
};

-static const struct qcom_llcc_config sm8550_cfg = {
- .sct_data = sm8550_data,
- .size = ARRAY_SIZE(sm8550_data),
- .need_llcc_cfg = true,
- .reg_offset = llcc_v2_1_reg_offset,
- .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+static const struct qcom_llcc_config sm8550_cfg[] = {
+ {
+ .sct_data = sm8550_data,
+ .size = ARRAY_SIZE(sm8550_data),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v2_1_reg_offset,
+ .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+ },
+ { },
};

static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
@@ -966,8 +999,8 @@ static int qcom_llcc_probe(struct platform_device *pdev)
num_banks >>= LLCC_LB_CNT_SHIFT;
drv_data->num_banks = num_banks;

- llcc_cfg = cfg->sct_data;
- sz = cfg->size;
+ llcc_cfg = cfg[0].sct_data;
+ sz = cfg[0].size;

for (i = 0; i < sz; i++)
if (llcc_cfg[i].slice_id > drv_data->max_slices)
@@ -1016,17 +1049,17 @@ static int qcom_llcc_probe(struct platform_device *pdev)
}

static const struct of_device_id qcom_llcc_of_match[] = {
- { .compatible = "qcom,sc7180-llcc", .data = &sc7180_cfg },
- { .compatible = "qcom,sc7280-llcc", .data = &sc7280_cfg },
- { .compatible = "qcom,sc8180x-llcc", .data = &sc8180x_cfg },
- { .compatible = "qcom,sc8280xp-llcc", .data = &sc8280xp_cfg },
- { .compatible = "qcom,sdm845-llcc", .data = &sdm845_cfg },
- { .compatible = "qcom,sm6350-llcc", .data = &sm6350_cfg },
- { .compatible = "qcom,sm8150-llcc", .data = &sm8150_cfg },
- { .compatible = "qcom,sm8250-llcc", .data = &sm8250_cfg },
- { .compatible = "qcom,sm8350-llcc", .data = &sm8350_cfg },
- { .compatible = "qcom,sm8450-llcc", .data = &sm8450_cfg },
- { .compatible = "qcom,sm8550-llcc", .data = &sm8550_cfg },
+ { .compatible = "qcom,sc7180-llcc", .data = sc7180_cfg },
+ { .compatible = "qcom,sc7280-llcc", .data = sc7280_cfg },
+ { .compatible = "qcom,sc8180x-llcc", .data = sc8180x_cfg },
+ { .compatible = "qcom,sc8280xp-llcc", .data = sc8280xp_cfg },
+ { .compatible = "qcom,sdm845-llcc", .data = sdm845_cfg },
+ { .compatible = "qcom,sm6350-llcc", .data = sm6350_cfg },
+ { .compatible = "qcom,sm8150-llcc", .data = sm8150_cfg },
+ { .compatible = "qcom,sm8250-llcc", .data = sm8250_cfg },
+ { .compatible = "qcom,sm8350-llcc", .data = sm8350_cfg },
+ { .compatible = "qcom,sm8450-llcc", .data = sm8450_cfg },
+ { .compatible = "qcom,sm8550-llcc", .data = sm8550_cfg },
{ }
};
MODULE_DEVICE_TABLE(of, qcom_llcc_of_match);
--
2.39.1


2023-03-13 12:41:35

by Komal Bajaj

[permalink] [raw]
Subject: [PATCH v2 5/5] soc: qcom: llcc: Add QDU1000 and QRU1000 LLCC support

Add LLCC configuration data for QDU1000 and QRU1000 SoCs
and updating macro name for LLCC_DRE to LLCC_ECC as per
the latest specification.

Signed-off-by: Komal Bajaj <[email protected]>
---
drivers/soc/qcom/llcc-qcom.c | 65 +++++++++++++++++++++++++++++-
include/linux/soc/qcom/llcc-qcom.h | 2 +-
2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index f4d3e266c629..2dd9fc4e2858 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -188,7 +188,7 @@ static const struct llcc_slice_config sc8280xp_data[] = {
{ LLCC_MMUHWT, 13, 1024, 1, 1, 0xfff, 0x0, 0, 0, 0, 0, 1, 0 },
{ LLCC_DISP, 16, 6144, 1, 1, 0xfff, 0x0, 0, 0, 0, 1, 0, 0 },
{ LLCC_AUDHW, 22, 2048, 1, 1, 0xfff, 0x0, 0, 0, 0, 1, 0, 0 },
- { LLCC_DRE, 26, 1024, 1, 1, 0xfff, 0x0, 0, 0, 0, 1, 0, 0 },
+ { LLCC_ECC, 26, 1024, 1, 1, 0xfff, 0x0, 0, 0, 0, 1, 0, 0 },
{ LLCC_CVP, 28, 512, 3, 1, 0xfff, 0x0, 0, 0, 0, 1, 0, 0 },
{ LLCC_APTCM, 30, 1024, 3, 1, 0x0, 0x1, 1, 0, 0, 1, 0, 0 },
{ LLCC_WRCACHE, 31, 1024, 1, 1, 0xfff, 0x0, 0, 0, 0, 0, 1, 0 },
@@ -351,6 +351,36 @@ static const struct llcc_slice_config sm8550_data[] = {
{LLCC_VIDVSP, 28, 256, 4, 1, 0xFFFFFF, 0x0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, },
};

+static const struct llcc_slice_config qdu1000_data_2ch[] = {
+ {LLCC_MDMHPGRW, 7, 512, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_MODHW, 9, 256, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_MDMPNG, 21, 256, 0, 1, 0x3, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_ECC, 26, 512, 3, 1, 0xFFC, 0x0, 0, 0, 0, 0, 1, 0, 0 },
+ {LLCC_MODPE, 29, 256, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_APTCM, 30, 256, 3, 1, 0x0, 0xC, 1, 0, 0, 1, 0, 0, 0 },
+ {LLCC_WRCACHE, 31, 128, 1, 1, 0x3, 0x0, 0, 0, 0, 0, 1, 0, 0 },
+};
+
+static const struct llcc_slice_config qdu1000_data_4ch[] = {
+ {LLCC_MDMHPGRW, 7, 1024, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_MODHW, 9, 512, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_MDMPNG, 21, 512, 0, 1, 0x3, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_ECC, 26, 1024, 3, 1, 0xFFC, 0x0, 0, 0, 0, 0, 1, 0, 0 },
+ {LLCC_MODPE, 29, 512, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_APTCM, 30, 512, 3, 1, 0x0, 0xC, 1, 0, 0, 1, 0, 0, 0 },
+ {LLCC_WRCACHE, 31, 256, 1, 1, 0x3, 0x0, 0, 0, 0, 0, 1, 0, 0 },
+};
+
+static const struct llcc_slice_config qdu1000_data_8ch[] = {
+ {LLCC_MDMHPGRW, 7, 2048, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_MODHW, 9, 1024, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_MDMPNG, 21, 1024, 0, 1, 0x3, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_ECC, 26, 2048, 3, 1, 0xFFC, 0x0, 0, 0, 0, 0, 1, 0, 0 },
+ {LLCC_MODPE, 29, 1024, 1, 1, 0xFFF, 0x0, 0, 0, 0, 1, 0, 0, 0 },
+ {LLCC_APTCM, 30, 1024, 3, 1, 0x0, 0xC, 1, 0, 0, 1, 0, 0, 0 },
+ {LLCC_WRCACHE, 31, 512, 1, 1, 0x3, 0x0, 0, 0, 0, 0, 1, 0, 0 },
+};
+
static const struct llcc_edac_reg_offset llcc_v1_edac_reg_offset = {
.trp_ecc_error_status0 = 0x20344,
.trp_ecc_error_status1 = 0x20348,
@@ -538,6 +568,38 @@ static const struct qcom_llcc_config sm8550_cfg[] = {
{ },
};

+static const struct qcom_llcc_config qdu1000_cfg[] = {
+ {
+ .sct_data = qdu1000_data_8ch,
+ .size = ARRAY_SIZE(qdu1000_data_8ch),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v2_1_reg_offset,
+ .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+ },
+ {
+ .sct_data = qdu1000_data_4ch,
+ .size = ARRAY_SIZE(qdu1000_data_4ch),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v2_1_reg_offset,
+ .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+ },
+ {
+ .sct_data = qdu1000_data_4ch,
+ .size = ARRAY_SIZE(qdu1000_data_4ch),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v2_1_reg_offset,
+ .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+ },
+ {
+ .sct_data = qdu1000_data_2ch,
+ .size = ARRAY_SIZE(qdu1000_data_2ch),
+ .need_llcc_cfg = true,
+ .reg_offset = llcc_v2_1_reg_offset,
+ .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
+ },
+ { },
+};
+
static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;

/**
@@ -1110,6 +1172,7 @@ static const struct of_device_id qcom_llcc_of_match[] = {
{ .compatible = "qcom,sm8350-llcc", .data = sm8350_cfg },
{ .compatible = "qcom,sm8450-llcc", .data = sm8450_cfg },
{ .compatible = "qcom,sm8550-llcc", .data = sm8550_cfg },
+ { .compatible = "qcom,qdu1000-llcc", .data = qdu1000_cfg},
{ }
};
MODULE_DEVICE_TABLE(of, qcom_llcc_of_match);
diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
index 225891a02f5d..150b2836c8b9 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -30,7 +30,7 @@
#define LLCC_NPU 23
#define LLCC_WLHW 24
#define LLCC_PIMEM 25
-#define LLCC_DRE 26
+#define LLCC_ECC 26
#define LLCC_CVP 28
#define LLCC_MODPE 29
#define LLCC_APTCM 30
--
2.39.1


2023-03-13 12:41:38

by Komal Bajaj

[permalink] [raw]
Subject: [PATCH v2 4/5] soc: qcom: Add LLCC support for multi channel DDR

Add LLCC support for multi channel DDR configurations
based off of a feature register.

Signed-off-by: Komal Bajaj <[email protected]>
---
drivers/soc/qcom/llcc-qcom.c | 56 ++++++++++++++++++++++++++++--
include/linux/soc/qcom/llcc-qcom.h | 2 ++
2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 00699a0c047e..f4d3e266c629 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -17,6 +17,7 @@
#include <linux/regmap.h>
#include <linux/sizes.h>
#include <linux/slab.h>
+#include <linux/firmware/qcom/qcom_scm.h>
#include <linux/soc/qcom/llcc-qcom.h>

#define ACTIVATE BIT(0)
@@ -924,6 +925,40 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev,
return ret;
}

+static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u32 *cfg_index)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *ch_res = NULL;
+
+ u32 ch_reg_sz;
+ u32 ch_reg_off;
+ u32 val;
+ int ret = 0;
+
+ ch_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "multi_channel_register");
+ if (ch_res) {
+ if (of_property_read_u32(dev->of_node, "multi-ch-bit-off", &ch_reg_off)) {
+ dev_err(&pdev->dev,
+ "Couldn't get offset for multi channel feature register\n");
+ return -ENODEV;
+ }
+ if (of_property_read_u32_index(dev->of_node, "multi-ch-bit-off", 1, &ch_reg_sz)) {
+ dev_err(&pdev->dev,
+ "Couldn't get size of multi channel feature register\n");
+ return -ENODEV;
+ }
+
+ if (qcom_scm_io_readl(ch_res->start, &val)) {
+ dev_err(&pdev->dev, "Couldn't access multi channel feature register\n");
+ ret = -EINVAL;
+ }
+ *cfg_index = (val >> ch_reg_off) & ((1 << ch_reg_sz) - 1);
+ } else
+ *cfg_index = 0;
+
+ return ret;
+}
+
static int qcom_llcc_remove(struct platform_device *pdev)
{
/* Set the global pointer to a error code to avoid referencing it */
@@ -956,10 +991,13 @@ static int qcom_llcc_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
int ret, i;
struct platform_device *llcc_edac;
- const struct qcom_llcc_config *cfg;
+ const struct qcom_llcc_config *cfg, *entry;
const struct llcc_slice_config *llcc_cfg;
+
u32 sz;
+ u32 cfg_index;
u32 version;
+ u32 no_of_entries = 0;

drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
if (!drv_data) {
@@ -999,8 +1037,20 @@ static int qcom_llcc_probe(struct platform_device *pdev)
num_banks >>= LLCC_LB_CNT_SHIFT;
drv_data->num_banks = num_banks;

- llcc_cfg = cfg[0].sct_data;
- sz = cfg[0].size;
+ ret = qcom_llcc_get_cfg_index(pdev, &cfg_index);
+ if (ret)
+ goto err;
+
+ for (entry = cfg; entry->sct_data; entry++, no_of_entries++)
+ ;
+ if (cfg_index >= no_of_entries) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ drv_data->cfg_index = cfg_index;
+ llcc_cfg = cfg[cfg_index].sct_data;
+ sz = cfg[cfg_index].size;

for (i = 0; i < sz; i++)
if (llcc_cfg[i].slice_id > drv_data->max_slices)
diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
index ad1fd718169d..225891a02f5d 100644
--- a/include/linux/soc/qcom/llcc-qcom.h
+++ b/include/linux/soc/qcom/llcc-qcom.h
@@ -125,6 +125,7 @@ struct llcc_edac_reg_offset {
* @cfg: pointer to the data structure for slice configuration
* @edac_reg_offset: Offset of the LLCC EDAC registers
* @lock: mutex associated with each slice
+ * @cfg_index: index of config table if multiple configs present for a target
* @cfg_size: size of the config data table
* @max_slices: max slices as read from device tree
* @num_banks: Number of llcc banks
@@ -139,6 +140,7 @@ struct llcc_drv_data {
const struct llcc_slice_config *cfg;
const struct llcc_edac_reg_offset *edac_reg_offset;
struct mutex lock;
+ u32 cfg_index;
u32 cfg_size;
u32 max_slices;
u32 num_banks;
--
2.39.1


2023-03-14 14:10:52

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC


On Mon, 13 Mar 2023 18:10:37 +0530, Komal Bajaj wrote:
> Add description for additional nodes needed to support
> mulitple channel DDR configurations in LLCC.
>
> Signed-off-by: Komal Bajaj <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/msm/qcom,llcc.example.dtb: system-cache-controller@1100000: reg: [[17825792, 2097152], [19922944, 327680]] is too short
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/msm/qcom,llcc.example.dtb: system-cache-controller@1100000: reg-names: ['llcc_base', 'llcc_broadcast_base'] is too short
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2023-03-14 23:24:03

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] soc: qcom: Add LLCC support for multi channel DDR

Hi Komal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.3-rc2 next-20230314]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Komal-Bajaj/soc-qcom-llcc-Refactor-llcc-driver-to-support-multiple-configuration/20230313-204310
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230313124040.9463-5-quic_kbajaj%40quicinc.com
patch subject: [PATCH v2 4/5] soc: qcom: Add LLCC support for multi channel DDR
config: mips-randconfig-r021-20230312 (https://download.01.org/0day-ci/archive/20230315/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/fe14182092383680f24bc88d81d199261453fa71
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Komal-Bajaj/soc-qcom-llcc-Refactor-llcc-driver-to-support-multiple-configuration/20230313-204310
git checkout fe14182092383680f24bc88d81d199261453fa71
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "qcom_scm_io_readl" [drivers/soc/qcom/llcc-qcom.ko] undefined!

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-03-15 07:37:42

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] soc: qcom: Add LLCC support for multi channel DDR

On 13/03/2023 13:40, Komal Bajaj wrote:
> Add LLCC support for multi channel DDR configurations
> based off of a feature register.
>
> Signed-off-by: Komal Bajaj <[email protected]>
> ---
> drivers/soc/qcom/llcc-qcom.c | 56 ++++++++++++++++++++++++++++--

kernel robot still reports some issues here. You might be missing some
dependency or symbol export.

Best regards,
Krzysztof


2023-03-15 07:42:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC

On 13/03/2023 13:40, Komal Bajaj wrote:
> Add description for additional nodes needed to support
> mulitple channel DDR configurations in LLCC.
>
> Signed-off-by: Komal Bajaj <[email protected]>

+Cc Mani,

This will conflict with:
https://lore.kernel.org/all/[email protected]/

Please rebase on top of Mani's patches (assuming they are not
conflicting in principle)

> ---
> Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> index 38efcad56dbd..9a4a76caf490 100644
> --- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> @@ -37,15 +37,24 @@ properties:
> items:

minItems: 2

> - description: LLCC base register region
> - description: LLCC broadcast base register region
> + - description: Feature register to decide which LLCC configuration
> + to use, this is optional
>
> reg-names:

minItems: 2

> items:
> - const: llcc_base
> - const: llcc_broadcast_base
> + - const: multi_channel_register
>
> interrupts:
> maxItems: 1
>
> + multi-ch-bit-off:
> + items:
> + - description: Specifies the offset in bits into the multi_channel_register
> + and the number of bits used to decide which LLCC configuration
> + to use

There are here few issues.
First, I don't fully understand the property. What is an LLCC
configuration? Like some fused values?

Second, don't make it a register specific, it will not scale easily to
any new version of this interface. Although how this should look like
depends on what is it.

Third, you need vendor prefix and type (unless this is a generic
property, but does not look like). Then "items" is probably wrong. Line
break after "description: "

Best regards,
Krzysztof


2023-03-15 13:49:13

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC

On Wed, Mar 15, 2023 at 08:41:21AM +0100, Krzysztof Kozlowski wrote:
> On 13/03/2023 13:40, Komal Bajaj wrote:
> > Add description for additional nodes needed to support
> > mulitple channel DDR configurations in LLCC.
> >
> > Signed-off-by: Komal Bajaj <[email protected]>
>
> +Cc Mani,
>

Thanks, Krzysztof!

> This will conflict with:
> https://lore.kernel.org/all/[email protected]/
>
> Please rebase on top of Mani's patches (assuming they are not
> conflicting in principle)
>
> > ---
> > Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> > index 38efcad56dbd..9a4a76caf490 100644
> > --- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> > +++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> > @@ -37,15 +37,24 @@ properties:
> > items:
>
> minItems: 2
>
> > - description: LLCC base register region
> > - description: LLCC broadcast base register region
> > + - description: Feature register to decide which LLCC configuration
> > + to use, this is optional
> >
> > reg-names:
>
> minItems: 2
>
> > items:
> > - const: llcc_base
> > - const: llcc_broadcast_base
> > + - const: multi_channel_register

Is this the actual register region or a specific register offset? We generally
try to pass the base address of the region along with the size and use the
offset inside the driver to access any specific registers.

Thanks,
Mani

> >
> > interrupts:
> > maxItems: 1
> >
> > + multi-ch-bit-off:
> > + items:
> > + - description: Specifies the offset in bits into the multi_channel_register
> > + and the number of bits used to decide which LLCC configuration
> > + to use
>
> There are here few issues.
> First, I don't fully understand the property. What is an LLCC
> configuration? Like some fused values?
>
> Second, don't make it a register specific, it will not scale easily to
> any new version of this interface. Although how this should look like
> depends on what is it.
>
> Third, you need vendor prefix and type (unless this is a generic
> property, but does not look like). Then "items" is probably wrong. Line
> break after "description: "
>
> Best regards,
> Krzysztof
>

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

2023-03-15 14:08:25

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] soc: qcom: Add LLCC support for multi channel DDR

On Mon, Mar 13, 2023 at 06:10:39PM +0530, Komal Bajaj wrote:
> Add LLCC support for multi channel DDR configurations
> based off of a feature register.
>

Please elaborate more in the commit message on why this patch is needed and how
it is implemented.

> Signed-off-by: Komal Bajaj <[email protected]>
> ---
> drivers/soc/qcom/llcc-qcom.c | 56 ++++++++++++++++++++++++++++--
> include/linux/soc/qcom/llcc-qcom.h | 2 ++
> 2 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 00699a0c047e..f4d3e266c629 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -17,6 +17,7 @@
> #include <linux/regmap.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> +#include <linux/firmware/qcom/qcom_scm.h>

Sort the includes alphabetically

> #include <linux/soc/qcom/llcc-qcom.h>
>
> #define ACTIVATE BIT(0)
> @@ -924,6 +925,40 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev,
> return ret;
> }
>
> +static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u32 *cfg_index)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *ch_res = NULL;

No need to initialize the pointer

> +

No need of a newline

> + u32 ch_reg_sz;
> + u32 ch_reg_off;
> + u32 val;
> + int ret = 0;
> +
> + ch_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "multi_channel_register");
> + if (ch_res) {
> + if (of_property_read_u32(dev->of_node, "multi-ch-bit-off", &ch_reg_off)) {
> + dev_err(&pdev->dev,
> + "Couldn't get offset for multi channel feature register\n");
> + return -ENODEV;
> + }
> + if (of_property_read_u32_index(dev->of_node, "multi-ch-bit-off", 1, &ch_reg_sz)) {
> + dev_err(&pdev->dev,
> + "Couldn't get size of multi channel feature register\n");
> + return -ENODEV;
> + }
> +
> + if (qcom_scm_io_readl(ch_res->start, &val)) {

You didn't mention this SCM call in the commit message. Also, for SCM, you need
to select the driver in Kconfig.

> + dev_err(&pdev->dev, "Couldn't access multi channel feature register\n");
> + ret = -EINVAL;

Catch the actual error no from qcom_scm_io_readl().

So in the case of failure, you still want to calculate cfg_index?

> + }
> + *cfg_index = (val >> ch_reg_off) & ((1 << ch_reg_sz) - 1);
> + } else

Use braces for else condition

> + *cfg_index = 0;
> +
> + return ret;
> +}
> +
> static int qcom_llcc_remove(struct platform_device *pdev)
> {
> /* Set the global pointer to a error code to avoid referencing it */
> @@ -956,10 +991,13 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> int ret, i;
> struct platform_device *llcc_edac;
> - const struct qcom_llcc_config *cfg;
> + const struct qcom_llcc_config *cfg, *entry;
> const struct llcc_slice_config *llcc_cfg;
> +

No need of newline

> u32 sz;
> + u32 cfg_index;
> u32 version;
> + u32 no_of_entries = 0;

num_entries?

>
> drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
> if (!drv_data) {
> @@ -999,8 +1037,20 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> num_banks >>= LLCC_LB_CNT_SHIFT;
> drv_data->num_banks = num_banks;
>
> - llcc_cfg = cfg[0].sct_data;
> - sz = cfg[0].size;
> + ret = qcom_llcc_get_cfg_index(pdev, &cfg_index);
> + if (ret)
> + goto err;
> +
> + for (entry = cfg; entry->sct_data; entry++, no_of_entries++)
> + ;

Wrap ; in the above line itself

> + if (cfg_index >= no_of_entries) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + drv_data->cfg_index = cfg_index;

Where is this cached value used?

Thanks,
Mani

> + llcc_cfg = cfg[cfg_index].sct_data;
> + sz = cfg[cfg_index].size;
>
> for (i = 0; i < sz; i++)
> if (llcc_cfg[i].slice_id > drv_data->max_slices)
> diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
> index ad1fd718169d..225891a02f5d 100644
> --- a/include/linux/soc/qcom/llcc-qcom.h
> +++ b/include/linux/soc/qcom/llcc-qcom.h
> @@ -125,6 +125,7 @@ struct llcc_edac_reg_offset {
> * @cfg: pointer to the data structure for slice configuration
> * @edac_reg_offset: Offset of the LLCC EDAC registers
> * @lock: mutex associated with each slice
> + * @cfg_index: index of config table if multiple configs present for a target
> * @cfg_size: size of the config data table
> * @max_slices: max slices as read from device tree
> * @num_banks: Number of llcc banks
> @@ -139,6 +140,7 @@ struct llcc_drv_data {
> const struct llcc_slice_config *cfg;
> const struct llcc_edac_reg_offset *edac_reg_offset;
> struct mutex lock;
> + u32 cfg_index;
> u32 cfg_size;
> u32 max_slices;
> u32 num_banks;
> --
> 2.39.1
>

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

2023-04-06 09:37:15

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC

On 06/04/2023 11:19, Komal Bajaj wrote:
>
>>>>
>>>> interrupts:
>>>> maxItems: 1
>>>>
>>>> + multi-ch-bit-off:
>>>> + items:
>>>> + - description: Specifies the offset in bits into the multi_channel_register
>>>> + and the number of bits used to decide which LLCC configuration
>>>> + to use
>>> There are here few issues.
>>> First, I don't fully understand the property. What is an LLCC
>>> configuration? Like some fused values?
>
> There are different configuration for LLCC based on the number of
> DDR channel it uses. Here, we are basically trying to get information
> about the same.
>
>>>
>>> Second, don't make it a register specific, it will not scale easily to
>>> any new version of this interface. Although how this should look like
>>> depends on what is it.
>
> LLCC driver can only get DDR channel information from the register.

And why would that exactly matter to DT? How does it solve my concern
that your approach does not scale?

Best regards,
Krzysztof

2023-04-06 10:01:41

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC



On 4/6/2023 2:49 PM, Komal Bajaj wrote:
> Didn't see my reply on the list, so sending it again.
> And also I see that the dt patch is already applied.

The reason why you are not seeing your replies at

https://lore.kernel.org/lkml/[email protected]/

is because your reply cc-list contain some invalid domain
(codeaurora.org) email id's and any list/email mentioned
after that would not be getting your emails.

-- Mukesh

>
> Thanks Krzysztof and Manivannan for reviewing the patch.
>
>
> On 3/15/2023 7:18 PM, Manivannan Sadhasivam wrote:
>> On Wed, Mar 15, 2023 at 08:41:21AM +0100, Krzysztof Kozlowski wrote:
>>> On 13/03/2023 13:40, Komal Bajaj wrote:
>>>> Add description for additional nodes needed to support
>>>> mulitple channel DDR configurations in LLCC.
>>>>
>>>> Signed-off-by: Komal Bajaj<[email protected]>
>>> +Cc Mani,
>>>
>> Thanks, Krzysztof!
>>
>>> This will conflict with:
>>> https://lore.kernel.org/all/[email protected]/
>>>
>>> Please rebase on top of Mani's patches (assuming they are not
>>> conflicting in principle)
>>>
>>>> ---
>>>> Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
>>>> index 38efcad56dbd..9a4a76caf490 100644
>>>> --- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
>>>> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
>>>> @@ -37,15 +37,24 @@ properties:
>>>> items:
>>> minItems: 2
>>>
>>>> - description: LLCC base register region
>>>> - description: LLCC broadcast base register region
>>>> + - description: Feature register to decide which LLCC configuration
>>>> + to use, this is optional
>>>>
>>>> reg-names:
>>> minItems: 2
>>>
>>>> items:
>>>> - const: llcc_base
>>>> - const: llcc_broadcast_base
>>>> + - const: multi_channel_register
>> Is this the actual register region or a specific register offset? We generally
>> try to pass the base address of the region along with the size and use the
>> offset inside the driver to access any specific registers.
>>
>> Thanks,
>> Mani
>
> This is a specific register offset outside the LLCC register region which has the
> information of number of DDR channel.
>
>>>>
>>>> interrupts:
>>>> maxItems: 1
>>>>
>>>> + multi-ch-bit-off:
>>>> + items:
>>>> + - description: Specifies the offset in bits into the multi_channel_register
>>>> + and the number of bits used to decide which LLCC configuration
>>>> + to use
>>> There are here few issues.
>>> First, I don't fully understand the property. What is an LLCC
>>> configuration? Like some fused values?
>
> There are different configuration for LLCC based on the number of
> DDR channel it uses. Here, we are basically trying to get information
> about the same.
>
>>> Second, don't make it a register specific, it will not scale easily to
>>> any new version of this interface. Although how this should look like
>>> depends on what is it.
>
> LLCC driver can only get DDR channel information from the register.
>
>>> Third, you need vendor prefix and type (unless this is a generic
>>> property, but does not look like). Then "items" is probably wrong. Line
>>> break after "description: "
>
> Noted, will take care of this in the next patchset.
>
> Thanks
> Komal
>
>>> Best regards,
>>> Krzysztof
>>>
>

2023-04-06 11:14:50

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC



On 4/6/2023 3:04 PM, Krzysztof Kozlowski wrote:
> On 06/04/2023 11:19, Komal Bajaj wrote:
>>
>>>>>
>>>>> interrupts:
>>>>> maxItems: 1
>>>>>
>>>>> + multi-ch-bit-off:
>>>>> + items:
>>>>> + - description: Specifies the offset in bits into the multi_channel_register
>>>>> + and the number of bits used to decide which LLCC configuration
>>>>> + to use
>>>> There are here few issues.
>>>> First, I don't fully understand the property. What is an LLCC
>>>> configuration? Like some fused values?
>>
>> There are different configuration for LLCC based on the number of
>> DDR channel it uses. Here, we are basically trying to get information
>> about the same.
>>
>>>>
>>>> Second, don't make it a register specific, it will not scale easily to
>>>> any new version of this interface. Although how this should look like
>>>> depends on what is it.
>>
>> LLCC driver can only get DDR channel information from the register.

Actually, any one can read this property there is no boundation to that.
However, some of the bits information of this register only matters to
LLCC to make decision.

> And why would that exactly matter to DT? How does it solve my concern
> that your approach does not scale?

I understand your concern about not making it register specific, however
this register is a secure fuse register where the interested bits are
known to us and should only be used by llcc to select right slice
configuration table to apply.

How about something like this,

Add another property like qcom,tcsr_feature_config under qcom,scm
and read this property under qcom_scm driver and expose
read exported symbol for LLCC driver. LLCC driver can use API and
apply bit offset/bit size to get right value inside the target driver
data(e.g .data = &XXXX_cfg }) or maintain it inside same
tcsr_feature_config DT (this can be discussed)
Also, we can have a yaml file for tcsr_feature_config.

e.g..


scm: scm {
compatible = "qcom,scm-sm8450", "qcom,scm";
qcom,dload-mode = <&tcsr 0x13000>;
...
qcom,tcsr_feature_config = <&tcsr_feature_config>
};

tcsr_feature_config: syscon@fd484000 {
compatible = "qcom, XXXX", qcom,tcsr-featureconfig";
reg = <0xXXXXXXXX 0xYYYY>;
};


-- Mukesh

>
> Best regards,
> Krzysztof
>

2023-04-07 05:53:13

by Abel Vesa

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] soc: qcom: llcc: Refactor llcc driver to support multiple configuration

On 23-03-13 18:10:36, Komal Bajaj wrote:
> Refactor driver to support multiple configuration for llcc on a target.
>
> Signed-off-by: Komal Bajaj <[email protected]>

LGTM.

Reviewed-by: Abel Vesa <[email protected]>

> ---
> drivers/soc/qcom/llcc-qcom.c | 191 ++++++++++++++++++++---------------
> 1 file changed, 112 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 23ce2f78c4ed..00699a0c047e 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -416,92 +416,125 @@ static const u32 llcc_v2_1_reg_offset[] = {
> [LLCC_COMMON_STATUS0] = 0x0003400c,
> };
>
> -static const struct qcom_llcc_config sc7180_cfg = {
> - .sct_data = sc7180_data,
> - .size = ARRAY_SIZE(sc7180_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sc7180_cfg[] = {
> + {
> + .sct_data = sc7180_data,
> + .size = ARRAY_SIZE(sc7180_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sc7280_cfg = {
> - .sct_data = sc7280_data,
> - .size = ARRAY_SIZE(sc7280_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sc7280_cfg[] = {
> + {
> + .sct_data = sc7280_data,
> + .size = ARRAY_SIZE(sc7280_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sc8180x_cfg = {
> - .sct_data = sc8180x_data,
> - .size = ARRAY_SIZE(sc8180x_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sc8180x_cfg[] = {
> + {
> + .sct_data = sc8180x_data,
> + .size = ARRAY_SIZE(sc8180x_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sc8280xp_cfg = {
> - .sct_data = sc8280xp_data,
> - .size = ARRAY_SIZE(sc8280xp_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sc8280xp_cfg[] = {
> + {
> + .sct_data = sc8280xp_data,
> + .size = ARRAY_SIZE(sc8280xp_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sdm845_cfg = {
> - .sct_data = sdm845_data,
> - .size = ARRAY_SIZE(sdm845_data),
> - .need_llcc_cfg = false,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sdm845_cfg[] = {
> + {
> + .sct_data = sdm845_data,
> + .size = ARRAY_SIZE(sdm845_data),
> + .need_llcc_cfg = false,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sm6350_cfg = {
> - .sct_data = sm6350_data,
> - .size = ARRAY_SIZE(sm6350_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sm6350_cfg[] = {
> + {
> + .sct_data = sm6350_data,
> + .size = ARRAY_SIZE(sm6350_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sm8150_cfg = {
> - .sct_data = sm8150_data,
> - .size = ARRAY_SIZE(sm8150_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sm8150_cfg[] = {
> + {
> + .sct_data = sm8150_data,
> + .size = ARRAY_SIZE(sm8150_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sm8250_cfg = {
> - .sct_data = sm8250_data,
> - .size = ARRAY_SIZE(sm8250_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sm8250_cfg[] = {
> + {
> + .sct_data = sm8250_data,
> + .size = ARRAY_SIZE(sm8250_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sm8350_cfg = {
> - .sct_data = sm8350_data,
> - .size = ARRAY_SIZE(sm8350_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v1_reg_offset,
> - .edac_reg_offset = &llcc_v1_edac_reg_offset,
> +static const struct qcom_llcc_config sm8350_cfg[] = {
> + {
> + .sct_data = sm8350_data,
> + .size = ARRAY_SIZE(sm8350_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v1_reg_offset,
> + .edac_reg_offset = &llcc_v1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sm8450_cfg = {
> - .sct_data = sm8450_data,
> - .size = ARRAY_SIZE(sm8450_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v2_1_reg_offset,
> - .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
> +static const struct qcom_llcc_config sm8450_cfg[] = {
> + {
> + .sct_data = sm8450_data,
> + .size = ARRAY_SIZE(sm8450_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v2_1_reg_offset,
> + .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
> + },
> + { },
> };
>
> -static const struct qcom_llcc_config sm8550_cfg = {
> - .sct_data = sm8550_data,
> - .size = ARRAY_SIZE(sm8550_data),
> - .need_llcc_cfg = true,
> - .reg_offset = llcc_v2_1_reg_offset,
> - .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
> +static const struct qcom_llcc_config sm8550_cfg[] = {
> + {
> + .sct_data = sm8550_data,
> + .size = ARRAY_SIZE(sm8550_data),
> + .need_llcc_cfg = true,
> + .reg_offset = llcc_v2_1_reg_offset,
> + .edac_reg_offset = &llcc_v2_1_edac_reg_offset,
> + },
> + { },
> };
>
> static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
> @@ -966,8 +999,8 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> num_banks >>= LLCC_LB_CNT_SHIFT;
> drv_data->num_banks = num_banks;
>
> - llcc_cfg = cfg->sct_data;
> - sz = cfg->size;
> + llcc_cfg = cfg[0].sct_data;
> + sz = cfg[0].size;
>
> for (i = 0; i < sz; i++)
> if (llcc_cfg[i].slice_id > drv_data->max_slices)
> @@ -1016,17 +1049,17 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> }
>
> static const struct of_device_id qcom_llcc_of_match[] = {
> - { .compatible = "qcom,sc7180-llcc", .data = &sc7180_cfg },
> - { .compatible = "qcom,sc7280-llcc", .data = &sc7280_cfg },
> - { .compatible = "qcom,sc8180x-llcc", .data = &sc8180x_cfg },
> - { .compatible = "qcom,sc8280xp-llcc", .data = &sc8280xp_cfg },
> - { .compatible = "qcom,sdm845-llcc", .data = &sdm845_cfg },
> - { .compatible = "qcom,sm6350-llcc", .data = &sm6350_cfg },
> - { .compatible = "qcom,sm8150-llcc", .data = &sm8150_cfg },
> - { .compatible = "qcom,sm8250-llcc", .data = &sm8250_cfg },
> - { .compatible = "qcom,sm8350-llcc", .data = &sm8350_cfg },
> - { .compatible = "qcom,sm8450-llcc", .data = &sm8450_cfg },
> - { .compatible = "qcom,sm8550-llcc", .data = &sm8550_cfg },
> + { .compatible = "qcom,sc7180-llcc", .data = sc7180_cfg },
> + { .compatible = "qcom,sc7280-llcc", .data = sc7280_cfg },
> + { .compatible = "qcom,sc8180x-llcc", .data = sc8180x_cfg },
> + { .compatible = "qcom,sc8280xp-llcc", .data = sc8280xp_cfg },
> + { .compatible = "qcom,sdm845-llcc", .data = sdm845_cfg },
> + { .compatible = "qcom,sm6350-llcc", .data = sm6350_cfg },
> + { .compatible = "qcom,sm8150-llcc", .data = sm8150_cfg },
> + { .compatible = "qcom,sm8250-llcc", .data = sm8250_cfg },
> + { .compatible = "qcom,sm8350-llcc", .data = sm8350_cfg },
> + { .compatible = "qcom,sm8450-llcc", .data = sm8450_cfg },
> + { .compatible = "qcom,sm8550-llcc", .data = sm8550_cfg },
> { }
> };
> MODULE_DEVICE_TABLE(of, qcom_llcc_of_match);
> --
> 2.39.1
>