Re: [PATCH v2 7/9] ram: octeon: Add MIPS Octeon3 DDR4 support (part 3/3)

Hi Daniel,

On 19.08.20 16:58, Daniel Schwierzeck wrote:

Am Montag, den 17.08.2020, 14:12 +0200 schrieb Stefan Roese:

...

From: Aaron Williams awilliams@marvell.com

This Octeon 3 DDR driver is ported from the 2013 Cavium / Marvell U-Boot repository. It currently supports DDR4 on Octeon 3. It can be later extended to support also DDR3 and Octeon 2 platforms.

Part 3 includes the DIMM SPD handling code and the Kconfig / Makefile integration.

Signed-off-by: Aaron Williams awilliams@marvell.com Signed-off-by: Stefan Roese sr@denx.de

---

(no changes since v1)

drivers/ram/Kconfig | 1 + drivers/ram/Makefile | 2 + drivers/ram/octeon/Kconfig | 17 ++ drivers/ram/octeon/Makefile | 8 + drivers/ram/octeon/dimm_spd_eeprom.c | 410 +++++++++++++++++++++++++++ 5 files changed, 438 insertions(+) create mode 100644 drivers/ram/octeon/Kconfig create mode 100644 drivers/ram/octeon/Makefile create mode 100644 drivers/ram/octeon/dimm_spd_eeprom.c

diff --git a/drivers/ram/Kconfig b/drivers/ram/Kconfig index 7e6e981897..a0e859afd6 100644 --- a/drivers/ram/Kconfig +++ b/drivers/ram/Kconfig @@ -76,3 +76,4 @@ config IMXRT_SDRAM source "drivers/ram/rockchip/Kconfig" source "drivers/ram/sifive/Kconfig" source "drivers/ram/stm32mp1/Kconfig" +source "drivers/ram/octeon/Kconfig" diff --git a/drivers/ram/Makefile b/drivers/ram/Makefile index 769c9d6218..d685a579a0 100644 --- a/drivers/ram/Makefile +++ b/drivers/ram/Makefile @@ -19,3 +19,5 @@ obj-$(CONFIG_K3_J721E_DDRSS) += k3-j721e/ obj-$(CONFIG_IMXRT_SDRAM) += imxrt_sdram.o

obj-$(CONFIG_RAM_SIFIVE) += sifive/

+obj-$(CONFIG_ARCH_OCTEON) += octeon/ diff --git a/drivers/ram/octeon/Kconfig b/drivers/ram/octeon/Kconfig new file mode 100644 index 0000000000..eb5a1208ed --- /dev/null +++ b/drivers/ram/octeon/Kconfig @@ -0,0 +1,17 @@ +config RAM_OCTEON

• bool "Ram drivers for Octeon SoCs"
• depends on RAM && ARCH_OCTEON
• default n
• help

• This enables support for RAM drivers for Octeon SoCs.
  

+if RAM_OCTEON

+config RAM_OCTEON_DDR4

• bool "Octeon III DDR4 RAM support"
• default n
• help

• This enables support for DDR4 RAM suppoort for Octeon III.  This does
  

• not include support for Octeon CN70XX.
  

+endif # RAM_OCTEON diff --git a/drivers/ram/octeon/Makefile b/drivers/ram/octeon/Makefile new file mode 100644 index 0000000000..27649d1e6f --- /dev/null +++ b/drivers/ram/octeon/Makefile @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (c) 2020 Marvell, Inc. +#

+obj-$(CONFIG_RAM_OCTEON_DDR4) += octeon_ddr.o +obj-$(CONFIG_RAM_OCTEON_DDR4) += octeon3_lmc.o +obj-y += dimm_spd_eeprom.o diff --git a/drivers/ram/octeon/dimm_spd_eeprom.c b/drivers/ram/octeon/dimm_spd_eeprom.c new file mode 100644 index 0000000000..efff3be71f --- /dev/null +++ b/drivers/ram/octeon/dimm_spd_eeprom.c @@ -0,0 +1,410 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • Copyright (C) 2020 Marvell International Ltd.
• • https://spdx.org/licenses

this line is superfluous

Yes.

...

• */

+#include <common.h>

try to avoid common.h in new code

Yes.

...

+#include <i2c.h> +#include <ram.h>

+#include <mach/octeon_ddr.h>

