[PATCH 0/7] Support generating DIMM parameters on more systems

[PATCH 0/7] Support generating DIMM parameters on more systems

[John Watts]

Boards that with swappable RAM sticks need to read SPD data and generate
useful parameters for the board's memory controller.

Currently the only board that does this is the ls1046ardb.
This patch series modifies and re-organizes that code so it's usable by
other boards.

I have used and tested this patch series on the i.MX6 and ensured the
existing board still compiles.

There is one API change: spd_read_eeprom now takes a memtype parameter.
This is required as the current code only supports reading DDR4 chips
while the i.MX6 uses DDR3 chips.

John Watts (7):
  I2C: i.MX: Add early i2c support for i.MX6
  ddr: fsl: Pass mclk_ps to ddr*_compute_dimm_parameters
  ddr_dimms: Move FSL dimm_params to include/ddr_dimms.h
  ddr: fsl: Remove includes to fsl-specific code
  ddr_dimms: Move ddr*_dimm_params to common
  ddr_dimms: Remove mclk_ps for DDR3 and DDR4
  ddr_spd: Support reading SPD from DDR3 sticks

 arch/arm/boards/ls1046ardb/lowlevel.c         |   2 +-
 common/Makefile                               |   4 +
 .../ddr/fsl => common}/ddr1_dimm_params.c     |  13 +-
 .../ddr/fsl => common}/ddr2_dimm_params.c     |   7 +-
 .../ddr/fsl => common}/ddr3_dimm_params.c     |   6 +-
 .../ddr/fsl => common}/ddr4_dimm_params.c     |   6 +-
 common/ddr_spd.c                              |  16 ++-
 drivers/ddr/fsl/Makefile                      |   4 -
 drivers/ddr/fsl/fsl_ddr.h                     |  12 --
 drivers/ddr/fsl/main.c                        |   9 +-
 drivers/i2c/busses/i2c-imx-early.c            |  14 +++
 include/ddr_dimms.h                           | 114 ++++++++++++++++++
 include/ddr_spd.h                             |   3 +-
 include/pbl/i2c.h                             |   1 +
 include/soc/fsl/fsl_ddr_sdram.h               |  92 +-------------
 15 files changed, 166 insertions(+), 137 deletions(-)
 rename {drivers/ddr/fsl => common}/ddr1_dimm_params.c (96%)
 rename {drivers/ddr/fsl => common}/ddr2_dimm_params.c (98%)
 rename {drivers/ddr/fsl => common}/ddr3_dimm_params.c (98%)
 rename {drivers/ddr/fsl => common}/ddr4_dimm_params.c (98%)
 create mode 100644 include/ddr_dimms.h

-- 
2.39.0

[PATCH 1/7] I2C: i.MX: Add early i2c support for i.MX6

[John Watts]

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/i2c/busses/i2c-imx-early.c | 14 ++++++++++++++
 include/pbl/i2c.h                  |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx-early.c b/drivers/i2c/busses/i2c-imx-early.c
index 4e0f7e517d..6c8bdc7904 100644
--- a/drivers/i2c/busses/i2c-imx-early.c
+++ b/drivers/i2c/busses/i2c-imx-early.c
@@ -303,6 +303,20 @@ struct pbl_i2c *ls1046_i2c_init(void __iomem *regs)
 	return &fsl_i2c.i2c;
 }
 
