[PATCH v3 00/10] soc: qcom: llcc: Add support for QDU1000/QRU1000

[PATCH v3 00/10] soc: qcom: llcc: Add support for QDU1000/QRU1000

[Komal Bajaj]

This patch series does the following -
 * Add support for reading secure fuse region in qfprom driver
 * Add support for qfprom on QDU1000 and QRU1000 SoCs
 * Refactor LLCC driver to support multiple configuration
 * Add support for multi channel DDR configuration in LLCC
 * Add LLCC support for the Qualcomm QDU1000 and QRU1000 SoCs

Changes in v3-
 - Addressed comments from Krzysztof and Mani.
 - Using qfprom to read DDR configuration from feature register.

Changes in v2:
  - Addressing comments from Konrad.

Komal Bajaj (10):
  nvmem: qfprom: Add support for secure reading
  dt-bindings: nvmem: qfprom: Add compatible for QDU1000/QRU1000
  arm64: dts: qcom: qdu1000: Add properties to qfprom for multi channel
    DDR
  nvmem: qfprom: Add support for secure reading on QDU1000/QRU1000
  soc: qcom: llcc: Refactor llcc driver to support multiple
    configuration
  soc: qcom: Add LLCC support for multi channel DDR
  dt-bindings: arm: msm: Add LLCC compatible for QDU1000/QRU1000
  Revert "arm64: dts: qcom: qdu1000: Add LLCC/system-cache-controller"
  arm64: dts: qcom: qdu1000: Add LLCC/system-cache-controller
  soc: qcom: llcc: Add QDU1000 and QRU1000 LLCC support

 .../devicetree/bindings/cache/qcom,llcc.yaml  |   2 +
 .../bindings/nvmem/qcom,qfprom.yaml           |   1 +
 arch/arm64/boot/dts/qcom/qdu1000.dtsi         |  30 +-
 drivers/nvmem/qfprom.c                        |  74 ++++-
 drivers/soc/qcom/llcc-qcom.c                  | 304 +++++++++++++-----
 include/linux/soc/qcom/llcc-qcom.h            |   2 +-
 6 files changed, 301 insertions(+), 112 deletions(-)

-- 
2.17.1

[PATCH v3 01/10] nvmem: qfprom: Add support for secure reading

[Komal Bajaj]

For some of the Qualcomm SoC's, it is possible that
some of the fuse regions or entire qfprom region is
protected from non-secure access. In such situations,
linux will have to use secure calls to read the region.
With that motivation, add the support of reading secure
regions in qfprom driver. Ensuring the address to read
is word aligned since our secure I/O only supports word
size I/O.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 drivers/nvmem/Kconfig  |  1 +
 drivers/nvmem/qfprom.c | 69 +++++++++++++++++++++++++++++++++---------
 2 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index b291b27048c7..3d896ba29b89 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -209,6 +209,7 @@ config NVMEM_QCOM_QFPROM
 	tristate "QCOM QFPROM Support"
 	depends on ARCH_QCOM || COMPILE_TEST
 	depends on HAS_IOMEM
+	select QCOM_SCM
 	help
 	  Say y here to enable QFPROM support. The QFPROM provides access
 	  functions for QFPROM data to rest of the drivers via nvmem interface.
diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index c1e893c8a247..20662e2d3732 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -16,6 +16,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
+#include <linux/firmware/qcom/qcom_scm.h>

 /* Blow timer clock frequency in Mhz */
 #define QFPROM_BLOW_TIMER_OFFSET 0x03c