+#define DEVICE_TYPE DDR4_SPD_KEY_BYTE_DEVICE_TYPE // same for DDR3 and DDR4 +#define MODULE_TYPE DDR4_SPD_KEY_BYTE_MODULE_TYPE // same for DDR3 and DDR4 +#define BUS_WIDTH(t) (((t) == DDR4_DRAM) ? \

• 	 DDR4_SPD_MODULE_MEMORY_BUS_WIDTH : \
  

• 	 DDR3_SPD_MEMORY_BUS_WIDTH)
  

+/*

• • Allow legacy code to encode bus number in the upper bits of the address
• • These are only supported in read_spd()
• */

+#define OCTEON_TWSI_BUS_IN_ADDR_BIT 12 +#define OCTEON_TWSI_BUS_IN_ADDR_MASK (15 << OCTEON_TWSI_BUS_IN_ADDR_BIT) +#define OCTEON_TWSI_GET_BUS(addr) \

• (((addr) >> OCTEON_TWSI_BUS_IN_ADDR_BIT) & 0xf)

+const char *ddr3_dimm_types[] = {

• /* 0000 */ "Undefined",
• /* 0001 */ "RDIMM",
• /* 0010 */ "UDIMM",
• /* 0011 */ "SO-DIMM",
• /* 0100 */ "Micro-DIMM",
• /* 0101 */ "Mini-RDIMM",
• /* 0110 */ "Mini-UDIMM",
• /* 0111 */ "Mini-CDIMM",
• /* 1000 */ "72b-SO-UDIMM",
• /* 1001 */ "72b-SO-RDIMM",
• /* 1010 */ "72b-SO-CDIMM"
• /* 1011 */ "LRDIMM",
• /* 1100 */ "16b-SO-DIMM",
• /* 1101 */ "32b-SO-DIMM",
• /* 1110 */ "Reserved",
• /* 1111 */ "Reserved"

+};

+const char *ddr4_dimm_types[] = {

• /* 0000 */ "Extended",
• /* 0001 */ "RDIMM",
• /* 0010 */ "UDIMM",
• /* 0011 */ "SO-DIMM",
• /* 0100 */ "LRDIMM",
• /* 0101 */ "Mini-RDIMM",
• /* 0110 */ "Mini-UDIMM",
• /* 0111 */ "Reserved",
• /* 1000 */ "72b-SO-RDIMM",
• /* 1001 */ "72b-SO-UDIMM",
• /* 1010 */ "Reserved",
• /* 1011 */ "Reserved",
• /* 1100 */ "16b-SO-DIMM",
• /* 1101 */ "32b-SO-DIMM",
• /* 1110 */ "Reserved",
• /* 1111 */ "Reserved"

+};

+static u16 ddr3_crc16(u8 *ptr, int count) +{

• /* From DDR3 SPD specification */
• int crc, i;
• crc = 0;
• while (--count >= 0) {

• crc = crc ^ (int)*ptr++ << 8;
  

• for (i = 0; i < 8; ++i) {
  

• 	if (crc & 0x8000)
  

• 		crc = crc << 1 ^ 0x1021;
  

• 	else
  

• 		crc = crc << 1;
  

• }
  

• }
• return (crc & 0xFFFF);

+}

+static int validate_spd_checksum_ddr4(struct dimm_config *dimm_config,

• 		      int dimm_index, int twsi_addr, int silent)
  

+{

• u8 *spd_data = dimm_config->spd_data[dimm_index];
• int crc_bytes = 126;
• u16 crc_comp;
• /* Check byte 0 to see how many bytes checksum is over */
• if (spd_data[0] & 0x80)

• crc_bytes = 117;
  

• crc_comp = ddr3_crc16(spd_data, crc_bytes);
• if (spd_data[126] == (crc_comp & 0xff) &&

•    spd_data[127] == (crc_comp >> 8))
  

• return 1;
  

• if (!silent) {

• printf("DDR4 SPD CRC error, spd addr: 0x%x, calculated crc: 0x%04x, read crc: 0x%02x%02x\n",
  

•        twsi_addr, crc_comp, spd_data[127], spd_data[126]);
  

• }
• return 0;

+}

+static int validate_spd_checksum(struct ddr_priv *priv,

• 		 struct dimm_config *dimm_config,
  

• 		 int dimm_index, int twsi_addr,
  

• 		 int silent, u8 rv)
  

