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

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

[Komal Bajaj]

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

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

[Komal Bajaj]

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

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 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

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

[Komal Bajaj]

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

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 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

[PATCH v2 3/5] dt-bindings: arm: msm: Add LLCC compatible for QDU1000/QRU1000

[Komal Bajaj]

Add LLCC compatible for QDU1000/QRU1000 SoCs.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
index 9a4a76caf490..afb1b84907e0 100644
--- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
@@ -32,6 +32,7 @@ properties:
       - qcom,sm8350-llcc
       - qcom,sm8450-llcc
       - qcom,sm8550-llcc
+      - qcom,qdu1000-llcc
 
   reg:
     items:
-- 
2.39.1

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

[Komal Bajaj]

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

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 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

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

[Komal Bajaj]

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 <quic_kbajaj@quicinc.com>
---
 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

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

[Rob Herring]

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 <quic_kbajaj@quicinc.com>
> ---
>  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/20230313124040.9463-3-quic_kbajaj@quicinc.com

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.

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

[kernel test robot]

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/202303150713.cMgW2Qnu-lkp@intel.com/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 <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303150713.cMgW2Qnu-lkp@intel.com/

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

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

[Krzysztof Kozlowski]

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 <quic_kbajaj@quicinc.com>
> ---
>  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

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

[Krzysztof Kozlowski]

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 <quic_kbajaj@quicinc.com>

+Cc Mani,

This will conflict with:
https://lore.kernel.org/all/20230314080443.64635-3-manivannan.sadhasivam@linaro.org/

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

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

[Manivannan Sadhasivam]

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 <quic_kbajaj@quicinc.com>
> 
> +Cc Mani,
> 

Thanks, Krzysztof!

> This will conflict with:
> https://lore.kernel.org/all/20230314080443.64635-3-manivannan.sadhasivam@linaro.org/
> 
> 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
> 

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

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

[Manivannan Sadhasivam]

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 <quic_kbajaj@quicinc.com>
> ---
>  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
> 

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

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

[Krzysztof Kozlowski]

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

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

[Mukesh Ojha]

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/20230313124040.9463-1-quic_kbajaj@quicinc.com/

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<quic_kbajaj@quicinc.com>
>>> +Cc Mani,
>>>
>> Thanks, Krzysztof!
>>
>>> This will conflict with:
>>> https://lore.kernel.org/all/20230314080443.64635-3-manivannan.sadhasivam@linaro.org/
>>>
>>> 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
>>>
> 

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

[Mukesh Ojha]

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
> 

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

[Abel Vesa]

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 <quic_kbajaj@quicinc.com>

LGTM.

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

> ---
>  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
>