[PATCH v1 0/2] tegra20-emc: Identify memory chip by LPDDR configuration

[PATCH v1 0/2] tegra20-emc: Identify memory chip by LPDDR configuration

[Dmitry Osipenko]

Support memory chip identification by LPDDR configuration, which is
needed by Asus Transformer TF101 tablet device that doesn't store RAMCODE
in Tegra's NVMEM.

Dmitry Osipenko (2):
  dt-bindings: memory: tegra20: emc: Document optional LPDDR properties
  memory: tegra20-emc: Support timings matching by LPDDR configuration

 .../nvidia,tegra20-emc.yaml                   |  43 +++-
 drivers/memory/tegra/tegra20-emc.c            | 209 ++++++++++++++++--
 2 files changed, 236 insertions(+), 16 deletions(-)

-- 
2.32.0

[PATCH v1 1/2] dt-bindings: memory: tegra20: emc: Document optional LPDDR properties

[Dmitry Osipenko]

Some Tegra20 boards don't use RAM code for the memory chip identification
and the identity information should read out from LPDDR chip in this case.
Document new optional generic LPDDR properties that will be used for the
memory chip identification if RAM code isn't provided.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 .../nvidia,tegra20-emc.yaml                   | 43 +++++++++++++++++--
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
index cac6842dc8f1..6d01b1bf6304 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
@@ -158,6 +158,46 @@ patternProperties:
         description:
           Value of RAM_CODE this timing set is used for.
 
+      jedec,lpddr-manufacturer-id:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Unique manufacturer ID of SDRAM chip this timing set is used for.
+          See MR5 description in JEDEC LPDDR2 specification (JESD209-2).
+
+      jedec,lpddr-revision-id1:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Revision 1 value of SDRAM chip this timing set is used for.
+          See MR6 description in chip vendor specification.
+
+      jedec,lpddr-revision-id2:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Revision 2 value of SDRAM chip this timing set is used for.
+          See MR7 description in chip vendor specification.
+
+      jedec,lpddr-density-mbits:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Density in megabits of SDRAM chip this timing set is used for.
+          See MR8 description in JEDEC LPDDR2 specification.
+
+      jedec,lpddr-io-width-bits:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          IO bus width in bits of SDRAM chip this timing set is used for.
+          See MR8 description in JEDEC LPDDR2 specification.
+
+      jedec,lpddr-type:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          LPDDR type which corresponds to a number of words SDRAM pre-fetches
+          per column request that this timing set is used for.
+          See MR8 description in JEDEC LPDDR2 specification.
+        enum:
+          - 4 # S4 (4 words prefetch architecture)
+          - 2 # S2 (2 words prefetch architecture)
+
       "#address-cells":
         const: 1
 
@@ -168,9 +208,6 @@ patternProperties:
       "^emc-table@[0-9]+$":
         $ref: "#/$defs/emc-table"
 
-    required:
-      - nvidia,ram-code
-
     additionalProperties: false
 
 required:
-- 
2.32.0

[PATCH v1 2/2] memory: tegra20-emc: Support timings matching by LPDDR configuration

[Dmitry Osipenko]

Asus Transformer TF101 doesn't provide RAM code and in this case memory
timings should be selected based on identity information read out from
SDRAM chip. Support matching timings by LPDDR configuration.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/memory/tegra/tegra20-emc.c | 209 +++++++++++++++++++++++++++--
 1 file changed, 196 insertions(+), 13 deletions(-)

diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index c3462dbc8c22..8384707f021c 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -5,6 +5,7 @@
  * Author: Dmitry Osipenko <digetx@gmail.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/clk/tegra.h>
 #include <linux/debugfs.h>
@@ -32,6 +33,7 @@
 #define EMC_INTSTATUS				0x000
 #define EMC_INTMASK				0x004
 #define EMC_DBG					0x008
+#define EMC_ADR_CFG_0				0x010
 #define EMC_TIMING_CONTROL			0x028
 #define EMC_RC					0x02c
 #define EMC_RFC					0x030