+{

• if (ddr_verbose(priv))

• debug("Validating DIMM at address 0x%x\n", twsi_addr);
  

• if (rv >= 0x8 && rv <= 0xA)

• printf("%s: Error: DDR2 support disabled\n", __func__);
  

• if (rv == 0xB)

• printf("%s: Error: DDR3 support disabled\n", __func__);
  

• if (rv == 0xC) {

• return validate_spd_checksum_ddr4(dimm_config, dimm_index,
  

• 				  twsi_addr, silent);
  

• }
• if (!silent) {

• printf("Unrecognized DIMM type: 0x%x at spd address: 0x%x\n",
  

•        rv, twsi_addr);
  

• }
• return 0;

+}

+/*

• • Read an DIMM SPD value, either using TWSI to read it from the DIMM, or
• • from a provided array.
• */

+int read_spd(struct dimm_config *dimm_config, int dimm_index, int spd_field) +{

• dimm_index = !!dimm_index;
• if (spd_field >= SPD_EEPROM_SIZE) {

• printf("ERROR: Trying to read unsupported SPD EEPROM value %d\n",
  

•        spd_field);
  

• }
• /*

• * If pointer to data is provided, use it, otherwise read from SPD
  

• * over twsi
  

• */
  

• if (dimm_config->spd_ptrs[dimm_index])

• return dimm_config->spd_ptrs[dimm_index][spd_field];
  

• else if (dimm_config->spd_addrs[dimm_index])

• return dimm_config->spd_data[dimm_index][spd_field];
  

• return -1;

+}

+int read_spd_init(struct dimm_config *dimm_config, int dimm_index) +{

• u8 busno = OCTEON_TWSI_GET_BUS(dimm_config->spd_addrs[dimm_index]);
• u8 cmdno = dimm_config->spd_addrs[dimm_index];

this is the only Octeon specific part in this code. So I think this file could be implemented as a generic SPD EEPROM driver where busno and/or cmdno could be configured via device-tree.

Please note that this code relies heavily on the Octeon specific structs like "struct dimm_config". Rewriting this in some platform independant way would be a quite big task - with the additional work to transform all data from generic infos to the Octeon structs.

As you might know, many other DDR drivers in U-Boot implement their own SPD DIMM infrastructure. This is definitely not good, I agree. If I would have written this Octeon DDR driver from scratch, I might have tried to combine some of these other SPD DIMM drivers into a common code base. But while porting this quite old code from 2013 U-Boot, it did not really fit IMHO.

Thanks, Stefan

...

• struct udevice *dev_i2c;
• u8 *spd_data;
• int ret;
• if (dimm_config->spd_cached[dimm_index])

• return 0;
  

• dimm_config->spd_cached[dimm_index] = 1;
• spd_data = dimm_config->spd_data[dimm_index];
• ret = i2c_get_chip_for_busnum(busno, cmdno, 2, &dev_i2c);
• if (ret) {

• debug("Cannot find SPL EEPROM: %d\n", ret);
  

• return -ENODEV;
  

• }
• ret = dm_i2c_read(dev_i2c, 0, spd_data, SPD_EEPROM_SIZE);
• return ret;

+}

+int validate_dimm(struct ddr_priv *priv, struct dimm_config *dimm_config,

•   int dimm_index)
  