@@ -59,21 +60,22 @@ struct qfprom_soc_data {
 /**
  * struct qfprom_priv - structure holding qfprom attributes
  *
- * @qfpraw:       iomapped memory space for qfprom-efuse raw address space.
- * @qfpconf:      iomapped memory space for qfprom-efuse configuration address
- *                space.
+ * @qfpraw: iomapped memory space for qfprom-efuse raw address space.
+ * @qfpconf: iomapped memory space for qfprom-efuse configuration address space.
  * @qfpcorrected: iomapped memory space for qfprom corrected address space.
- * @qfpsecurity:  iomapped memory space for qfprom security control space.
- * @dev:          qfprom device structure.
- * @secclk:       Clock supply.
- * @vcc:          Regulator supply.
- * @soc_data:     Data that for things that varies from SoC to SoC.
+ * @qfpsecurity: iomapped memory space for qfprom security control space.
+ * @qfpseccorrected: starting physical address for qfprom secure corrected address space.
+ * @dev: qfprom device structure.
+ * @secclk: Clock supply.
+ * @vcc: Regulator supply.
+ * @soc_data: Data that for things that varies from SoC to SoC.
  */
 struct qfprom_priv {
 	void __iomem *qfpraw;
 	void __iomem *qfpconf;
 	void __iomem *qfpcorrected;
 	void __iomem *qfpsecurity;
+	phys_addr_t qfpseccorrected;
 	struct device *dev;
 	struct clk *secclk;
 	struct regulator *vcc;
@@ -99,10 +101,12 @@ struct qfprom_touched_values {
  *
  * @keepout: Array of keepout regions for this SoC.
  * @nkeepout: Number of elements in the keepout array.
+ * @secure: Is qfprom region for this SoC protected from non-secure access.
  */
 struct qfprom_soc_compatible_data {
 	const struct nvmem_keepout *keepout;
 	unsigned int nkeepout;
+	bool secure;
 };

 static const struct nvmem_keepout sc7180_qfprom_keepout[] = {
@@ -334,6 +338,34 @@ static int qfprom_reg_read(void *context,
 	return 0;
 }

+static int qfprom_sec_reg_read(void *context, unsigned int reg, void *_val, size_t bytes)
+{
+	struct qfprom_priv *priv = context;
+	u8 *val = _val;
+	int buf_start, buf_end, index, i = 0;
+	char *buffer;
+	u32 read_val;
+
+	buf_start = ALIGN_DOWN(reg, 4);
+	buf_end = ALIGN(reg + bytes, 4);
+	buffer = kzalloc(buf_end - buf_start, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+	for (index = buf_start; index < buf_end; index += 4, i += 4) {
+		if (qcom_scm_io_readl(priv->qfpseccorrected + index, &read_val)) {
+			dev_err(priv->dev, "Couldn't access feature register\n");
+			kfree_sensitive(buffer);
+			return -EINVAL;
+		}
+		memcpy(buffer + i, &read_val, 4);
+	}
+
+	memcpy(val, buffer + reg % 4, bytes);
+	kfree_sensitive(buffer);
+	return 0;
+}
+
 static void qfprom_runtime_disable(void *data)
 {
 	pm_runtime_disable(data);
@@ -373,13 +405,6 @@ static int qfprom_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;

-	/* The corrected section is always provided */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	priv->qfpcorrected = devm_ioremap_resource(dev, res);
-	if (IS_ERR(priv->qfpcorrected))
-		return PTR_ERR(priv->qfpcorrected);
-
-	econfig.size = resource_size(res);
 	econfig.dev = dev;
 	econfig.priv = priv;

@@ -390,6 +415,20 @@ static int qfprom_probe(struct platform_device *pdev)
 		econfig.nkeepout = soc_data->nkeepout;
 	}

+	/* The corrected section is always provided */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (soc_data && soc_data->secure) {
+		priv->qfpseccorrected = res->start;
+		econfig.reg_read = qfprom_sec_reg_read;
+	} else {
+		priv->qfpcorrected = devm_ioremap_resource(dev, res);
+		if (IS_ERR(priv->qfpcorrected))
+			return PTR_ERR(priv->qfpcorrected);
+	}
+
+	econfig.size = resource_size(res);
+
 	/*
 	 * If more than one region is provided then the OS has the ability
 	 * to write.
--
2.17.1

[PATCH v3 02/10] dt-bindings: nvmem: qfprom: Add compatible for QDU1000/QRU1000

[Komal Bajaj]

Document the QFPROM on QDU1000/QRU1000 SOCs.

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

diff --git a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
index 8d8503dd934b..59082f6e8c9f 100644
--- a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
+++ b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
@@ -26,6 +26,7 @@ properties:
           - qcom,msm8996-qfprom
           - qcom,msm8998-qfprom
           - qcom,qcs404-qfprom
+          - qcom,qdu1000-qfprom
           - qcom,sc7180-qfprom
           - qcom,sc7280-qfprom
           - qcom,sdm630-qfprom
--
2.17.1

[PATCH v3 03/10] arm64: dts: qcom: qdu1000: Add properties to qfprom for multi channel DDR

[Komal Bajaj]

Add properties to the qfprom node for multi channel DDR.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qdu1000.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qdu1000.dtsi b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
index 734438113bba..418644f396bc 100644
--- a/arch/arm64/boot/dts/qcom/qdu1000.dtsi
+++ b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
@@ -1333,6 +1333,18 @@
 			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
 			multi-ch-bit-off = <24 2>;
 		};
+
+		qfprom: efuse@221c8000 {
+			compatible = "qcom,qdu1000-qfprom", "qcom,qfprom";
+			reg = <0 0x221c8000 0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			multi_chan_ddr: multi_chan_ddr@12b {
+				reg = <0x12b 0x1>;
+				bits = <0 2>;
+			};
+		};
 	};

 	timer {
--
2.17.1

[PATCH v3 04/10] nvmem: qfprom: Add support for secure reading on QDU1000/QRU1000

[Komal Bajaj]

Add qfprom driver support for QDU1000/QRU1000 SOCs.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 drivers/nvmem/qfprom.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index 20662e2d3732..12a7981a8a71 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -109,6 +109,10 @@ struct qfprom_soc_compatible_data {
 	bool secure;
 };

+static const struct qfprom_soc_compatible_data qdu1000_qfprom = {
+	.secure = true
+};
+
 static const struct nvmem_keepout sc7180_qfprom_keepout[] = {
 	{.start = 0x128, .end = 0x148},
 	{.start = 0x220, .end = 0x228}
@@ -490,6 +494,7 @@ static int qfprom_probe(struct platform_device *pdev)

 static const struct of_device_id qfprom_of_match[] = {
 	{ .compatible = "qcom,qfprom",},
+	{ .compatible = "qcom,qdu1000-qfprom", .data = &qdu1000_qfprom},
 	{ .compatible = "qcom,sc7180-qfprom", .data = &sc7180_qfprom},
 	{ .compatible = "qcom,sc7280-qfprom", .data = &sc7280_qfprom},
 	{/* sentinel */},
--
2.17.1

[PATCH v3 05/10] 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 | 210 ++++++++++++++++++++---------------
 1 file changed, 123 insertions(+), 87 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 67c19ed2219a..6cf373da5df9 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -423,101 +423,137 @@ 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,
-	.no_edac	= true,
+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,
+		.no_edac	= true,
+	},
+	{ },
 };

-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 sm7150_cfg = {
-	.sct_data       = sm7150_data,
-	.size           = ARRAY_SIZE(sm7150_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 sm7150_cfg[] = {
+	{
+		.sct_data       = sm7150_data,
+		.size           = ARRAY_SIZE(sm7150_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;
@@ -1004,8 +1040,8 @@ static int qcom_llcc_probe(struct platform_device *pdev)

 	drv_data->version = version;

-	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)
@@ -1051,18 +1087,18 @@ 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,sm7150-llcc", .data = &sm7150_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,sm7150-llcc", .data = sm7150_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.17.1

[PATCH v3 06/10] soc: qcom: Add LLCC support for multi channel DDR

[Komal Bajaj]

Add LLCC support for multi channel DDR configuration
based on a feature register.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 drivers/soc/qcom/llcc-qcom.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
index 6cf373da5df9..1da337e7a378 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -12,6 +12,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
@@ -943,6 +944,19 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev,
 	return ret;
 }

+static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u8 *cfg_index)
+{
+	int ret = 0;
+
+	ret = nvmem_cell_read_u8(&pdev->dev, "multi_chan_ddr", cfg_index);
+	if (ret == -ENOENT) {
+		*cfg_index = 0;
+		return 0;
+	}
+
+	return ret;
+}
+
 static int qcom_llcc_remove(struct platform_device *pdev)
 {
 	/* Set the global pointer to a error code to avoid referencing it */
@@ -975,11 +989,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;
+	u8 cfg_index;
 	u32 version;
 	struct regmap *regmap;
+	u32 num_entries = 0;

 	drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
 	if (!drv_data) {
@@ -1040,8 +1056,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)

 	drv_data->version = version;

-	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++, num_entries++);
+	if (cfg_index >= num_entries || cfg_index < 0) {
+		ret = -EINVAL;
+		goto err;
+	}
+
+	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)
--
2.17.1

[PATCH v3 07/10] 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/cache/qcom,llcc.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/cache/qcom,llcc.yaml b/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
index d8b91944180a..f289a0294339 100644
--- a/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
+++ b/Documentation/devicetree/bindings/cache/qcom,llcc.yaml
@@ -20,6 +20,7 @@ description: |
 properties:
   compatible:
     enum:
+      - qcom,qdu1000-llcc
       - qcom,sc7180-llcc
       - qcom,sc7280-llcc
       - qcom,sc8180x-llcc
@@ -92,6 +93,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,qdu1000-llcc
               - qcom,sc8180x-llcc
               - qcom,sc8280xp-llcc
     then:
--
2.17.1

[PATCH v3 08/10] Revert "arm64: dts: qcom: qdu1000: Add LLCC/system-cache-controller"

[Komal Bajaj]

This reverts commit 6209038f131fee84ff1536dc59864f54d06740f2.
Reverting change as it was merged before corresponding driver side
changes, resulting in incompatibility with the current implementation.
Committing the revert now.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qdu1000.dtsi | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qdu1000.dtsi b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
index 418644f396bc..ff21e7a6b312 100644
--- a/arch/arm64/boot/dts/qcom/qdu1000.dtsi
+++ b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
@@ -1322,18 +1322,6 @@
 			#interconnect-cells = <2>;
 		};

-		system-cache-controller@19200000 {
-			compatible = "qcom,qdu1000-llcc";
-			reg = <0 0x19200000 0 0xd80000>,
-			      <0 0x1a200000 0 0x80000>,
-			      <0 0x221c8128 0 0x4>;
-			reg-names = "llcc_base",
-				    "llcc_broadcast_base",
-				    "multi_channel_register";
-			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
-			multi-ch-bit-off = <24 2>;
-		};
-
 		qfprom: efuse@221c8000 {
 			compatible = "qcom,qdu1000-qfprom", "qcom,qfprom";
 			reg = <0 0x221c8000 0 0x1000>;
--
2.17.1

[PATCH v3 09/10] arm64: dts: qcom: qdu1000: Add LLCC/system-cache-controller

[Komal Bajaj]

Add a DT node for Last level cache (aka. system cache) controller
which provides control over the last level cache present on QDU1000
and QRU1000 SoCs.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qdu1000.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qdu1000.dtsi b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
index ff21e7a6b312..3c557ca27500 100644
--- a/arch/arm64/boot/dts/qcom/qdu1000.dtsi
+++ b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
@@ -1322,6 +1322,22 @@
 			#interconnect-cells = <2>;
 		};

+		system-cache-controller@19200000 {
+			compatible = "qcom,qdu1000-llcc";
+			reg = <0 0x19200000 0 0x80000>, <0 0x19600000 0 0x80000>,
+			      <0 0x19300000 0 0x80000>, <0 0x19700000 0 0x80000>,
+			      <0 0x19a00000 0 0x80000>, <0 0x19b00000 0 0x80000>,
+			      <0 0x19e00000 0 0x80000>, <0 0x19f00000 0 0x80000>,
+			      <0 0x1a200000 0 0x80000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+				    "llcc3_base", "llcc4_base", "llcc5_base",
+				    "llcc6_base", "llcc7_base", "llcc_broadcast_base";
+			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
+
+			nvmem-cell-names = "multi_chan_ddr";
+			nvmem-cells = <&multi_chan_ddr>;
+		};
+
 		qfprom: efuse@221c8000 {
 			compatible = "qcom,qdu1000-qfprom", "qcom,qfprom";
 			reg = <0 0x221c8000 0 0x1000>;
--
2.17.1

[PATCH v3 10/10] 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 1da337e7a378..01da6be1f861 100644
--- a/drivers/soc/qcom/llcc-qcom.c
+++ b/drivers/soc/qcom/llcc-qcom.c
@@ -187,7 +187,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 },
@@ -358,6 +358,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,
@@ -557,6 +587,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;

 /**
@@ -1125,6 +1187,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 423220e66026..1359ed8a8a3f 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.17.1

Re: [PATCH v3 00/10] soc: qcom: llcc: Add support for QDU1000/QRU1000

[Krzysztof Kozlowski]

On 12/05/2023 14:21, Komal Bajaj wrote:
> This patch series does the following -
>  * Add support for reading secure fuse region in qfprom driver
>  * Add support for qfprom on QDU1000 and QRU1000 SoCs
>  * Refactor LLCC driver to support multiple configuration
>  * Add support for multi channel DDR configuration in LLCC
>  * Add LLCC support for the Qualcomm QDU1000 and QRU1000 SoCs
> 
> Changes in v3-
>  - Addressed comments from Krzysztof and Mani.

Please be more specific, what exactly changed.

Best regards,
Krzysztof

Re: [PATCH v3 02/10] dt-bindings: nvmem: qfprom: Add compatible for QDU1000/QRU1000

[Krzysztof Kozlowski]

On 12/05/2023 14:21, Komal Bajaj wrote:
> Document the QFPROM on QDU1000/QRU1000 SOCs.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
> index 8d8503dd934b..59082f6e8c9f 100644
> --- a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
> @@ -26,6 +26,7 @@ properties:
>            - qcom,msm8996-qfprom
>            - qcom,msm8998-qfprom
>            - qcom,qcs404-qfprom
> +          - qcom,qdu1000-qfprom

Above qcs, to keep alphabetical order.

That's a new patch? Nothing in changelog suggested it...


Best regards,
Krzysztof

Re: [PATCH v3 03/10] arm64: dts: qcom: qdu1000: Add properties to qfprom for multi channel DDR

[Krzysztof Kozlowski]

On 12/05/2023 14:21, Komal Bajaj wrote:
> Add properties to the qfprom node for multi channel DDR.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>

DTS goes to the end of patchset. Better - to separate patchset with
links to bindings.

Subject: drop redundant pieces, it's too long without actual need.
"properties to" can be skipped.

> ---
>  arch/arm64/boot/dts/qcom/qdu1000.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qdu1000.dtsi b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
> index 734438113bba..418644f396bc 100644
> --- a/arch/arm64/boot/dts/qcom/qdu1000.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
> @@ -1333,6 +1333,18 @@
>  			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
>  			multi-ch-bit-off = <24 2>;
>  		};
> +
> +		qfprom: efuse@221c8000 {
> +			compatible = "qcom,qdu1000-qfprom", "qcom,qfprom";
> +			reg = <0 0x221c8000 0 0x1000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			multi_chan_ddr: multi_chan_ddr@12b {

No underscores in node names.



Best regards,
Krzysztof

Re: [PATCH v3 04/10] nvmem: qfprom: Add support for secure reading on QDU1000/QRU1000

[Krzysztof Kozlowski]

On 12/05/2023 14:21, Komal Bajaj wrote:
> Add qfprom driver support for QDU1000/QRU1000 SOCs.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  drivers/nvmem/qfprom.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> index 20662e2d3732..12a7981a8a71 100644
> --- a/drivers/nvmem/qfprom.c
> +++ b/drivers/nvmem/qfprom.c
> @@ -109,6 +109,10 @@ struct qfprom_soc_compatible_data {
>  	bool secure;
>  };
> 
> +static const struct qfprom_soc_compatible_data qdu1000_qfprom = {
> +	.secure = true
> +};
> +
>  static const struct nvmem_keepout sc7180_qfprom_keepout[] = {
>  	{.start = 0x128, .end = 0x148},
>  	{.start = 0x220, .end = 0x228}
> @@ -490,6 +494,7 @@ static int qfprom_probe(struct platform_device *pdev)
> 
>  static const struct of_device_id qfprom_of_match[] = {
>  	{ .compatible = "qcom,qfprom",},
> +	{ .compatible = "qcom,qdu1000-qfprom", .data = &qdu1000_qfprom},
>  	{ .compatible = "qcom,sc7180-qfprom", .data = &sc7180_qfprom},

I have doubts that this is still compatible with qcom,qfprom. It uses
entirely different read method. That's why generic fallbacks are bad,
one more case to my growing list of awesome examples. :)

Best regards,
Krzysztof

Re: [PATCH v3 04/10] nvmem: qfprom: Add support for secure reading on QDU1000/QRU1000

[Dmitry Baryshkov]

On Fri, 12 May 2023 at 20:01, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 12/05/2023 14:21, Komal Bajaj wrote:
> > Add qfprom driver support for QDU1000/QRU1000 SOCs.
> >
> > Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> > ---
> >  drivers/nvmem/qfprom.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> > index 20662e2d3732..12a7981a8a71 100644
> > --- a/drivers/nvmem/qfprom.c
> > +++ b/drivers/nvmem/qfprom.c
> > @@ -109,6 +109,10 @@ struct qfprom_soc_compatible_data {
> >       bool secure;
> >  };
> >
> > +static const struct qfprom_soc_compatible_data qdu1000_qfprom = {
> > +     .secure = true
> > +};
> > +
> >  static const struct nvmem_keepout sc7180_qfprom_keepout[] = {
> >       {.start = 0x128, .end = 0x148},
> >       {.start = 0x220, .end = 0x228}
> > @@ -490,6 +494,7 @@ static int qfprom_probe(struct platform_device *pdev)
> >
> >  static const struct of_device_id qfprom_of_match[] = {
> >       { .compatible = "qcom,qfprom",},
> > +     { .compatible = "qcom,qdu1000-qfprom", .data = &qdu1000_qfprom},
> >       { .compatible = "qcom,sc7180-qfprom", .data = &sc7180_qfprom},
>
> I have doubts that this is still compatible with qcom,qfprom. It uses
> entirely different read method. That's why generic fallbacks are bad,
> one more case to my growing list of awesome examples. :)

Yes, it looks like it should be 'qcom,qdu1000-qfprom",
"qcom,scm-qfprom". And possibly a separate driver for scm-qfprom.


-- 
With best wishes
Dmitry

Re: [PATCH v3 06/10] soc: qcom: Add LLCC support for multi channel DDR

[kernel test robot]

Hi Komal,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.4-rc1 next-20230512]
[cannot apply to robh/for-next]
[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/nvmem-qfprom-Add-support-for-secure-reading/20230512-202430
base:   linus/master
patch link:    https://lore.kernel.org/r/20230512122134.24339-7-quic_kbajaj%40quicinc.com
patch subject: [PATCH v3 06/10] soc: qcom: Add LLCC support for multi channel DDR
config: riscv-randconfig-r042-20230509 (https://download.01.org/0day-ci/archive/20230513/202305130303.wwdZb5hy-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b0fb98227c90adf2536c9ad644a74d5e92961111)
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 riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/52808ee1c4720767ab330b371d356ffbd8fe7235
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Komal-Bajaj/nvmem-qfprom-Add-support-for-secure-reading/20230512-202430
        git checkout 52808ee1c4720767ab330b371d356ffbd8fe7235
        # 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=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/soc/qcom/

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/202305130303.wwdZb5hy-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/soc/qcom/llcc-qcom.c:951:8: error: call to undeclared function 'nvmem_cell_read_u8'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
           ret = nvmem_cell_read_u8(&pdev->dev, "multi_chan_ddr", cfg_index);
                 ^
   1 error generated.


vim +/nvmem_cell_read_u8 +951 drivers/soc/qcom/llcc-qcom.c

   946	
   947	static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u8 *cfg_index)
   948	{
   949		int ret = 0;
   950	
 > 951		ret = nvmem_cell_read_u8(&pdev->dev, "multi_chan_ddr", cfg_index);
   952		if (ret == -ENOENT) {
   953			*cfg_index = 0;
   954			return 0;
   955		}
   956	
   957		return ret;
   958	}
   959	

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

Re: [PATCH v3 06/10] soc: qcom: Add LLCC support for multi channel DDR

[kernel test robot]

Hi Komal,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.4-rc1 next-20230512]
[cannot apply to robh/for-next]
[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/nvmem-qfprom-Add-support-for-secure-reading/20230512-202430
base:   linus/master
patch link:    https://lore.kernel.org/r/20230512122134.24339-7-quic_kbajaj%40quicinc.com
patch subject: [PATCH v3 06/10] soc: qcom: Add LLCC support for multi channel DDR
config: nios2-buildonly-randconfig-r001-20230509 (https://download.01.org/0day-ci/archive/20230513/202305130308.9zOC4OZC-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 12.1.0
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
        # https://github.com/intel-lab-lkp/linux/commit/52808ee1c4720767ab330b371d356ffbd8fe7235
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Komal-Bajaj/nvmem-qfprom-Add-support-for-secure-reading/20230512-202430
        git checkout 52808ee1c4720767ab330b371d356ffbd8fe7235
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=nios2 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=nios2 SHELL=/bin/bash drivers/soc/qcom/

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/202305130308.9zOC4OZC-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/soc/qcom/llcc-qcom.c: In function 'qcom_llcc_get_cfg_index':
>> drivers/soc/qcom/llcc-qcom.c:951:15: error: implicit declaration of function 'nvmem_cell_read_u8'; did you mean 'nvmem_cell_read_u64'? [-Werror=implicit-function-declaration]
     951 |         ret = nvmem_cell_read_u8(&pdev->dev, "multi_chan_ddr", cfg_index);
         |               ^~~~~~~~~~~~~~~~~~
         |               nvmem_cell_read_u64
   cc1: some warnings being treated as errors


vim +951 drivers/soc/qcom/llcc-qcom.c

   946	
   947	static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u8 *cfg_index)
   948	{
   949		int ret = 0;
   950	
 > 951		ret = nvmem_cell_read_u8(&pdev->dev, "multi_chan_ddr", cfg_index);
   952		if (ret == -ENOENT) {
   953			*cfg_index = 0;
   954			return 0;
   955		}
   956	
   957		return ret;
   958	}
   959	

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

Re: [PATCH v3 07/10] dt-bindings: arm: msm: Add LLCC compatible for QDU1000/QRU1000

[Krzysztof Kozlowski]

On 12/05/2023 14:21, Komal Bajaj wrote:
> Add LLCC compatible for QDU1000/QRU1000 SoCs.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching). In this case - cache: qcom,llcc: - and drop
redundant parts in subject.

With subject fixes:


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v3 08/10] Revert "arm64: dts: qcom: qdu1000: Add LLCC/system-cache-controller"

[Krzysztof Kozlowski]

On 12/05/2023 14:21, Komal Bajaj wrote:
> This reverts commit 6209038f131fee84ff1536dc59864f54d06740f2.
> Reverting change as it was merged before corresponding driver side
> changes, resulting in incompatibility with the current implementation.
> Committing the revert now.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qdu1000.dtsi | 12 ------------

This does not make much sense as separate commit. We do not drop drivers
to re-add them with bugs fixed. The same here - if the node is not
correct, just fix it instead of first removing and then re-adding.

Best regards,
Krzysztof

Re: [PATCH v3 09/10] arm64: dts: qcom: qdu1000: Add LLCC/system-cache-controller

[Krzysztof Kozlowski]

On 12/05/2023 14:21, Komal Bajaj wrote:
> Add a DT node for Last level cache (aka. system cache) controller
> which provides control over the last level cache present on QDU1000
> and QRU1000 SoCs.
> 

Must be squashed. It's difficult to spot the changes against original code.

Best regards,
Krzysztof

Re: [PATCH v3 02/10] dt-bindings: nvmem: qfprom: Add compatible for QDU1000/QRU1000

[Komal Bajaj]

On 5/12/2023 10:26 PM, Krzysztof Kozlowski wrote:
> On 12/05/2023 14:21, Komal Bajaj wrote:
>> Document the QFPROM on QDU1000/QRU1000 SOCs.
>>
>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
>> index 8d8503dd934b..59082f6e8c9f 100644
>> --- a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
>> +++ b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
>> @@ -26,6 +26,7 @@ properties:
>>             - qcom,msm8996-qfprom
>>             - qcom,msm8998-qfprom
>>             - qcom,qcs404-qfprom
>> +          - qcom,qdu1000-qfprom
> Above qcs, to keep alphabetical order.
qdu alphatecially comes after qcs, right? Did I misinterpret your comment?
>
> That's a new patch? Nothing in changelog suggested it...
Yes, that is a new patch from v2. Will mention in cover letter in the 
next patch.

Thanks,
Komal
>
> Best regards,
> Krzysztof
>

Re: [PATCH v3 04/10] nvmem: qfprom: Add support for secure reading on QDU1000/QRU1000

[Komal Bajaj]

On 5/12/2023 11:01 PM, Dmitry Baryshkov wrote:
> On Fri, 12 May 2023 at 20:01, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 12/05/2023 14:21, Komal Bajaj wrote:
>>> Add qfprom driver support for QDU1000/QRU1000 SOCs.
>>>
>>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>>> ---
>>>   drivers/nvmem/qfprom.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
>>> index 20662e2d3732..12a7981a8a71 100644
>>> --- a/drivers/nvmem/qfprom.c
>>> +++ b/drivers/nvmem/qfprom.c
>>> @@ -109,6 +109,10 @@ struct qfprom_soc_compatible_data {
>>>        bool secure;
>>>   };
>>>
>>> +static const struct qfprom_soc_compatible_data qdu1000_qfprom = {
>>> +     .secure = true
>>> +};
>>> +
>>>   static const struct nvmem_keepout sc7180_qfprom_keepout[] = {
>>>        {.start = 0x128, .end = 0x148},
>>>        {.start = 0x220, .end = 0x228}
>>> @@ -490,6 +494,7 @@ static int qfprom_probe(struct platform_device *pdev)
>>>
>>>   static const struct of_device_id qfprom_of_match[] = {
>>>        { .compatible = "qcom,qfprom",},
>>> +     { .compatible = "qcom,qdu1000-qfprom", .data = &qdu1000_qfprom},
>>>        { .compatible = "qcom,sc7180-qfprom", .data = &sc7180_qfprom},
>> I have doubts that this is still compatible with qcom,qfprom. It uses
>> entirely different read method. That's why generic fallbacks are bad,
>> one more case to my growing list of awesome examples. :)
Okay, will do that.
> Yes, it looks like it should be 'qcom,qdu1000-qfprom",
> "qcom,scm-qfprom". And possibly a separate driver for scm-qfprom.
The only difference here is in read method, which can be controlled by a 
single property,
do we really need to write a separate driver for just reading secure 
feature register.

Thanks,
Komal
>
>

Re: [PATCH v3 07/10] dt-bindings: arm: msm: Add LLCC compatible for QDU1000/QRU1000

[Komal Bajaj]

On 5/13/2023 2:59 PM, Krzysztof Kozlowski wrote:
> On 12/05/2023 14:21, Komal Bajaj wrote:
>> Add LLCC compatible for QDU1000/QRU1000 SoCs.
>>
>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching). In this case - cache: qcom,llcc: - and drop
> redundant parts in subject.
>
> With subject fixes:
Okay, will do that.

Thanks,
Komal
>
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> Best regards,
> Krzysztof
>

Re: [PATCH v3 02/10] dt-bindings: nvmem: qfprom: Add compatible for QDU1000/QRU1000

[Krzysztof Kozlowski]

On 15/05/2023 09:08, Komal Bajaj wrote:
> 
> 
> On 5/12/2023 10:26 PM, Krzysztof Kozlowski wrote:
>> On 12/05/2023 14:21, Komal Bajaj wrote:
>>> Document the QFPROM on QDU1000/QRU1000 SOCs.
>>>
>>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>>> ---
>>>   Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
>>> index 8d8503dd934b..59082f6e8c9f 100644
>>> --- a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
>>> +++ b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
>>> @@ -26,6 +26,7 @@ properties:
>>>             - qcom,msm8996-qfprom
>>>             - qcom,msm8998-qfprom
>>>             - qcom,qcs404-qfprom
>>> +          - qcom,qdu1000-qfprom
>> Above qcs, to keep alphabetical order.
> qdu alphatecially comes after qcs, right? Did I misinterpret your comment?

Ups...

Best regards,
Krzysztof

Re: [PATCH v3 02/10] dt-bindings: nvmem: qfprom: Add compatible for QDU1000/QRU1000

[Krzysztof Kozlowski]

On 12/05/2023 14:21, Komal Bajaj wrote:
> Document the QFPROM on QDU1000/QRU1000 SOCs.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml | 1 +


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v3 01/10] nvmem: qfprom: Add support for secure reading

[Bjorn Andersson]

On Fri, May 12, 2023 at 05:51:25PM +0530, Komal Bajaj wrote:
> For some of the Qualcomm SoC's, it is possible that
> some of the fuse regions or entire qfprom region is
> protected from non-secure access. In such situations,
> linux will have to use secure calls to read the region.
> With that motivation, add the support of reading secure
> regions in qfprom driver. Ensuring the address to read
> is word aligned since our secure I/O only supports word
> size I/O.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  drivers/nvmem/Kconfig  |  1 +
>  drivers/nvmem/qfprom.c | 69 +++++++++++++++++++++++++++++++++---------
>  2 files changed, 55 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index b291b27048c7..3d896ba29b89 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -209,6 +209,7 @@ config NVMEM_QCOM_QFPROM
>  	tristate "QCOM QFPROM Support"
>  	depends on ARCH_QCOM || COMPILE_TEST
>  	depends on HAS_IOMEM
> +	select QCOM_SCM
>  	help
>  	  Say y here to enable QFPROM support. The QFPROM provides access
>  	  functions for QFPROM data to rest of the drivers via nvmem interface.
> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> index c1e893c8a247..20662e2d3732 100644
> --- a/drivers/nvmem/qfprom.c
> +++ b/drivers/nvmem/qfprom.c
> @@ -16,6 +16,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/property.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
> 
>  /* Blow timer clock frequency in Mhz */
>  #define QFPROM_BLOW_TIMER_OFFSET 0x03c
> @@ -59,21 +60,22 @@ struct qfprom_soc_data {
>  /**
>   * struct qfprom_priv - structure holding qfprom attributes
>   *
> - * @qfpraw:       iomapped memory space for qfprom-efuse raw address space.
> - * @qfpconf:      iomapped memory space for qfprom-efuse configuration address
> - *                space.
> + * @qfpraw: iomapped memory space for qfprom-efuse raw address space.
> + * @qfpconf: iomapped memory space for qfprom-efuse configuration address space.

Adjusting the indentation makes it unnecessarily hard to see what you
actually changed.

>   * @qfpcorrected: iomapped memory space for qfprom corrected address space.
> - * @qfpsecurity:  iomapped memory space for qfprom security control space.
> - * @dev:          qfprom device structure.
> - * @secclk:       Clock supply.
> - * @vcc:          Regulator supply.
> - * @soc_data:     Data that for things that varies from SoC to SoC.
> + * @qfpsecurity: iomapped memory space for qfprom security control space.
> + * @qfpseccorrected: starting physical address for qfprom secure corrected address space.
> + * @dev: qfprom device structure.
> + * @secclk: Clock supply.
> + * @vcc: Regulator supply.
> + * @soc_data: Data that for things that varies from SoC to SoC.
>   */
>  struct qfprom_priv {
>  	void __iomem *qfpraw;
>  	void __iomem *qfpconf;
>  	void __iomem *qfpcorrected;
>  	void __iomem *qfpsecurity;
> +	phys_addr_t qfpseccorrected;
>  	struct device *dev;
>  	struct clk *secclk;
>  	struct regulator *vcc;
> @@ -99,10 +101,12 @@ struct qfprom_touched_values {
>   *
>   * @keepout: Array of keepout regions for this SoC.
>   * @nkeepout: Number of elements in the keepout array.
> + * @secure: Is qfprom region for this SoC protected from non-secure access.
>   */
>  struct qfprom_soc_compatible_data {
>  	const struct nvmem_keepout *keepout;
>  	unsigned int nkeepout;
> +	bool secure;
>  };
> 
>  static const struct nvmem_keepout sc7180_qfprom_keepout[] = {
> @@ -334,6 +338,34 @@ static int qfprom_reg_read(void *context,
>  	return 0;
>  }
> 
> +static int qfprom_sec_reg_read(void *context, unsigned int reg, void *_val, size_t bytes)
> +{
> +	struct qfprom_priv *priv = context;
> +	u8 *val = _val;
> +	int buf_start, buf_end, index, i = 0;
> +	char *buffer;
> +	u32 read_val;
> +
> +	buf_start = ALIGN_DOWN(reg, 4);
> +	buf_end = ALIGN(reg + bytes, 4);
> +	buffer = kzalloc(buf_end - buf_start, GFP_KERNEL);
> +	if (!buffer)
> +		return -ENOMEM;

I don't you need all these variables, the full temp buffer or the two
memcpy... I think something like this should do the trick:

	unsigned int i;
	u8 *val = _val;
	u8 tmp[4];

	for (i = 0; i < bytes; i++, reg++)
		if (i == 0 || reg % 4 == 0)
			qcom_scm_io_readl(qfpseccorrected + (reg & ~3), tmp);

		val[i] = tmp[reg & 3];
	}

> +
> +	for (index = buf_start; index < buf_end; index += 4, i += 4) {
> +		if (qcom_scm_io_readl(priv->qfpseccorrected + index, &read_val)) {
> +			dev_err(priv->dev, "Couldn't access feature register\n");

What's a "feature register"?

Regards,
Bjorn

> +			kfree_sensitive(buffer);
> +			return -EINVAL;
> +		}
> +		memcpy(buffer + i, &read_val, 4);
> +	}
> +
> +	memcpy(val, buffer + reg % 4, bytes);
> +	kfree_sensitive(buffer);
> +	return 0;
> +}
> +
>  static void qfprom_runtime_disable(void *data)
>  {
>  	pm_runtime_disable(data);
> @@ -373,13 +405,6 @@ static int qfprom_probe(struct platform_device *pdev)
>  	if (!priv)
>  		return -ENOMEM;
> 
> -	/* The corrected section is always provided */
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	priv->qfpcorrected = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(priv->qfpcorrected))
> -		return PTR_ERR(priv->qfpcorrected);
> -
> -	econfig.size = resource_size(res);
>  	econfig.dev = dev;
>  	econfig.priv = priv;
> 
> @@ -390,6 +415,20 @@ static int qfprom_probe(struct platform_device *pdev)
>  		econfig.nkeepout = soc_data->nkeepout;
>  	}
> 
> +	/* The corrected section is always provided */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	if (soc_data && soc_data->secure) {
> +		priv->qfpseccorrected = res->start;
> +		econfig.reg_read = qfprom_sec_reg_read;
> +	} else {
> +		priv->qfpcorrected = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(priv->qfpcorrected))
> +			return PTR_ERR(priv->qfpcorrected);
> +	}
> +
> +	econfig.size = resource_size(res);
> +
>  	/*
>  	 * If more than one region is provided then the OS has the ability
>  	 * to write.
> --
> 2.17.1
> 

Re: [PATCH v3 04/10] nvmem: qfprom: Add support for secure reading on QDU1000/QRU1000

[Bjorn Andersson]

On Mon, May 15, 2023 at 02:02:11PM +0530, Komal Bajaj wrote:
> 
> 
> On 5/12/2023 11:01 PM, Dmitry Baryshkov wrote:
> > On Fri, 12 May 2023 at 20:01, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > > On 12/05/2023 14:21, Komal Bajaj wrote:
> > > > Add qfprom driver support for QDU1000/QRU1000 SOCs.
> > > > 
> > > > Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> > > > ---
> > > >   drivers/nvmem/qfprom.c | 5 +++++
> > > >   1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> > > > index 20662e2d3732..12a7981a8a71 100644
> > > > --- a/drivers/nvmem/qfprom.c
> > > > +++ b/drivers/nvmem/qfprom.c
> > > > @@ -109,6 +109,10 @@ struct qfprom_soc_compatible_data {
> > > >        bool secure;
> > > >   };
> > > > 
> > > > +static const struct qfprom_soc_compatible_data qdu1000_qfprom = {
> > > > +     .secure = true
> > > > +};
> > > > +
> > > >   static const struct nvmem_keepout sc7180_qfprom_keepout[] = {
> > > >        {.start = 0x128, .end = 0x148},
> > > >        {.start = 0x220, .end = 0x228}
> > > > @@ -490,6 +494,7 @@ static int qfprom_probe(struct platform_device *pdev)
> > > > 
> > > >   static const struct of_device_id qfprom_of_match[] = {
> > > >        { .compatible = "qcom,qfprom",},
> > > > +     { .compatible = "qcom,qdu1000-qfprom", .data = &qdu1000_qfprom},
> > > >        { .compatible = "qcom,sc7180-qfprom", .data = &sc7180_qfprom},
> > > I have doubts that this is still compatible with qcom,qfprom. It uses
> > > entirely different read method. That's why generic fallbacks are bad,
> > > one more case to my growing list of awesome examples. :)
> Okay, will do that.
> > Yes, it looks like it should be 'qcom,qdu1000-qfprom",
> > "qcom,scm-qfprom". And possibly a separate driver for scm-qfprom.
> The only difference here is in read method, which can be controlled by a
> single property,
> do we really need to write a separate driver for just reading secure feature
> register.

I presume that if reads are hidden behind scm, then the most of the
driver - which deals with writing to qfprom - isn't going to be at all
applicable.

So, I actually think it would make sense to put that in a separate
qfprom-scm driver, which handles the generic fallback of
"qcom,qfprom-scm".

Regards,
Bjorn

> 
> Thanks,
> Komal
> > 
> > 
> 

Re: [PATCH v3 01/10] nvmem: qfprom: Add support for secure reading

[Komal Bajaj]

On 5/28/2023 2:20 AM, Bjorn Andersson wrote:
> On Fri, May 12, 2023 at 05:51:25PM +0530, Komal Bajaj wrote:
>> For some of the Qualcomm SoC's, it is possible that
>> some of the fuse regions or entire qfprom region is
>> protected from non-secure access. In such situations,
>> linux will have to use secure calls to read the region.
>> With that motivation, add the support of reading secure
>> regions in qfprom driver. Ensuring the address to read
>> is word aligned since our secure I/O only supports word
>> size I/O.
>>
>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>> ---
>>   drivers/nvmem/Kconfig  |  1 +
>>   drivers/nvmem/qfprom.c | 69 +++++++++++++++++++++++++++++++++---------
>>   2 files changed, 55 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>> index b291b27048c7..3d896ba29b89 100644
>> --- a/drivers/nvmem/Kconfig
>> +++ b/drivers/nvmem/Kconfig
>> @@ -209,6 +209,7 @@ config NVMEM_QCOM_QFPROM
>>   	tristate "QCOM QFPROM Support"
>>   	depends on ARCH_QCOM || COMPILE_TEST
>>   	depends on HAS_IOMEM
>> +	select QCOM_SCM
>>   	help
>>   	  Say y here to enable QFPROM support. The QFPROM provides access
>>   	  functions for QFPROM data to rest of the drivers via nvmem interface.
>> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
>> index c1e893c8a247..20662e2d3732 100644
>> --- a/drivers/nvmem/qfprom.c
>> +++ b/drivers/nvmem/qfprom.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/pm_runtime.h>
>>   #include <linux/property.h>
>>   #include <linux/regulator/consumer.h>
>> +#include <linux/firmware/qcom/qcom_scm.h>
>>
>>   /* Blow timer clock frequency in Mhz */
>>   #define QFPROM_BLOW_TIMER_OFFSET 0x03c
>> @@ -59,21 +60,22 @@ struct qfprom_soc_data {
>>   /**
>>    * struct qfprom_priv - structure holding qfprom attributes
>>    *
>> - * @qfpraw:       iomapped memory space for qfprom-efuse raw address space.
>> - * @qfpconf:      iomapped memory space for qfprom-efuse configuration address
>> - *                space.
>> + * @qfpraw: iomapped memory space for qfprom-efuse raw address space.
>> + * @qfpconf: iomapped memory space for qfprom-efuse configuration address space.
> Adjusting the indentation makes it unnecessarily hard to see what you
> actually changed.
Okay, will maintain the existing indentation.
>
>>    * @qfpcorrected: iomapped memory space for qfprom corrected address space.
>> - * @qfpsecurity:  iomapped memory space for qfprom security control space.
>> - * @dev:          qfprom device structure.
>> - * @secclk:       Clock supply.
>> - * @vcc:          Regulator supply.
>> - * @soc_data:     Data that for things that varies from SoC to SoC.
>> + * @qfpsecurity: iomapped memory space for qfprom security control space.
>> + * @qfpseccorrected: starting physical address for qfprom secure corrected address space.
>> + * @dev: qfprom device structure.
>> + * @secclk: Clock supply.
>> + * @vcc: Regulator supply.
>> + * @soc_data: Data that for things that varies from SoC to SoC.
>>    */
>>   struct qfprom_priv {
>>   	void __iomem *qfpraw;
>>   	void __iomem *qfpconf;
>>   	void __iomem *qfpcorrected;
>>   	void __iomem *qfpsecurity;
>> +	phys_addr_t qfpseccorrected;
>>   	struct device *dev;
>>   	struct clk *secclk;
>>   	struct regulator *vcc;
>> @@ -99,10 +101,12 @@ struct qfprom_touched_values {
>>    *
>>    * @keepout: Array of keepout regions for this SoC.
>>    * @nkeepout: Number of elements in the keepout array.
>> + * @secure: Is qfprom region for this SoC protected from non-secure access.
>>    */
>>   struct qfprom_soc_compatible_data {
>>   	const struct nvmem_keepout *keepout;
>>   	unsigned int nkeepout;
>> +	bool secure;
>>   };
>>
>>   static const struct nvmem_keepout sc7180_qfprom_keepout[] = {
>> @@ -334,6 +338,34 @@ static int qfprom_reg_read(void *context,
>>   	return 0;
>>   }
>>
>> +static int qfprom_sec_reg_read(void *context, unsigned int reg, void *_val, size_t bytes)
>> +{
>> +	struct qfprom_priv *priv = context;
>> +	u8 *val = _val;
>> +	int buf_start, buf_end, index, i = 0;
>> +	char *buffer;
>> +	u32 read_val;
>> +
>> +	buf_start = ALIGN_DOWN(reg, 4);
>> +	buf_end = ALIGN(reg + bytes, 4);
>> +	buffer = kzalloc(buf_end - buf_start, GFP_KERNEL);
>> +	if (!buffer)
>> +		return -ENOMEM;
> I don't you need all these variables, the full temp buffer or the two
> memcpy... I think something like this should do the trick:
>
> 	unsigned int i;
> 	u8 *val = _val;
> 	u8 tmp[4];
>
> 	for (i = 0; i < bytes; i++, reg++)
> 		if (i == 0 || reg % 4 == 0)
> 			qcom_scm_io_readl(qfpseccorrected + (reg & ~3), tmp);
>
> 		val[i] = tmp[reg & 3];
> 	}
Thanks, for this. Will make the suggested change in the next patch series.
>> +
>> +	for (index = buf_start; index < buf_end; index += 4, i += 4) {
>> +		if (qcom_scm_io_readl(priv->qfpseccorrected + index, &read_val)) {
>> +			dev_err(priv->dev, "Couldn't access feature register\n");
> What's a "feature register"?
It's a fuse register that we are trying to read here.

Thanks
Komal

>
> Regards,
> Bjorn
>
>> +			kfree_sensitive(buffer);
>> +			return -EINVAL;
>> +		}
>> +		memcpy(buffer + i, &read_val, 4);
>> +	}
>> +
>> +	memcpy(val, buffer + reg % 4, bytes);
>> +	kfree_sensitive(buffer);
>> +	return 0;
>> +}
>> +
>>   static void qfprom_runtime_disable(void *data)
>>   {
>>   	pm_runtime_disable(data);
>> @@ -373,13 +405,6 @@ static int qfprom_probe(struct platform_device *pdev)
>>   	if (!priv)
>>   		return -ENOMEM;
>>
>> -	/* The corrected section is always provided */
>> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -	priv->qfpcorrected = devm_ioremap_resource(dev, res);
>> -	if (IS_ERR(priv->qfpcorrected))
>> -		return PTR_ERR(priv->qfpcorrected);
>> -
>> -	econfig.size = resource_size(res);
>>   	econfig.dev = dev;
>>   	econfig.priv = priv;
>>
>> @@ -390,6 +415,20 @@ static int qfprom_probe(struct platform_device *pdev)
>>   		econfig.nkeepout = soc_data->nkeepout;
>>   	}
>>
>> +	/* The corrected section is always provided */
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +	if (soc_data && soc_data->secure) {
>> +		priv->qfpseccorrected = res->start;
>> +		econfig.reg_read = qfprom_sec_reg_read;
>> +	} else {
>> +		priv->qfpcorrected = devm_ioremap_resource(dev, res);
>> +		if (IS_ERR(priv->qfpcorrected))
>> +			return PTR_ERR(priv->qfpcorrected);
>> +	}
>> +
>> +	econfig.size = resource_size(res);
>> +
>>   	/*
>>   	 * If more than one region is provided then the OS has the ability
>>   	 * to write.
>> --
>> 2.17.1
>>

Re: [PATCH v3 06/10] soc: qcom: Add LLCC support for multi channel DDR

[Bjorn Andersson]

On Fri, May 12, 2023 at 05:51:30PM +0530, Komal Bajaj wrote:
> Add LLCC support for multi channel DDR configuration
> based on a feature register.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  drivers/soc/qcom/llcc-qcom.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
> index 6cf373da5df9..1da337e7a378 100644
> --- a/drivers/soc/qcom/llcc-qcom.c
> +++ b/drivers/soc/qcom/llcc-qcom.c
> @@ -12,6 +12,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/nvmem-consumer.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/regmap.h>
> @@ -943,6 +944,19 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev,
>  	return ret;
>  }
> 
> +static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u8 *cfg_index)
> +{
> +	int ret = 0;

First use is an assignment, no need to initialize here.

> +
> +	ret = nvmem_cell_read_u8(&pdev->dev, "multi_chan_ddr", cfg_index);
> +	if (ret == -ENOENT) {
> +		*cfg_index = 0;

Does nvmem_cell_read_u8() cahnge cfg_index when it fails with -ENOENT?

> +		return 0;
> +	}
> +
> +	return ret;
> +}
> +
>  static int qcom_llcc_remove(struct platform_device *pdev)
>  {
>  	/* Set the global pointer to a error code to avoid referencing it */
> @@ -975,11 +989,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;
> +	u8 cfg_index;
>  	u32 version;
>  	struct regmap *regmap;
> +	u32 num_entries = 0;
> 
>  	drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
>  	if (!drv_data) {
> @@ -1040,8 +1056,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
> 
>  	drv_data->version = version;
> 
> -	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++, num_entries++);

This is not readable, move the increment of num_entries out of there.

> +	if (cfg_index >= num_entries || cfg_index < 0) {

How can cfg_index be negative?

Regards,
Bjorn

> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	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)
> --
> 2.17.1
> 

Re: [PATCH v3 06/10] soc: qcom: Add LLCC support for multi channel DDR

[Komal Bajaj]

On 6/13/2023 10:41 PM, Bjorn Andersson wrote:
> On Fri, May 12, 2023 at 05:51:30PM +0530, Komal Bajaj wrote:
>> Add LLCC support for multi channel DDR configuration
>> based on a feature register.
>>
>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>> ---
>>   drivers/soc/qcom/llcc-qcom.c | 32 +++++++++++++++++++++++++++++---
>>   1 file changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/llcc-qcom.c b/drivers/soc/qcom/llcc-qcom.c
>> index 6cf373da5df9..1da337e7a378 100644
>> --- a/drivers/soc/qcom/llcc-qcom.c
>> +++ b/drivers/soc/qcom/llcc-qcom.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>> +#include <linux/nvmem-consumer.h>
>>   #include <linux/of.h>
>>   #include <linux/of_device.h>
>>   #include <linux/regmap.h>
>> @@ -943,6 +944,19 @@ static int qcom_llcc_cfg_program(struct platform_device *pdev,
>>   	return ret;
>>   }
>>
>> +static int qcom_llcc_get_cfg_index(struct platform_device *pdev, u8 *cfg_index)
>> +{
>> +	int ret = 0;
> First use is an assignment, no need to initialize here.

Noted.

>
>> +
>> +	ret = nvmem_cell_read_u8(&pdev->dev, "multi_chan_ddr", cfg_index);
>> +	if (ret == -ENOENT) {
>> +		*cfg_index = 0;
> Does nvmem_cell_read_u8() cahnge cfg_index when it fails with -ENOENT?

No, if nvmem_cell_read_u8() fails with any error, cfg_index will not be 
changed.

Thanks
Komal
>> +		return 0;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>   static int qcom_llcc_remove(struct platform_device *pdev)
>>   {
>>   	/* Set the global pointer to a error code to avoid referencing it */
>> @@ -975,11 +989,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;
>> +	u8 cfg_index;
>>   	u32 version;
>>   	struct regmap *regmap;
>> +	u32 num_entries = 0;
>>
>>   	drv_data = devm_kzalloc(dev, sizeof(*drv_data), GFP_KERNEL);
>>   	if (!drv_data) {
>> @@ -1040,8 +1056,18 @@ static int qcom_llcc_probe(struct platform_device *pdev)
>>
>>   	drv_data->version = version;
>>
>> -	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++, num_entries++);
> This is not readable, move the increment of num_entries out of there.
>
>> +	if (cfg_index >= num_entries || cfg_index < 0) {
> How can cfg_index be negative?
>
> Regards,
> Bjorn
>
>> +		ret = -EINVAL;
>> +		goto err;
>> +	}
>> +
>> +	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)
>> --
>> 2.17.1
>>