@@ -68,6 +70,7 @@
 #define EMC_QUSE_EXTRA				0x0ac
 #define EMC_ODT_WRITE				0x0b0
 #define EMC_ODT_READ				0x0b4
+#define EMC_MRR					0x0ec
 #define EMC_FBIO_CFG5				0x104
 #define EMC_FBIO_CFG6				0x114
 #define EMC_STAT_CONTROL			0x160
@@ -94,6 +97,7 @@
 
 #define EMC_REFRESH_OVERFLOW_INT		BIT(3)
 #define EMC_CLKCHANGE_COMPLETE_INT		BIT(4)
+#define EMC_MRR_DIVLD_INT			BIT(5)
 
 #define EMC_DBG_READ_MUX_ASSEMBLY		BIT(0)
 #define EMC_DBG_WRITE_MUX_ACTIVE		BIT(1)
@@ -102,11 +106,25 @@
 #define EMC_DBG_CFG_PRIORITY			BIT(24)
 
 #define EMC_FBIO_CFG5_DRAM_WIDTH_X16		BIT(4)
+#define EMC_FBIO_CFG5_DRAM_TYPE			GENMASK(1, 0)
+
+#define EMC_MRR_DEV_SELECTN			GENMASK(31, 30)
+#define EMC_MRR_MRR_MA				GENMASK(23, 16)
+#define EMC_MRR_MRR_DATA			GENMASK(15, 0)
+
+#define EMC_ADR_CFG_0_EMEM_NUMDEV		GENMASK(25, 24)
 
 #define EMC_PWR_GATHER_CLEAR			(1 << 8)
 #define EMC_PWR_GATHER_DISABLE			(2 << 8)
 #define EMC_PWR_GATHER_ENABLE			(3 << 8)
 