+{

• int spd_addr;
• dimm_index = !!dimm_index; /* Normalize to 0/1 */
• spd_addr = dimm_config->spd_addrs[dimm_index];
• debug("Validating dimm %d, spd addr: 0x%02x spd ptr: %p\n",

•      dimm_index,
  

•      dimm_config->spd_addrs[dimm_index],
  

•      dimm_config->spd_ptrs[dimm_index]);
  

• /* Only validate 'real' dimms, assume compiled in values are OK */
• if (!dimm_config->spd_ptrs[dimm_index]) {

• int val0, val1;
  

• int dimm_type;
  

• int ret;
  

• ret = read_spd_init(dimm_config, dimm_index);
  

• if (ret)
  

• 	return 0;
  

• dimm_type = read_spd(dimm_config, dimm_index,
  

• 		     DDR2_SPD_MEM_TYPE) & 0xff;
  

• switch (dimm_type) {
  

• case 0x0B:              /* DDR3 */
  

• 	if (ddr_verbose(priv))
  

• 		printf("Validating DDR3 DIMM %d\n", dimm_index);
  

• 	val0 = read_spd(dimm_config, dimm_index,
  

• 			DDR3_SPD_DENSITY_BANKS);
  

• 	val1 = read_spd(dimm_config, dimm_index,
  

• 			DDR3_SPD_ADDRESSING_ROW_COL_BITS);
  

• 	if (val0 < 0 && val1 < 0) {
  

• 		if (ddr_verbose(priv))
  

• 			printf("Error reading SPD for DIMM %d\n",
  

• 			       dimm_index);
  

• 		return 0; /* Failed to read dimm */
  

• 	}
  

• 	if (val0 == 0xff && val1 == 0xff) {
  

• 		if (ddr_verbose(priv))
  

• 			printf("Blank or unreadable SPD for DIMM %d\n",
  

• 			       dimm_index);
  

• 		/* Blank SPD or otherwise unreadable device */
  

• 		return 0;
  

• 	}
  

• 	/* Don't treat bad checksums as fatal */
  

• 	validate_spd_checksum(priv, dimm_config, dimm_index,
  

• 			      spd_addr, 0, dimm_type);
  

• 	break;
  

• case 0x0C:              /* DDR4 */
  

• 	if (ddr_verbose(priv))
  

• 		printf("Validating DDR4 DIMM %d\n", dimm_index);
  

• 	val0 = read_spd(dimm_config, dimm_index,
  

• 			DDR4_SPD_DENSITY_BANKS);
  

• 	val1 = read_spd(dimm_config, dimm_index,
  

• 			DDR4_SPD_ADDRESSING_ROW_COL_BITS);
  

• 	if (val0 < 0 && val1 < 0) {
  

• 		if (ddr_verbose(priv))
  

• 			printf("Error reading SPD for DIMM %d\n",
  

• 			       dimm_index);
  

• 		return 0; /* Failed to read dimm */
  

• 	}
  

• 	if (val0 == 0xff && val1 == 0xff) {
  

• 		if (ddr_verbose(priv)) {
  

• 			printf("Blank or unreadable SPD for DIMM %d\n",
  

• 			       dimm_index);
  

• 		}
  

• 		/* Blank SPD or otherwise unreadable device */
  

• 		return 0;
  

• 	}
  

• 	/* Don't treat bad checksums as fatal */
  

• 	validate_spd_checksum(priv, dimm_config, dimm_index,
  

• 			      spd_addr, 0, dimm_type);
  

• 	break;
  

• case 0x00:
  

• 	/* Terminator detected. Fail silently. */
  

• 	return 0;
  

• default:
  

• 	debug("Unknown DIMM type 0x%x for DIMM %d @ 0x%x\n",
  

• 	      dimm_type, dimm_index,
  

• 	      dimm_config->spd_addrs[dimm_index]);
  

• 	return 0;      /* Failed to read dimm */
  

• }
  

• }
• return 1;

+}

+int get_ddr_type(struct dimm_config *dimm_config, int upper_dimm) +{

• int spd_ddr_type;
• spd_ddr_type = read_spd(dimm_config, upper_dimm, DEVICE_TYPE);
• debug("%s:%d spd_ddr_type=0x%02x\n", __func__, __LINE__,

•      spd_ddr_type);
  

• /* we return only DDR4 or DDR3 */
• return (spd_ddr_type == 0x0C) ? DDR4_DRAM : DDR3_DRAM;

+}

+static int get_dimm_ecc(struct dimm_config *dimm_config, int upper_dimm,

• 	int ddr_type)
  

+{

• return !!(read_spd(dimm_config, upper_dimm, BUS_WIDTH(ddr_type)) & 8);

+}

+int get_dimm_module_type(struct dimm_config *dimm_config, int upper_dimm,

• 	 int ddr_type)
  

+{

• return read_spd(dimm_config, upper_dimm, MODULE_TYPE) & 0x0f;

+}

+char *printable_rank_spec(char *buffer, int num_ranks, int dram_width,

• 	  int spd_package)
  