+struct pbl_i2c *imx6_i2c_early_init(void __iomem *regs)
+{
+	fsl_i2c.regs = regs;
+	fsl_i2c.regshift = 2;
+	fsl_i2c.i2cr_ien_opcode = I2CR_IEN_OPCODE_1;
+	fsl_i2c.i2sr_clr_opcode = I2SR_CLR_OPCODE_W0C;
+	/* Divider for ~100kHz when coming from the ROM */
+	fsl_i2c.ifdr = 0x36;
+
+	fsl_i2c.i2c.xfer = i2c_fsl_xfer;
+
+	return &fsl_i2c.i2c;
+}
+
 struct pbl_i2c *imx8m_i2c_early_init(void __iomem *regs)
 {
 	fsl_i2c.regs = regs;
diff --git a/include/pbl/i2c.h b/include/pbl/i2c.h
index d9910f531f..32e456d46b 100644
--- a/include/pbl/i2c.h
+++ b/include/pbl/i2c.h
@@ -16,6 +16,7 @@ static inline int pbl_i2c_xfer(struct pbl_i2c *i2c,
 }
 
 struct pbl_i2c *imx8m_i2c_early_init(void __iomem *regs);
+struct pbl_i2c *imx6_i2c_early_init(void __iomem *regs);
 struct pbl_i2c *ls1046_i2c_init(void __iomem *regs);
 
 static inline int i2c_dev_probe(struct pbl_i2c *i2c, int addr, bool onebyte)
-- 
2.39.0

Re: [PATCH 1/7] I2C: i.MX: Add early i2c support for i.MX6

[Marco Felsch]

Hi John,

thanks for your patch.

albeit it is obvious we tend to have a small commit message here. The
code change lgtm.

Regards,
  Marco

On 23-01-22, John Watts wrote:
> Signed-off-by: John Watts <contact@jookia.org>
> ---
>  drivers/i2c/busses/i2c-imx-early.c | 14 ++++++++++++++
>  include/pbl/i2c.h                  |  1 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-imx-early.c b/drivers/i2c/busses/i2c-imx-early.c
> index 4e0f7e517d..6c8bdc7904 100644
> --- a/drivers/i2c/busses/i2c-imx-early.c
> +++ b/drivers/i2c/busses/i2c-imx-early.c
> @@ -303,6 +303,20 @@ struct pbl_i2c *ls1046_i2c_init(void __iomem *regs)
>  	return &fsl_i2c.i2c;
>  }
>  
> +struct pbl_i2c *imx6_i2c_early_init(void __iomem *regs)
> +{
> +	fsl_i2c.regs = regs;
> +	fsl_i2c.regshift = 2;
> +	fsl_i2c.i2cr_ien_opcode = I2CR_IEN_OPCODE_1;
> +	fsl_i2c.i2sr_clr_opcode = I2SR_CLR_OPCODE_W0C;
> +	/* Divider for ~100kHz when coming from the ROM */
> +	fsl_i2c.ifdr = 0x36;
> +
> +	fsl_i2c.i2c.xfer = i2c_fsl_xfer;
> +
> +	return &fsl_i2c.i2c;
> +}
> +
>  struct pbl_i2c *imx8m_i2c_early_init(void __iomem *regs)
>  {
>  	fsl_i2c.regs = regs;
> diff --git a/include/pbl/i2c.h b/include/pbl/i2c.h
> index d9910f531f..32e456d46b 100644
> --- a/include/pbl/i2c.h
> +++ b/include/pbl/i2c.h
> @@ -16,6 +16,7 @@ static inline int pbl_i2c_xfer(struct pbl_i2c *i2c,
>  }
>  
>  struct pbl_i2c *imx8m_i2c_early_init(void __iomem *regs);
> +struct pbl_i2c *imx6_i2c_early_init(void __iomem *regs);
>  struct pbl_i2c *ls1046_i2c_init(void __iomem *regs);
>  
>  static inline int i2c_dev_probe(struct pbl_i2c *i2c, int addr, bool onebyte)
> -- 
> 2.39.0
> 
> 
> 

Re: [PATCH 1/7] I2C: i.MX: Add early i2c support for i.MX6

[John Watts]

Thanks Marco,

The lack of commit message is a good catch though I'm not entirely sure
what to write in it.

One criticism of this patch I forgot to note is it it needs udelay() defined in
the PBL by the board. It might be better to move that in to this patch.

John.

On Mon, Jan 23, 2023 at 10:01:23AM +0100, Marco Felsch wrote:
> Hi John,
> 
> thanks for your patch.
> 
> albeit it is obvious we tend to have a small commit message here. The
> code change lgtm.
> 
> Regards,
>   Marco
> 
> On 23-01-22, John Watts wrote:
> > Signed-off-by: John Watts <contact@jookia.org>
> > ---
> >  drivers/i2c/busses/i2c-imx-early.c | 14 ++++++++++++++
> >  include/pbl/i2c.h                  |  1 +
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/i2c/busses/i2c-imx-early.c b/drivers/i2c/busses/i2c-imx-early.c
> > index 4e0f7e517d..6c8bdc7904 100644
> > --- a/drivers/i2c/busses/i2c-imx-early.c
> > +++ b/drivers/i2c/busses/i2c-imx-early.c
> > @@ -303,6 +303,20 @@ struct pbl_i2c *ls1046_i2c_init(void __iomem *regs)
> >  	return &fsl_i2c.i2c;
> >  }
> >  
> > +struct pbl_i2c *imx6_i2c_early_init(void __iomem *regs)
> > +{
> > +	fsl_i2c.regs = regs;
> > +	fsl_i2c.regshift = 2;
> > +	fsl_i2c.i2cr_ien_opcode = I2CR_IEN_OPCODE_1;
> > +	fsl_i2c.i2sr_clr_opcode = I2SR_CLR_OPCODE_W0C;
> > +	/* Divider for ~100kHz when coming from the ROM */
> > +	fsl_i2c.ifdr = 0x36;
> > +
> > +	fsl_i2c.i2c.xfer = i2c_fsl_xfer;
> > +
> > +	return &fsl_i2c.i2c;
> > +}
> > +
> >  struct pbl_i2c *imx8m_i2c_early_init(void __iomem *regs)
> >  {
> >  	fsl_i2c.regs = regs;
> > diff --git a/include/pbl/i2c.h b/include/pbl/i2c.h
> > index d9910f531f..32e456d46b 100644
> > --- a/include/pbl/i2c.h
> > +++ b/include/pbl/i2c.h
> > @@ -16,6 +16,7 @@ static inline int pbl_i2c_xfer(struct pbl_i2c *i2c,
> >  }
> >  
> >  struct pbl_i2c *imx8m_i2c_early_init(void __iomem *regs);
> > +struct pbl_i2c *imx6_i2c_early_init(void __iomem *regs);
> >  struct pbl_i2c *ls1046_i2c_init(void __iomem *regs);
> >  
> >  static inline int i2c_dev_probe(struct pbl_i2c *i2c, int addr, bool onebyte)
> > -- 
> > 2.39.0
> > 
> > 
> > 

[PATCH 2/7] ddr: fsl: Pass mclk_ps to ddr*_compute_dimm_parameters

[John Watts]

This is in preparation for use for generic SPD calculations.

I had to also rewrite uses of mclk_to_picos like this:

-       pdimm->trtp_ps = mclk_to_picos(c, 2);   /* By the book. */
+       pdimm->trtp_ps = mclk_ps * 2;   /* By the book. */

This is the same result as:

mclk_to_picos(c, mclk) expands to: get_memory_clk_period_ps(c) * mclk,
and that can just be rewritten as mclk_ps * mclk.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/ddr/fsl/ddr1_dimm_params.c | 10 +++++-----
 drivers/ddr/fsl/ddr2_dimm_params.c |  4 ++--
 drivers/ddr/fsl/ddr3_dimm_params.c |  2 +-
 drivers/ddr/fsl/ddr4_dimm_params.c |  2 +-
 drivers/ddr/fsl/fsl_ddr.h          |  8 ++++----
 drivers/ddr/fsl/main.c             |  9 +++++----
 6 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/ddr/fsl/ddr1_dimm_params.c b/drivers/ddr/fsl/ddr1_dimm_params.c
index f5f9067073..9a7f26bd49 100644
--- a/drivers/ddr/fsl/ddr1_dimm_params.c
+++ b/drivers/ddr/fsl/ddr1_dimm_params.c
@@ -216,7 +216,7 @@ compute_derated_DDR1_CAS_latency(unsigned int mclk_ps)
  *
  * FIXME: use #define for the retvals
  */
-unsigned int ddr1_compute_dimm_parameters(struct fsl_ddr_controller *c,
+unsigned int ddr1_compute_dimm_parameters(unsigned int mclk_ps,
 					  const struct ddr1_spd_eeprom *spd,
 					  struct dimm_params *pdimm)
 {
@@ -288,15 +288,15 @@ unsigned int ddr1_compute_dimm_parameters(struct fsl_ddr_controller *c,
 
 	/* Compute CAS latencies below that defined by SPD */
 	pdimm->caslat_lowest_derated = compute_derated_DDR1_CAS_latency(
-					get_memory_clk_period_ps(c));
+					mclk_ps);
 
 	/* Compute timing parameters */
 	pdimm->trcd_ps = spd->trcd * 250;
 	pdimm->trp_ps = spd->trp * 250;
 	pdimm->tras_ps = spd->tras * 1000;
 
-	pdimm->twr_ps = mclk_to_picos(c, 3);
-	pdimm->twtr_ps = mclk_to_picos(c, 1);
+	pdimm->twr_ps = mclk_ps * 3;
+	pdimm->twtr_ps = mclk_ps * 1;
 	pdimm->trfc_ps = compute_trfc_ps_from_spd(0, spd->trfc);
 
 	pdimm->trrd_ps = spd->trrd * 250;
@@ -311,7 +311,7 @@ unsigned int ddr1_compute_dimm_parameters(struct fsl_ddr_controller *c,
 	pdimm->tdh_ps
 		= convert_bcd_hundredths_to_cycle_time_ps(spd->data_hold);
 
-	pdimm->trtp_ps = mclk_to_picos(c, 2);	/* By the book. */
+	pdimm->trtp_ps = mclk_ps * 2;	/* By the book. */
 	pdimm->tdqsq_max_ps = spd->tdqsq * 10;
 	pdimm->tqhs_ps = spd->tqhs * 10;
 
diff --git a/drivers/ddr/fsl/ddr2_dimm_params.c b/drivers/ddr/fsl/ddr2_dimm_params.c
index e33a8ded48..6f17d55c1f 100644
--- a/drivers/ddr/fsl/ddr2_dimm_params.c
+++ b/drivers/ddr/fsl/ddr2_dimm_params.c
@@ -201,7 +201,7 @@ compute_derated_DDR2_CAS_latency(unsigned int mclk_ps)
  *
  * FIXME: use #define for the retvals
  */
-unsigned int ddr2_compute_dimm_parameters(struct fsl_ddr_controller *c,
+unsigned int ddr2_compute_dimm_parameters(unsigned int mclk_ps,
 					  const struct ddr2_spd_eeprom *spd,
 					  struct dimm_params *pdimm)
 {
@@ -289,7 +289,7 @@ unsigned int ddr2_compute_dimm_parameters(struct fsl_ddr_controller *c,
 
 	/* Compute CAS latencies below that defined by SPD */
 	pdimm->caslat_lowest_derated = compute_derated_DDR2_CAS_latency(
-					get_memory_clk_period_ps(c));
+					mclk_ps);
 
 	/* Compute timing parameters */
 	pdimm->trcd_ps = spd->trcd * 250;
diff --git a/drivers/ddr/fsl/ddr3_dimm_params.c b/drivers/ddr/fsl/ddr3_dimm_params.c
index 92012a5af9..cfea35ca05 100644
--- a/drivers/ddr/fsl/ddr3_dimm_params.c
+++ b/drivers/ddr/fsl/ddr3_dimm_params.c
@@ -79,7 +79,7 @@ compute_ranksize(const struct ddr3_spd_eeprom *spd)
  * Writes the results to the struct dimm_params structure pointed by pdimm.
  *
  */
-unsigned int ddr3_compute_dimm_parameters(struct fsl_ddr_controller *c,
+unsigned int ddr3_compute_dimm_parameters(unsigned int mclk_ps,
 					 const struct ddr3_spd_eeprom *spd,
 					 struct dimm_params *pdimm)
 {
diff --git a/drivers/ddr/fsl/ddr4_dimm_params.c b/drivers/ddr/fsl/ddr4_dimm_params.c
index 0be2de8de6..41c63f3613 100644
--- a/drivers/ddr/fsl/ddr4_dimm_params.c
+++ b/drivers/ddr/fsl/ddr4_dimm_params.c
@@ -124,7 +124,7 @@ compute_ranksize(const struct ddr4_spd_eeprom *spd)
  * Writes the results to the struct dimm_params structure pointed by pdimm.
  *
  */
-unsigned int ddr4_compute_dimm_parameters(struct fsl_ddr_controller *c,
+unsigned int ddr4_compute_dimm_parameters(unsigned int mclk_ps,
 					  const struct ddr4_spd_eeprom *spd,
 					  struct dimm_params *pdimm)
 {
diff --git a/drivers/ddr/fsl/fsl_ddr.h b/drivers/ddr/fsl/fsl_ddr.h
index 459a7ee8e8..87edd10ac1 100644
--- a/drivers/ddr/fsl/fsl_ddr.h
+++ b/drivers/ddr/fsl/fsl_ddr.h
@@ -204,16 +204,16 @@ struct fsl_ddr_controller;
 
 u32 fsl_ddr_get_version(struct fsl_ddr_controller *c);
 
-unsigned int ddr1_compute_dimm_parameters(struct fsl_ddr_controller *c,
+unsigned int ddr1_compute_dimm_parameters(unsigned int mclk_ps,
 					  const struct ddr1_spd_eeprom *spd,
 					  struct dimm_params *pdimm);
-unsigned int ddr2_compute_dimm_parameters(struct fsl_ddr_controller *c,
+unsigned int ddr2_compute_dimm_parameters(unsigned int mclk_ps,
 					  const struct ddr2_spd_eeprom *spd,
 					  struct dimm_params *pdimm);
-unsigned int ddr3_compute_dimm_parameters(struct fsl_ddr_controller *c,
+unsigned int ddr3_compute_dimm_parameters(unsigned int mclk_ps,
 					  const struct ddr3_spd_eeprom *spd,
 					  struct dimm_params *pdimm);
-unsigned int ddr4_compute_dimm_parameters(struct fsl_ddr_controller *c,
+unsigned int ddr4_compute_dimm_parameters(unsigned int mclk_ps,
 					  const struct ddr4_spd_eeprom *spd,
 					  struct dimm_params *pdimm);
 void fsl_ddr_set_intl3r(const unsigned int granule_size);
diff --git a/drivers/ddr/fsl/main.c b/drivers/ddr/fsl/main.c
index aa2f2e1aa1..c70be6fcf9 100644
--- a/drivers/ddr/fsl/main.c
+++ b/drivers/ddr/fsl/main.c
@@ -238,19 +238,20 @@ static int compute_dimm_parameters(struct fsl_ddr_controller *c,
 				   struct spd_eeprom *spd,
 				   struct dimm_params *pdimm)
 {
+	unsigned int mclk_ps = get_memory_clk_period_ps(c);
 	const memctl_options_t *popts = &c->memctl_opts;
 	int ret = -EINVAL;
 
 	memset(pdimm, 0, sizeof(*pdimm));
 
 	if (is_ddr1(popts))
-		ret = ddr1_compute_dimm_parameters(c, (void *)spd, pdimm);
+		ret = ddr1_compute_dimm_parameters(mclk_ps, (void *)spd, pdimm);
 	else if (is_ddr2(popts))
-		ret = ddr2_compute_dimm_parameters(c, (void *)spd, pdimm);
+		ret = ddr2_compute_dimm_parameters(mclk_ps, (void *)spd, pdimm);
 	else if (is_ddr3(popts))
-		ret = ddr3_compute_dimm_parameters(c, (void *)spd, pdimm);
+		ret = ddr3_compute_dimm_parameters(mclk_ps, (void *)spd, pdimm);
 	else if (is_ddr4(popts))
-		ret = ddr4_compute_dimm_parameters(c, (void *)spd, pdimm);
+		ret = ddr4_compute_dimm_parameters(mclk_ps, (void *)spd, pdimm);
 
 	return ret;
 }
-- 
2.39.0

[PATCH 3/7] ddr_dimms: Move FSL dimm_params to include/ddr_dimms.h

[John Watts]

This is in preparation for use for generic SPD calculations.

Signed-off-by: John Watts <contact@jookia.org>
---
 include/ddr_dimms.h             | 101 ++++++++++++++++++++++++++++++++
 include/soc/fsl/fsl_ddr_sdram.h |  92 +----------------------------
 2 files changed, 102 insertions(+), 91 deletions(-)
 create mode 100644 include/ddr_dimms.h

diff --git a/include/ddr_dimms.h b/include/ddr_dimms.h
new file mode 100644
index 0000000000..3ec6209ae1
--- /dev/null
+++ b/include/ddr_dimms.h
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2008-2016 Freescale Semiconductor, Inc.
+ * Copyright 2017-2018 NXP Semiconductor
+ */
+
+#ifndef _DDR_DIMMS_H_
+#define _DDR_DIMMS_H_
+
+/* Parameters for a DDR dimm computed from the SPD */
+struct dimm_params {
+
+	/* DIMM organization parameters */
+	char mpart[19];		/* guaranteed null terminated */
+
+	unsigned int n_ranks;
+	unsigned int die_density;
+	unsigned long long rank_density;
+	unsigned long long capacity;
+	unsigned int data_width;
+	unsigned int primary_sdram_width;
+	unsigned int ec_sdram_width;
+	unsigned int registered_dimm;
+	unsigned int package_3ds;	/* number of dies in 3DS DIMM */
+	unsigned int device_width;	/* x4, x8, x16 components */
+
+	/* SDRAM device parameters */
+	unsigned int n_row_addr;
+	unsigned int n_col_addr;
+	unsigned int edc_config;	/* 0 = none, 1 = parity, 2 = ECC */
+	unsigned int bank_addr_bits; /* DDR4 */
+	unsigned int bank_group_bits; /* DDR4 */
+	unsigned int n_banks_per_sdram_device; /* !DDR4 */
+	unsigned int burst_lengths_bitmask;	/* BL=4 bit 2, BL=8 = bit 3 */
+
+	/* used in computing base address of DIMMs */
+	unsigned long long base_address;
+	/* mirrored DIMMs */
+	unsigned int mirrored_dimm;	/* only for ddr3 */
+
+	/* DIMM timing parameters */
+
+	int mtb_ps;	/* medium timebase ps */
+	int ftb_10th_ps; /* fine timebase, in 1/10 ps */
+	int taa_ps;	/* minimum CAS latency time */
+	int tfaw_ps;	/* four active window delay */
+
+	/*
+	 * SDRAM clock periods
+	 * The range for these are 1000-10000 so a short should be sufficient
+	 */
+	int tckmin_x_ps;
+	int tckmin_x_minus_1_ps;
+	int tckmin_x_minus_2_ps;
+	int tckmax_ps;
+
+	/* SPD-defined CAS latencies */
+	unsigned int caslat_x;
+	unsigned int caslat_x_minus_1;
+	unsigned int caslat_x_minus_2;
+
+	unsigned int caslat_lowest_derated;	/* Derated CAS latency */
+
+	/* basic timing parameters */
+	int trcd_ps;
+	int trp_ps;
+	int tras_ps;
+
+	int trfc1_ps; /* DDR4 */
+	int trfc2_ps; /* DDR4 */
+	int trfc4_ps; /* DDR4 */
+	int trrds_ps; /* DDR4 */
+	int trrdl_ps; /* DDR4 */
+	int tccdl_ps; /* DDR4 */
+	int trfc_slr_ps; /* DDR4 */
+	int twr_ps;	/* !DDR4, maximum = 63750 ps */
+	int trfc_ps;	/* max = 255 ns + 256 ns + .75 ns
+				       = 511750 ps */
+	int trrd_ps;	/* !DDR4, maximum = 63750 ps */
+	int twtr_ps;	/* !DDR4, maximum = 63750 ps */
+	int trtp_ps;	/* !DDR4, byte 38, spd->trtp */
+
+	int trc_ps;	/* maximum = 254 ns + .75 ns = 254750 ps */
+
+	int refresh_rate_ps;
+	int extended_op_srt;
+
+	int tis_ps;	/* DDR1, DDR2, byte 32, spd->ca_setup */
+	int tih_ps;	/* DDR1, DDR2, byte 33, spd->ca_hold */
+	int tds_ps;	/* DDR1, DDR2, byte 34, spd->data_setup */
+	int tdh_ps;	/* DDR1, DDR2, byte 35, spd->data_hold */
+	int tdqsq_max_ps;	/* DDR1, DDR2, byte 44, spd->tdqsq */
+	int tqhs_ps;	/* DDR1, DDR2, byte 45, spd->tqhs */
+
+	/* DDR3 & DDR4 RDIMM */
+	unsigned char rcw[16];	/* Register Control Word 0-15 */
+	unsigned int dq_mapping[18]; /* DDR4 */
+	unsigned int dq_mapping_ors; /* DDR4 */
+};
+
+#endif /* _DDR_DIMMS_H_ */
diff --git a/include/soc/fsl/fsl_ddr_sdram.h b/include/soc/fsl/fsl_ddr_sdram.h
index 80508ef5d5..23f0816599 100644
--- a/include/soc/fsl/fsl_ddr_sdram.h
+++ b/include/soc/fsl/fsl_ddr_sdram.h
@@ -8,6 +8,7 @@
 #define FSL_DDR_MEMCTL_H
 
 #include <ddr_spd.h>
+#include <ddr_dimms.h>
 #include <soc/fsl/fsl_immap.h>
 
 struct common_timing_params {
@@ -418,97 +419,6 @@ typedef struct memctl_options_s {
 #define EDC_ECC		2
 #define EDC_AC_PARITY	4
 
-/* Parameters for a DDR dimm computed from the SPD */
-struct dimm_params {
-
-	/* DIMM organization parameters */
-	char mpart[19];		/* guaranteed null terminated */
-
-	unsigned int n_ranks;
-	unsigned int die_density;
-	unsigned long long rank_density;
-	unsigned long long capacity;
-	unsigned int data_width;
-	unsigned int primary_sdram_width;
-	unsigned int ec_sdram_width;
-	unsigned int registered_dimm;
-	unsigned int package_3ds;	/* number of dies in 3DS DIMM */
-	unsigned int device_width;	/* x4, x8, x16 components */
-
-	/* SDRAM device parameters */
-	unsigned int n_row_addr;
-	unsigned int n_col_addr;
-	unsigned int edc_config;	/* 0 = none, 1 = parity, 2 = ECC */
-	unsigned int bank_addr_bits; /* DDR4 */
-	unsigned int bank_group_bits; /* DDR4 */
-	unsigned int n_banks_per_sdram_device; /* !DDR4 */
-	unsigned int burst_lengths_bitmask;	/* BL=4 bit 2, BL=8 = bit 3 */
-
-	/* used in computing base address of DIMMs */
-	unsigned long long base_address;
-	/* mirrored DIMMs */
-	unsigned int mirrored_dimm;	/* only for ddr3 */
-
-	/* DIMM timing parameters */
-
-	int mtb_ps;	/* medium timebase ps */
-	int ftb_10th_ps; /* fine timebase, in 1/10 ps */
-	int taa_ps;	/* minimum CAS latency time */
-	int tfaw_ps;	/* four active window delay */
-
-	/*
-	 * SDRAM clock periods
-	 * The range for these are 1000-10000 so a short should be sufficient
-	 */
-	int tckmin_x_ps;
-	int tckmin_x_minus_1_ps;
-	int tckmin_x_minus_2_ps;
-	int tckmax_ps;
-
-	/* SPD-defined CAS latencies */
-	unsigned int caslat_x;
-	unsigned int caslat_x_minus_1;
-	unsigned int caslat_x_minus_2;
-
-	unsigned int caslat_lowest_derated;	/* Derated CAS latency */
-
-	/* basic timing parameters */
-	int trcd_ps;
-	int trp_ps;
-	int tras_ps;
-
-	int trfc1_ps; /* DDR4 */
-	int trfc2_ps; /* DDR4 */
-	int trfc4_ps; /* DDR4 */
-	int trrds_ps; /* DDR4 */
-	int trrdl_ps; /* DDR4 */
-	int tccdl_ps; /* DDR4 */
-	int trfc_slr_ps; /* DDR4 */
-	int twr_ps;	/* !DDR4, maximum = 63750 ps */
-	int trfc_ps;	/* max = 255 ns + 256 ns + .75 ns
-				       = 511750 ps */
-	int trrd_ps;	/* !DDR4, maximum = 63750 ps */
-	int twtr_ps;	/* !DDR4, maximum = 63750 ps */
-	int trtp_ps;	/* !DDR4, byte 38, spd->trtp */
-
-	int trc_ps;	/* maximum = 254 ns + .75 ns = 254750 ps */
-
-	int refresh_rate_ps;
-	int extended_op_srt;
-
-	int tis_ps;	/* DDR1, DDR2, byte 32, spd->ca_setup */
-	int tih_ps;	/* DDR1, DDR2, byte 33, spd->ca_hold */
-	int tds_ps;	/* DDR1, DDR2, byte 34, spd->data_setup */
-	int tdh_ps;	/* DDR1, DDR2, byte 35, spd->data_hold */
-	int tdqsq_max_ps;	/* DDR1, DDR2, byte 44, spd->tdqsq */
-	int tqhs_ps;	/* DDR1, DDR2, byte 45, spd->tqhs */
-
-	/* DDR3 & DDR4 RDIMM */
-	unsigned char rcw[16];	/* Register Control Word 0-15 */
-	unsigned int dq_mapping[18]; /* DDR4 */
-	unsigned int dq_mapping_ors; /* DDR4 */
-};
-
 struct fsl_ddr_controller {
 	int num;
 	unsigned long ddr_freq;
-- 
2.39.0

[PATCH 4/7] ddr: fsl: Remove includes to fsl-specific code

[John Watts]

This is in preparation for use for generic SPD calculations.

Signed-off-by: John Watts <contact@jookia.org>
---
 drivers/ddr/fsl/ddr1_dimm_params.c | 3 +--
 drivers/ddr/fsl/ddr2_dimm_params.c | 3 +--
 drivers/ddr/fsl/ddr3_dimm_params.c | 3 +--
 drivers/ddr/fsl/ddr4_dimm_params.c | 3 +--
 include/ddr_dimms.h                | 2 ++
 5 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/ddr/fsl/ddr1_dimm_params.c b/drivers/ddr/fsl/ddr1_dimm_params.c
index 9a7f26bd49..3f8759c351 100644
--- a/drivers/ddr/fsl/ddr1_dimm_params.c
+++ b/drivers/ddr/fsl/ddr1_dimm_params.c
@@ -3,9 +3,8 @@
  * Copyright 2008 Freescale Semiconductor, Inc.
  */
 #include <common.h>
-#include <soc/fsl/fsl_ddr_sdram.h>
 #include <linux/log2.h>
-#include "fsl_ddr.h"
+#include <ddr_dimms.h>
 
 /*
  * Calculate the Density of each Physical Rank.
diff --git a/drivers/ddr/fsl/ddr2_dimm_params.c b/drivers/ddr/fsl/ddr2_dimm_params.c
index 6f17d55c1f..eb27dbee50 100644
--- a/drivers/ddr/fsl/ddr2_dimm_params.c
+++ b/drivers/ddr/fsl/ddr2_dimm_params.c
@@ -4,9 +4,8 @@
  */
 
 #include <common.h>
-#include <soc/fsl/fsl_ddr_sdram.h>
 #include <linux/log2.h>
-#include "fsl_ddr.h"
+#include <ddr_dimms.h>
 
 /*
  * Calculate the Density of each Physical Rank.
diff --git a/drivers/ddr/fsl/ddr3_dimm_params.c b/drivers/ddr/fsl/ddr3_dimm_params.c
index cfea35ca05..0526f0b435 100644
--- a/drivers/ddr/fsl/ddr3_dimm_params.c
+++ b/drivers/ddr/fsl/ddr3_dimm_params.c
@@ -9,8 +9,7 @@
  */
 
 #include <common.h>
-#include <soc/fsl/fsl_ddr_sdram.h>
-#include "fsl_ddr.h"
+#include <ddr_dimms.h>
 
 /*
  * Calculate the Density of each Physical Rank.
diff --git a/drivers/ddr/fsl/ddr4_dimm_params.c b/drivers/ddr/fsl/ddr4_dimm_params.c
index 41c63f3613..d711863bbb 100644
--- a/drivers/ddr/fsl/ddr4_dimm_params.c
+++ b/drivers/ddr/fsl/ddr4_dimm_params.c
@@ -11,8 +11,7 @@
  */
 
 #include <common.h>
-#include <soc/fsl/fsl_ddr_sdram.h>
-#include "fsl_ddr.h"
+#include <ddr_dimms.h>
 
 /*
  * Calculate the Density of each Physical Rank.
diff --git a/include/ddr_dimms.h b/include/ddr_dimms.h
index 3ec6209ae1..ea5ec85b82 100644
--- a/include/ddr_dimms.h
+++ b/include/ddr_dimms.h
@@ -7,6 +7,8 @@
 #ifndef _DDR_DIMMS_H_
 #define _DDR_DIMMS_H_
 
+#include <ddr_spd.h>
+
 /* Parameters for a DDR dimm computed from the SPD */
 struct dimm_params {
 
-- 
2.39.0

[PATCH 5/7] ddr_dimms: Move ddr*_dimm_params to common

[John Watts]

This code is no longer specific to the any board.

Signed-off-by: John Watts <contact@jookia.org>
---
 common/Makefile                                |  4 ++++
 {drivers/ddr/fsl => common}/ddr1_dimm_params.c |  0
 {drivers/ddr/fsl => common}/ddr2_dimm_params.c |  0
 {drivers/ddr/fsl => common}/ddr3_dimm_params.c |  0
 {drivers/ddr/fsl => common}/ddr4_dimm_params.c |  0
 drivers/ddr/fsl/Makefile                       |  4 ----
 drivers/ddr/fsl/fsl_ddr.h                      | 12 ------------
 include/ddr_dimms.h                            | 13 +++++++++++++
 8 files changed, 17 insertions(+), 16 deletions(-)
 rename {drivers/ddr/fsl => common}/ddr1_dimm_params.c (100%)
 rename {drivers/ddr/fsl => common}/ddr2_dimm_params.c (100%)
 rename {drivers/ddr/fsl => common}/ddr3_dimm_params.c (100%)
 rename {drivers/ddr/fsl => common}/ddr4_dimm_params.c (100%)

diff --git a/common/Makefile b/common/Makefile
index 25f5653f90..7131f15cc8 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -33,6 +33,10 @@ obj-$(CONFIG_CONSOLE_FULL)	+= console.o
 obj-$(CONFIG_CONSOLE_SIMPLE)	+= console_simple.o
 obj-y				+= console_countdown.o
 obj-pbl-$(CONFIG_DDR_SPD)	+= ddr_spd.o
+obj-pbl-$(CONFIG_DDR_SPD)	+= ddr1_dimm_params.o
+obj-pbl-$(CONFIG_DDR_SPD)	+= ddr2_dimm_params.o
+obj-pbl-$(CONFIG_DDR_SPD)	+= ddr3_dimm_params.o
+obj-pbl-$(CONFIG_DDR_SPD)	+= ddr4_dimm_params.o
 obj-$(CONFIG_ENV_HANDLING)	+= environment.o envfs-core.o
 obj-$(CONFIG_DEFAULT_ENVIRONMENT) += envfs-core.o
 obj-$(CONFIG_ENVIRONMENT_VARIABLES) += env.o
diff --git a/drivers/ddr/fsl/ddr1_dimm_params.c b/common/ddr1_dimm_params.c
similarity index 100%
rename from drivers/ddr/fsl/ddr1_dimm_params.c
rename to common/ddr1_dimm_params.c
diff --git a/drivers/ddr/fsl/ddr2_dimm_params.c b/common/ddr2_dimm_params.c
similarity index 100%
rename from drivers/ddr/fsl/ddr2_dimm_params.c
rename to common/ddr2_dimm_params.c
diff --git a/drivers/ddr/fsl/ddr3_dimm_params.c b/common/ddr3_dimm_params.c
similarity index 100%
rename from drivers/ddr/fsl/ddr3_dimm_params.c
rename to common/ddr3_dimm_params.c
diff --git a/drivers/ddr/fsl/ddr4_dimm_params.c b/common/ddr4_dimm_params.c
similarity index 100%
rename from drivers/ddr/fsl/ddr4_dimm_params.c
rename to common/ddr4_dimm_params.c
diff --git a/drivers/ddr/fsl/Makefile b/drivers/ddr/fsl/Makefile
index 394ae55383..787b4453f3 100644
--- a/drivers/ddr/fsl/Makefile
+++ b/drivers/ddr/fsl/Makefile
@@ -4,9 +4,5 @@
 
 pbl-y += main.o util.o ctrl_regs.o options.o lc_common_dimm_params.o
 
-pbl-y += ddr1_dimm_params.o
-pbl-y += ddr2_dimm_params.o
-pbl-y += ddr3_dimm_params.o
-pbl-y += ddr4_dimm_params.o
 obj-y += arm_ddr_gen3.o
 pbl-y += fsl_ddr_gen4.o
diff --git a/drivers/ddr/fsl/fsl_ddr.h b/drivers/ddr/fsl/fsl_ddr.h
index 87edd10ac1..e95cea1657 100644
--- a/drivers/ddr/fsl/fsl_ddr.h
+++ b/drivers/ddr/fsl/fsl_ddr.h
@@ -204,18 +204,6 @@ struct fsl_ddr_controller;
 
 u32 fsl_ddr_get_version(struct fsl_ddr_controller *c);
 
-unsigned int ddr1_compute_dimm_parameters(unsigned int mclk_ps,
-					  const struct ddr1_spd_eeprom *spd,
-					  struct dimm_params *pdimm);
-unsigned int ddr2_compute_dimm_parameters(unsigned int mclk_ps,
-					  const struct ddr2_spd_eeprom *spd,
-					  struct dimm_params *pdimm);
-unsigned int ddr3_compute_dimm_parameters(unsigned int mclk_ps,
-					  const struct ddr3_spd_eeprom *spd,
-					  struct dimm_params *pdimm);
-unsigned int ddr4_compute_dimm_parameters(unsigned int mclk_ps,
-					  const struct ddr4_spd_eeprom *spd,
-					  struct dimm_params *pdimm);
 void fsl_ddr_set_intl3r(const unsigned int granule_size);
 
 unsigned int compute_fsl_memctl_config_regs(struct fsl_ddr_controller *c);
diff --git a/include/ddr_dimms.h b/include/ddr_dimms.h
index ea5ec85b82..f44c46447a 100644
--- a/include/ddr_dimms.h
+++ b/include/ddr_dimms.h
@@ -100,4 +100,17 @@ struct dimm_params {
 	unsigned int dq_mapping_ors; /* DDR4 */
 };
 
+unsigned int ddr1_compute_dimm_parameters(unsigned int mclk_ps,
+					  const struct ddr1_spd_eeprom *spd,
+					  struct dimm_params *pdimm);
+unsigned int ddr2_compute_dimm_parameters(unsigned int mclk_ps,
+					  const struct ddr2_spd_eeprom *spd,
+					  struct dimm_params *pdimm);
+unsigned int ddr3_compute_dimm_parameters(unsigned int mclk_ps,
+					  const struct ddr3_spd_eeprom *spd,
+					  struct dimm_params *pdimm);
+unsigned int ddr4_compute_dimm_parameters(unsigned int mclk_ps,
+					  const struct ddr4_spd_eeprom *spd,
+					  struct dimm_params *pdimm);
+
 #endif /* _DDR_DIMMS_H_ */
-- 
2.39.0

[PATCH 6/7] ddr_dimms: Remove mclk_ps for DDR3 and DDR4

[John Watts]

This parameter is unused, throw it away.

Signed-off-by: John Watts <contact@jookia.org>
---
 common/ddr3_dimm_params.c | 3 +--
 common/ddr4_dimm_params.c | 3 +--
 drivers/ddr/fsl/main.c    | 4 ++--
 include/ddr_dimms.h       | 6 ++----
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/common/ddr3_dimm_params.c b/common/ddr3_dimm_params.c
index 0526f0b435..6c3dbc6877 100644
--- a/common/ddr3_dimm_params.c
+++ b/common/ddr3_dimm_params.c
@@ -78,8 +78,7 @@ compute_ranksize(const struct ddr3_spd_eeprom *spd)
  * Writes the results to the struct dimm_params structure pointed by pdimm.
  *
  */
-unsigned int ddr3_compute_dimm_parameters(unsigned int mclk_ps,
-					 const struct ddr3_spd_eeprom *spd,
+unsigned int ddr3_compute_dimm_parameters(const struct ddr3_spd_eeprom *spd,
 					 struct dimm_params *pdimm)
 {
 	int ret;
diff --git a/common/ddr4_dimm_params.c b/common/ddr4_dimm_params.c
index d711863bbb..9fa3225d90 100644
--- a/common/ddr4_dimm_params.c
+++ b/common/ddr4_dimm_params.c
@@ -123,8 +123,7 @@ compute_ranksize(const struct ddr4_spd_eeprom *spd)
  * Writes the results to the struct dimm_params structure pointed by pdimm.
  *
  */
-unsigned int ddr4_compute_dimm_parameters(unsigned int mclk_ps,
-					  const struct ddr4_spd_eeprom *spd,
+unsigned int ddr4_compute_dimm_parameters(const struct ddr4_spd_eeprom *spd,
 					  struct dimm_params *pdimm)
 {
 	int ret;
diff --git a/drivers/ddr/fsl/main.c b/drivers/ddr/fsl/main.c
index c70be6fcf9..c05f6d52fb 100644
--- a/drivers/ddr/fsl/main.c
+++ b/drivers/ddr/fsl/main.c
@@ -249,9 +249,9 @@ static int compute_dimm_parameters(struct fsl_ddr_controller *c,
 	else if (is_ddr2(popts))
 		ret = ddr2_compute_dimm_parameters(mclk_ps, (void *)spd, pdimm);
 	else if (is_ddr3(popts))
-		ret = ddr3_compute_dimm_parameters(mclk_ps, (void *)spd, pdimm);
+		ret = ddr3_compute_dimm_parameters((void *)spd, pdimm);
 	else if (is_ddr4(popts))
-		ret = ddr4_compute_dimm_parameters(mclk_ps, (void *)spd, pdimm);
+		ret = ddr4_compute_dimm_parameters((void *)spd, pdimm);
 
 	return ret;
 }
diff --git a/include/ddr_dimms.h b/include/ddr_dimms.h
index f44c46447a..db744ed5d8 100644
--- a/include/ddr_dimms.h
+++ b/include/ddr_dimms.h
@@ -106,11 +106,9 @@ unsigned int ddr1_compute_dimm_parameters(unsigned int mclk_ps,
 unsigned int ddr2_compute_dimm_parameters(unsigned int mclk_ps,
 					  const struct ddr2_spd_eeprom *spd,
 					  struct dimm_params *pdimm);
-unsigned int ddr3_compute_dimm_parameters(unsigned int mclk_ps,
-					  const struct ddr3_spd_eeprom *spd,
+unsigned int ddr3_compute_dimm_parameters(const struct ddr3_spd_eeprom *spd,
 					  struct dimm_params *pdimm);
-unsigned int ddr4_compute_dimm_parameters(unsigned int mclk_ps,
-					  const struct ddr4_spd_eeprom *spd,
+unsigned int ddr4_compute_dimm_parameters(const struct ddr4_spd_eeprom *spd,
 					  struct dimm_params *pdimm);
 
 #endif /* _DDR_DIMMS_H_ */
-- 
2.39.0

[PATCH 7/7] ddr_spd: Support reading SPD from DDR3 sticks

[John Watts]

DDR4 splits the read in two pages while other DDR types do not.
Introduce a new parameter to indicate how to read the SPD.

Signed-off-by: John Watts <contact@jookia.org>
---
 arch/arm/boards/ls1046ardb/lowlevel.c |  2 +-
 common/ddr_spd.c                      | 16 +++++++++++-----
 include/ddr_spd.h                     |  3 ++-
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boards/ls1046ardb/lowlevel.c b/arch/arm/boards/ls1046ardb/lowlevel.c
index 055e5f4c99..9eae032377 100644
--- a/arch/arm/boards/ls1046ardb/lowlevel.c
+++ b/arch/arm/boards/ls1046ardb/lowlevel.c
@@ -205,7 +205,7 @@ static noinline __noreturn void ls1046ardb_r_entry(unsigned long memsize)
 	IMD_USED_OF(fsl_ls1046a_rdb);
 
 	i2c = ls1046_i2c_init(IOMEM(LSCH2_I2C1_BASE_ADDR));
-	ret = spd_read_eeprom(i2c, 0x51, &spd_eeprom);
+	ret = spd_read_eeprom(i2c, 0x51, &spd_eeprom, SPD_MEMTYPE_DDR4);
 	if (ret) {
 		pr_err("Cannot read SPD EEPROM: %d\n", ret);
 		goto err;
diff --git a/common/ddr_spd.c b/common/ddr_spd.c
index dd3b8511e6..f7792360de 100644
--- a/common/ddr_spd.c
+++ b/common/ddr_spd.c
@@ -480,6 +480,7 @@ static int read_buf(struct pbl_i2c *i2c,
  * @i2c: I2C controller handle
  * @addr: I2C bus address for the EEPROM
  * @buf: buffer to read the SPD data to
+ * @memtype: Memory type, such as SPD_MEMTYPE_DDR4
  *
  * This function takes a I2C message transfer function and reads the contents
  * from a SPD EEPROM to the buffer provided at @buf. The buffer should at least
@@ -487,19 +488,24 @@ static int read_buf(struct pbl_i2c *i2c,
  * otherwise.
  */
 int spd_read_eeprom(struct pbl_i2c *i2c,
-		    uint8_t addr, void *buf)
+		    uint8_t addr, void *buf,
+		    int memtype)
 {
 	unsigned char *buf8 = buf;
 	int ret;
 
-	ret = read_buf(i2c, addr, SPD_SPA0_ADDRESS, buf);
-	if (ret < 0)
-		return ret;
+	if (memtype == SPD_MEMTYPE_DDR4) {
+		ret = read_buf(i2c, addr, SPD_SPA0_ADDRESS, buf);
+		if (ret < 0)
+			return ret;
 
-	if (buf8[2] == SPD_MEMTYPE_DDR4) {
 		ret = read_buf(i2c, addr, SPD_SPA1_ADDRESS, buf + 256);
 		if (ret < 0)
 			return ret;
+	} else {
+		ret = read_buf(i2c, addr, 0, buf);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
diff --git a/include/ddr_spd.h b/include/ddr_spd.h
index bcc2171d2a..a96d01df85 100644
--- a/include/ddr_spd.h
+++ b/include/ddr_spd.h
@@ -575,6 +575,7 @@ int ddr3_spd_check(const struct ddr3_spd_eeprom *spd);
 int ddr4_spd_check(const struct ddr4_spd_eeprom *spd);
 
 int spd_read_eeprom(struct pbl_i2c *i2c,
-		    uint8_t addr, void *buf);
+		    uint8_t addr, void *buf,
+		    int memtype);
 
 #endif /* _DDR_SPD_H_ */
-- 
2.39.0

Re: [PATCH 0/7] Support generating DIMM parameters on more systems

[Sascha Hauer]

On Sun, Jan 22, 2023 at 01:44:22AM +1100, John Watts wrote:
> Boards that with swappable RAM sticks need to read SPD data and generate
> useful parameters for the board's memory controller.
> 
> Currently the only board that does this is the ls1046ardb.
> This patch series modifies and re-organizes that code so it's usable by
> other boards.
> 
> I have used and tested this patch series on the i.MX6 and ensured the
> existing board still compiles.
> 
> There is one API change: spd_read_eeprom now takes a memtype parameter.
> This is required as the current code only supports reading DDR4 chips
> while the i.MX6 uses DDR3 chips.

Applied, thanks.

I added a sentence to the commit message of 1/7.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/7] Support generating DIMM parameters on more systems

[John Watts]

On Tue, Jan 24, 2023 at 09:26:26AM +0100, Sascha Hauer wrote:
> Applied, thanks.
> 
> I added a sentence to the commit message of 1/7.
> 
> Sascha

Thanks! :)