+enum emc_dram_type {
+	DRAM_TYPE_RESERVED,
+	DRAM_TYPE_DDR1,
+	DRAM_TYPE_LPDDR2,
+	DRAM_TYPE_DDR2,
+};
+
 static const u16 emc_timing_registers[] = {
 	EMC_RC,
 	EMC_RFC,
@@ -173,6 +191,16 @@ struct emc_rate_request {
 	unsigned long max_rate;
 };
 
+union lpddr_basic_config4 {
+	u32 value;
+
+	struct {
+		unsigned int prefetch_type : 2;
+		unsigned int density : 4;
+		unsigned int width : 2;
+	} __packed;
+};
+
 struct tegra_emc {
 	struct device *dev;
 	struct tegra_mc *mc;
@@ -201,6 +229,12 @@ struct tegra_emc {
 	struct mutex rate_lock;
 
 	struct devfreq_simple_ondemand_data ondemand_data;
+
+	/* memory chip identity information */
+	union lpddr_basic_config4 basic_conf4;
+	unsigned int manufacturer_id;
+	unsigned int revision_id1;
+	unsigned int revision_id2;
 };
 
 static irqreturn_t tegra_emc_isr(int irq, void *data)
@@ -422,8 +456,9 @@ static int tegra_emc_load_timings_from_dt(struct tegra_emc *emc,
 }
 
 static struct device_node *
-tegra_emc_find_node_by_ram_code(struct device *dev)
+tegra_emc_find_node_by_ram_code(struct tegra_emc *emc)
 {
+	struct device *dev = emc->dev;
 	struct device_node *np;
 	u32 value, ram_code;
 	int err;
@@ -442,8 +477,56 @@ tegra_emc_find_node_by_ram_code(struct device *dev)
 	     np = of_find_node_by_name(np, "emc-tables")) {
 		err = of_property_read_u32(np, "nvidia,ram-code", &value);
 		if (err || value != ram_code) {
-			of_node_put(np);
-			continue;
+			unsigned int mr_mismatches = 0;
+			bool mr_compared = false;
+
+			mr_mismatches += of_property_read_bool(np, "jedec,lpddr-manufacturer-id");
+			mr_mismatches += of_property_read_bool(np, "jedec,lpddr-revision-id1");
+			mr_mismatches += of_property_read_bool(np, "jedec,lpddr-revision-id2");
+			mr_mismatches += of_property_read_bool(np, "jedec,lpddr-density-mbits");
+			mr_mismatches += of_property_read_bool(np, "jedec,lpddr-io-width-bits");
+			mr_mismatches += of_property_read_bool(np, "jedec,lpddr-type");
+
+			if (!of_property_read_u32(np, "jedec,lpddr-manufacturer-id", &value) &&
+			    emc->manufacturer_id == value) {
+				mr_compared = true;
+				mr_mismatches--;
+			}
+
+			if (!of_property_read_u32(np, "jedec,lpddr-revision-id1", &value) &&
+			    emc->revision_id1 == value) {
+				mr_compared = true;
+				mr_mismatches--;
+			}
+
+			if (!of_property_read_u32(np, "jedec,lpddr-revision-id2", &value) &&
+			    emc->revision_id2 == value) {
+				mr_compared = true;
+				mr_mismatches--;
+			}
+
+			if (!of_property_read_u32(np, "jedec,lpddr-density-mbits", &value) &&
+			    64 << emc->basic_conf4.density == value) {
+				mr_compared = true;
+				mr_mismatches--;
+			}
+
+			if (!of_property_read_u32(np, "jedec,lpddr-io-width-bits", &value) &&
+			    32 >> emc->basic_conf4.width == value) {
+				mr_compared = true;
+				mr_mismatches--;
+			}
+
+			if (!of_property_read_u32(np, "jedec,lpddr-type", &value) &&
+			    4 >> emc->basic_conf4.prefetch_type == value) {
+				mr_compared = true;
+				mr_mismatches--;
+			}
+
+			if (!mr_compared || mr_mismatches) {
+				of_node_put(np);
+				continue;
+			}
 		}
 
 		return np;
@@ -455,10 +538,81 @@ tegra_emc_find_node_by_ram_code(struct device *dev)
 	return NULL;
 }
 
+static int emc_read_lpddr_mode_register(struct tegra_emc *emc,
+					unsigned int emem_dev,
+					unsigned int register_addr,
+					unsigned int *register_data)
+{
+	u32 memory_dev = emem_dev + 1, val;
+	int err;
+
+	/* clear data-valid interrupt status */
+	writel_relaxed(EMC_MRR_DIVLD_INT, emc->regs + EMC_INTSTATUS);
+
+	/* issue mode register read request */
+	val  = FIELD_PREP(EMC_MRR_DEV_SELECTN, memory_dev);
+	val |= FIELD_PREP(EMC_MRR_MRR_MA, register_addr);
+
+	writel_relaxed(val, emc->regs + EMC_MRR);
+
+	/* wait for the LPDDR2 data-valid interrupt */
+	err = readl_relaxed_poll_timeout_atomic(emc->regs + EMC_INTSTATUS, val,
+						val & EMC_MRR_DIVLD_INT,
+						1, 100);
+	if (err) {
+		dev_err(emc->dev, "mode-register %u read failed: %d\n",
+			register_addr, err);
+		return err;
+	}
+
+	/* read out register data */
+	val = readl_relaxed(emc->regs + EMC_MRR);
+	*register_data = FIELD_GET(EMC_MRR_MRR_DATA, val);
+
+	return 0;
+}
+
+static void emc_read_lpddr_sdram_info(struct tegra_emc *emc,
+				      unsigned int emem_dev,
+				      bool print_out)
+{
+	const char *emem_vendor_name = "";
+
+	/* these registers are standard for all LPDDR JEDEC memory chips */
+	emc_read_lpddr_mode_register(emc, emem_dev, 5, &emc->manufacturer_id);
+	emc_read_lpddr_mode_register(emc, emem_dev, 6, &emc->revision_id1);
+	emc_read_lpddr_mode_register(emc, emem_dev, 7, &emc->revision_id2);
+	emc_read_lpddr_mode_register(emc, emem_dev, 8, &emc->basic_conf4.value);
+
+	if (!print_out)
+		return;
+
+	switch (emc->manufacturer_id) {
+	case 0x303:
+		emem_vendor_name = " (Elpida)";
+		break;
+
+	case 0x606:
+		emem_vendor_name = " (Hynix)";
+		break;
+	}
+
+	dev_info(emc->dev, "SDRAM[dev%u]: manufacturer: 0x%x%s rev1: 0x%x rev2: 0x%x prefetch: S%u density: %uMbit iowidth: %ubit\n",
+		 emem_dev, emc->manufacturer_id, emem_vendor_name,
+		 emc->revision_id1, emc->revision_id2,
+		 4 >> emc->basic_conf4.prefetch_type,
+		 64 << emc->basic_conf4.density,
+		 32 >> emc->basic_conf4.width);
+}
+
 static int emc_setup_hw(struct tegra_emc *emc)
 {
+	u32 emc_cfg, emc_dbg, emc_fbio, emc_adr_cfg;
 	u32 intmask = EMC_REFRESH_OVERFLOW_INT;
-	u32 emc_cfg, emc_dbg, emc_fbio;
+	static bool print_sdram_info_once;
+	enum emc_dram_type dram_type;
+	const char *dram_type_str;
+	unsigned int emem_numdev;
 
 	emc_cfg = readl_relaxed(emc->regs + EMC_CFG_2);
 
@@ -496,7 +650,36 @@ static int emc_setup_hw(struct tegra_emc *emc)
 	else
 		emc->dram_bus_width = 32;
 
-	dev_info_once(emc->dev, "%ubit DRAM bus\n", emc->dram_bus_width);
+	dram_type = FIELD_GET(EMC_FBIO_CFG5_DRAM_TYPE, emc_fbio);
+
+	switch (dram_type) {
+	case DRAM_TYPE_RESERVED:
+		dram_type_str = "INVALID";
+		break;
+	case DRAM_TYPE_DDR1:
+		dram_type_str = "DDR1";
+		break;
+	case DRAM_TYPE_LPDDR2:
+		dram_type_str = "LPDDR2";
+		break;
+	case DRAM_TYPE_DDR2:
+		dram_type_str = "DDR2";
+		break;
+	}
+
+	emc_adr_cfg = readl_relaxed(emc->regs + EMC_ADR_CFG_0);
+	emem_numdev = FIELD_GET(EMC_ADR_CFG_0_EMEM_NUMDEV, emc_adr_cfg) + 1;
+
+	dev_info_once(emc->dev, "%ubit DRAM bus, %u %s %s attached\n",
+		      emc->dram_bus_width, emem_numdev, dram_type_str,
+		      emem_numdev == 2 ? "devices" : "device");
+
+	if (dram_type == DRAM_TYPE_LPDDR2) {
+		while (emem_numdev--)
+			emc_read_lpddr_sdram_info(emc, emem_numdev,
+						  !print_sdram_info_once);
+		print_sdram_info_once = true;
+	}
 
 	return 0;
 }
@@ -1049,14 +1232,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	emc->clk_nb.notifier_call = tegra_emc_clk_change_notify;
 	emc->dev = &pdev->dev;
 
-	np = tegra_emc_find_node_by_ram_code(&pdev->dev);
-	if (np) {
-		err = tegra_emc_load_timings_from_dt(emc, np);
-		of_node_put(np);
-		if (err)
-			return err;
-	}
-
 	emc->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(emc->regs))
 		return PTR_ERR(emc->regs);
@@ -1065,6 +1240,14 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	np = tegra_emc_find_node_by_ram_code(emc);
+	if (np) {
+		err = tegra_emc_load_timings_from_dt(emc, np);
+		of_node_put(np);
+		if (err)
+			return err;
+	}
+
 	err = devm_request_irq(&pdev->dev, irq, tegra_emc_isr, 0,
 			       dev_name(&pdev->dev), emc);
 	if (err) {
-- 
2.32.0

Re: [PATCH v1 1/2] dt-bindings: memory: tegra20: emc: Document optional LPDDR properties

[Krzysztof Kozlowski]

On 29/09/2021 22:03, Dmitry Osipenko wrote:
> Some Tegra20 boards don't use RAM code for the memory chip identification
> and the identity information should read out from LPDDR chip in this case.
> Document new optional generic LPDDR properties that will be used for the
> memory chip identification if RAM code isn't provided.

Please mention how they are going to be used. Naively I would assume
that these new properties describe the RAM you have. However it seems
you do not use them to configure the device but to compare with the
device. Why do you need them?

> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../nvidia,tegra20-emc.yaml                   | 43 +++++++++++++++++--
>  1 file changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
> index cac6842dc8f1..6d01b1bf6304 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml
> @@ -158,6 +158,46 @@ patternProperties:
>          description:
>            Value of RAM_CODE this timing set is used for.
>  
> +      jedec,lpddr-manufacturer-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Unique manufacturer ID of SDRAM chip this timing set is used for.
> +          See MR5 description in JEDEC LPDDR2 specification (JESD209-2).
> +
> +      jedec,lpddr-revision-id1:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Revision 1 value of SDRAM chip this timing set is used for.
> +          See MR6 description in chip vendor specification.
> +
> +      jedec,lpddr-revision-id2:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Revision 2 value of SDRAM chip this timing set is used for.
> +          See MR7 description in chip vendor specification.
> +
> +      jedec,lpddr-density-mbits:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Density in megabits of SDRAM chip this timing set is used for.
> +          See MR8 description in JEDEC LPDDR2 specification.
> +
> +      jedec,lpddr-io-width-bits:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          IO bus width in bits of SDRAM chip this timing set is used for.
> +          See MR8 description in JEDEC LPDDR2 specification.
> +
> +      jedec,lpddr-type:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          LPDDR type which corresponds to a number of words SDRAM pre-fetches
> +          per column request that this timing set is used for.
> +          See MR8 description in JEDEC LPDDR2 specification.
> +        enum:
> +          - 4 # S4 (4 words prefetch architecture)
> +          - 2 # S2 (2 words prefetch architecture)

I think instead you should use generic lpddr{2,3} bindings - have a
separate node and reference it via a phandle.

Best regards,
Krzysztof

Re: [PATCH v1 1/2] dt-bindings: memory: tegra20: emc: Document optional LPDDR properties

[Dmitry Osipenko]

30.09.2021 09:54, Krzysztof Kozlowski пишет:
> On 29/09/2021 22:03, Dmitry Osipenko wrote:
>> Some Tegra20 boards don't use RAM code for the memory chip identification
>> and the identity information should read out from LPDDR chip in this case.
>> Document new optional generic LPDDR properties that will be used for the
>> memory chip identification if RAM code isn't provided.
> 
> Please mention how they are going to be used. Naively I would assume
> that these new properties describe the RAM you have. However it seems
> you do not use them to configure the device but to compare with the
> device. Why do you need them?

Yes, the properties describe hardware configuration of external DRAM
chip. This information is read-only and it's actually used for
configuring SoC memory controller. This MC configuration is already
pre-configured by bootloader and partially it shouldn't be ever touched
by software. Kernel driver needs to reconfigure only a part of hardware
on memory freq changes. The memory timing data is tuned for a specific
DRAM chip and board, it doesn't include info which identifies the chip.
So we need to read out DRAM config from hardware and find the matching
timing in a device-tree by comparing the chip-unique properties. Note
that only LPDDR chips have that chip-identity info. Regular DDR chips
require SPD or other means, like NVMEM in case of Tegra.

I'll extend the commit message.

...
>> +          - 4 # S4 (4 words prefetch architecture)
>> +          - 2 # S2 (2 words prefetch architecture)
> 
> I think instead you should use generic lpddr{2,3} bindings - have a
> separate node and reference it via a phandle.

It indeed shouldn't be a problem to create lpddr binding and move these
props there.

Extra phandle shouldn't be needed, should be fine to keep these new DRAM
properties within the chip-descriptor nodes that we already have in
tegra device-trees. We'll only need to $ref the lpddr binding for the
descriptor node in the binding. I.e. to make it similar to regulator
bindings where there is generic regulator.yaml + hw-specific properties.

I'll try to implement this in v2, thanks!