+{

• int die_count = ((spd_package >> 4) & 7) + 1;
• if (spd_package & 0x80) { // non-monolithic

• if ((spd_package & 3) == 2) { // 3DS
  

• 	sprintf(buffer, "%dS%dRx%d", num_ranks, die_count,
  

• 		dram_width);
  

• } else { // MLS
  

• 	char hchar = (die_count == 2) ? 'D' : 'Q';
  

• 	sprintf(buffer, "%d%cRx%d", num_ranks, hchar,
  

• 		dram_width);
  

• }
  

• } else {

• sprintf(buffer, "%dRx%d", num_ranks, dram_width);
  

• }
• return buffer;

+}

+static void report_common_dimm(struct dimm_config *dimm_config, int upper_dimm,

• 	       int dimm, const char **dimm_types, int ddr_type,
  

• 	       char *volt_str, int if_num,
  

• 	       int num_ranks, int dram_width, int spd_package)
  

+{

• unsigned int spd_module_type;
• char rank_spec[8];
• int spd_ecc;
• spd_module_type = get_dimm_module_type(dimm_config, upper_dimm,

• 			       ddr_type);
  

• spd_ecc = get_dimm_ecc(dimm_config, upper_dimm, ddr_type);
• printable_rank_spec(rank_spec, num_ranks, dram_width, spd_package);
• printf("LMC%d.DIMM%d: DDR%d %s %s %s, %s\n",

•       if_num, dimm, ddr_type, dimm_types[spd_module_type],
  

•       rank_spec, spd_ecc ? "ECC" : "non-ECC", volt_str);
  

+}

+static void report_ddr3_dimm(struct dimm_config *dimm_config, int upper_dimm,

• 	     int dimm, int if_num)
  

+{

• int spd_voltage;
• char *volt_str;
• int spd_org = read_spd(dimm_config, upper_dimm,

• 	       DDR3_SPD_MODULE_ORGANIZATION);
  

• int num_ranks = 1 + ((spd_org >> 3) & 0x7);
• int dram_width = 4 << ((spd_org >> 0) & 0x7);
• spd_voltage = read_spd(dimm_config, upper_dimm,

• 	       DDR3_SPD_NOMINAL_VOLTAGE);
  

• if (spd_voltage == 0 || spd_voltage & 3)

• volt_str = "1.5V";
  

• if (spd_voltage & 2)

• volt_str = "1.35V";
  

• if (spd_voltage & 4)

• volt_str = "1.2xV";
  

• report_common_dimm(dimm_config, upper_dimm, dimm, ddr3_dimm_types,

• 	   DDR3_DRAM, volt_str, if_num,
  

• 	   num_ranks, dram_width, /*spd_package*/0);
  

+}

+static void report_ddr4_dimm(struct dimm_config *dimm_config, int upper_dimm,

• 	     int dimm, int if_num)
  

+{

• int spd_voltage;
• char *volt_str;
• int spd_package = 0xff & read_spd(dimm_config, upper_dimm,

• 			  DDR4_SPD_PACKAGE_TYPE);
  

• int spd_org = 0xff & read_spd(dimm_config, upper_dimm,

• 			  DDR4_SPD_MODULE_ORGANIZATION);
  

• int num_ranks = 1 + ((spd_org >> 3) & 0x7);
• int dram_width = 4 << ((spd_org >> 0) & 0x7);
• spd_voltage = read_spd(dimm_config, upper_dimm,

• 	       DDR4_SPD_MODULE_NOMINAL_VOLTAGE);
  

• if (spd_voltage == 0x01 || spd_voltage & 0x02)

• volt_str = "1.2V";
  

• if (spd_voltage == 0x04 || spd_voltage & 0x08)

• volt_str = "TBD1 V";
  

• if (spd_voltage == 0x10 || spd_voltage & 0x20)

• volt_str = "TBD2 V";
  

• report_common_dimm(dimm_config, upper_dimm, dimm, ddr4_dimm_types,

• 	   DDR4_DRAM, volt_str, if_num,
  

• 	   num_ranks, dram_width, spd_package);
  

+}

+void report_dimm(struct dimm_config *dimm_config, int upper_dimm,

•  int dimm, int if_num)
  

+{

• int ddr_type;
• /* ddr_type only indicates DDR4 or DDR3 */
• ddr_type = get_ddr_type(dimm_config, upper_dimm);
• if (ddr_type == DDR4_DRAM)

• report_ddr4_dimm(dimm_config, 0, dimm, if_num);
  

• else

• report_ddr3_dimm(dimm_config, 0, dimm, if_num);
  

+}

Viele Grüße, Stefan