mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/1] ddr: imx8m: Rework ddrphy code and fix DDR-1067 on iMX8MM
@ 2021-09-24  3:14 Trent Piepho
  2021-09-29  9:48 ` Ahmad Fatoum
  2021-10-01 12:14 ` Sascha Hauer
  0 siblings, 2 replies; 3+ messages in thread
From: Trent Piepho @ 2021-09-24  3:14 UTC (permalink / raw)
  To: barebox; +Cc: Trent Piepho

This code could not decide if it was using speeds in Hz, or MHz, or just
constants that identify specific speeds.

Rework it to use symbolic constants for all speeds.  Use arrays
for configurations values for all cases.  The DDR DRAM speed constants
will be the array index, so the linear search for the configuration is
eliminated.

For iMX8MM type SoCs using the integr PLL the speed DDR-1067 with a 266⅔
MHz clock will now work.

The iM8MQ type SSCG PLL SoCs would previously silently program the PLL
will zero values if a non-supported DDR rate was used.  Now they will
generate an error.

Note that some PLL tables have entries for speeds that the main entry
point for the code does not support, so they can't actually ever be
used.  This is not fixed.

Signed-off-by: Trent Piepho <tpiepho@gmail.com>
---
 drivers/ddr/imx8m/ddrphy_utils.c | 332 ++++++++++++++-----------------
 1 file changed, 147 insertions(+), 185 deletions(-)

diff --git a/drivers/ddr/imx8m/ddrphy_utils.c b/drivers/ddr/imx8m/ddrphy_utils.c
index 9a4e1a22e..a56033f78 100644
--- a/drivers/ddr/imx8m/ddrphy_utils.c
+++ b/drivers/ddr/imx8m/ddrphy_utils.c
@@ -13,6 +13,109 @@
 #include <mach/imx8m-regs.h>
 #include <mach/imx8m-ccm-regs.h>
 
+/* DDR Transfer rate, bus clock is transfer rate / 2, and the DDRC runs at bus
+ * clock / 2, which is therefor transfer rate / 4.  */
+enum ddr_rate {
+	DDR_4000,
+	DDR_3200,
+	DDR_3000,
+	DDR_2600, /* Unused */
+	DDR_2400,
+	DDR_2376, /* Unused */
+	DDR_1600,
+	DDR_1000, /* Unused */
+	DDR_1066,
+	DDR_667,
+	DDR_400,
+	DDR_250, /* Unused */
+	DDR_100,
+	DDR_NUM_RATES
+};
+
+/* PLL config for IMX8MM type DRAM PLL.  This PLL type isn't documented, but
+ * it looks like it is a basically a fractional PLL:
+ * Frequency = Ref (24 MHz) / P * M / 2^S
+ * Note: Divider is equal to register value
+ */
+#define MDIV(x) ((x) << 12)
+#define PDIV(x) ((x) << 4)
+#define SDIV(x) ((x) << 0)
+
+#define LOCK_STATUS     BIT(31)
+#define LOCK_SEL_MASK   BIT(29)
+#define CLKE_MASK       BIT(11)
+#define RST_MASK        BIT(9)
+#define BYPASS_MASK     BIT(4)
+
+static const struct imx8mm_fracpll_config {
+	uint32_t r1, r2;
+	bool valid;
+} imx8mm_fracpll_table[DDR_NUM_RATES] = {
+	[DDR_4000] = { .valid = true, .r1 = MDIV(250) | PDIV(3) | SDIV(1), .r2 = 0 },
+	[DDR_3200] = { .valid = true, .r1 = MDIV(300) | PDIV(9) | SDIV(0), .r2 = 0 },
+	[DDR_3000] = { .valid = true, .r1 = MDIV(250) | PDIV(8) | SDIV(0), .r2 = 0 },
+	[DDR_2600] = { .valid = true, .r1 = MDIV(325) | PDIV(3) | SDIV(2), .r2 = 0 },
+	[DDR_2400] = { .valid = true, .r1 = MDIV(300) | PDIV(3) | SDIV(2), .r2 = 0 },
+	[DDR_2376] = { .valid = true, .r1 = MDIV( 99) | PDIV(1) | SDIV(2), .r2 = 0 },
+	[DDR_1600] = { .valid = true, .r1 = MDIV(300) | PDIV(9) | SDIV(1), .r2 = 0 },
+	[DDR_1066] = { .valid = true, .r1 = MDIV(400) | PDIV(9) | SDIV(2), .r2 = 0 },
+	[DDR_667]  = { .valid = true, .r1 = MDIV(334) | PDIV(3) | SDIV(4), .r2 = 0 },
+	[DDR_400]  = { .valid = true, .r1 = MDIV(300) | PDIV(9) | SDIV(3), .r2 = 0 },
+};
+
+/* PLL config for IMX8MQ type DRAM PLL.  This is SSCG_PLL:
+ * Frequency = Ref (25 MHz) / divr1 * (2*divf1) / divr2 * divf2 / divq
+ * Note: IMX8MQ RM, §5.1.5.4.4 Fig. 5-8 shows ÷2 on divf2, but this is not true.
+ * Note: divider is register value + 1
+ */
+#define SSCG_PLL_LOCK			BIT(31)
+#define SSCG_PLL_DRAM_PLL_CLKE		BIT(9)
+#define SSCG_PLL_PD			BIT(7)
+#define SSCG_PLL_BYPASS1		BIT(5)
+#define SSCG_PLL_BYPASS2		BIT(4)
+
+#define SSCG_PLL_REF_DIVR2_MASK		(0x3f << 19)
+#define SSCG_PLL_REF_DIVR2_VAL(n)	(((n) << 19) & SSCG_PLL_REF_DIVR2_MASK)
+#define SSCG_PLL_FEEDBACK_DIV_F1_MASK	(0x3f << 13)
+#define SSCG_PLL_FEEDBACK_DIV_F1_VAL(n)	(((n) << 13) & SSCG_PLL_FEEDBACK_DIV_F1_MASK)
+#define SSCG_PLL_FEEDBACK_DIV_F2_MASK	(0x3f << 7)
+#define SSCG_PLL_FEEDBACK_DIV_F2_VAL(n)	(((n) << 7) & SSCG_PLL_FEEDBACK_DIV_F2_MASK)
+#define SSCG_PLL_OUTPUT_DIV_VAL_MASK	(0x3f << 1)
+#define SSCG_PLL_OUTPUT_DIV_VAL(n)	(((n) << 1) & SSCG_PLL_OUTPUT_DIV_VAL_MASK)
+
+#define SSCG_PLL_CFG2(divf1, divr2, divf2, divq) \
+	(SSCG_PLL_FEEDBACK_DIV_F1_VAL(divf1) | SSCG_PLL_FEEDBACK_DIV_F2_VAL(divf2) | \
+	SSCG_PLL_REF_DIVR2_VAL(divr2) | SSCG_PLL_OUTPUT_DIV_VAL(divq))
+
+static const struct imx8mq_ssgcpll_config {
+	uint32_t val;
+	bool valid;
+} imx8mq_ssgcpll_table[DDR_NUM_RATES] = {
+	[DDR_3200] = { .valid = true, .val = SSCG_PLL_CFG2(39, 29, 11, 0) },
+	[DDR_2400] = { .valid = true, .val = SSCG_PLL_CFG2(39, 29, 17, 1) },
+	[DDR_1600] = { .valid = true, .val = SSCG_PLL_CFG2(39, 29, 11, 1) },
+	[DDR_667]  = { .valid = true, .val = SSCG_PLL_CFG2(45, 30, 8, 3) }, /* ~166.935 MHz = 667.74 */
+};
+
+/* IMX8M Bypass clock config.  These configure dram_alt1_clk and the dram apb
+ * clock.  For the bypass config, clock rate = DRAM tranfer rate, rather than
+ * clock = dram / 4
+ */
+
+/* prediv is actual divider, register will be set to divider - 1 */
+#define CCM_ROOT_CFG(mux, prediv) (IMX8M_CCM_TARGET_ROOTn_ENABLE | \
+	IMX8M_CCM_TARGET_ROOTn_MUX(mux) | IMX8M_CCM_TARGET_ROOTn_PRE_DIV(prediv-1))
+
+static const struct imx8m_bypass_config {
+	uint32_t alt_clk;
+	uint32_t apb_clk;
+	bool valid;
+} imx8m_bypass_table[DDR_NUM_RATES] = {
+	[DDR_400] = { .valid = true, .alt_clk = CCM_ROOT_CFG(1, 2), .apb_clk = CCM_ROOT_CFG(3, 2) },
+	[DDR_250] = { .valid = true, .alt_clk = CCM_ROOT_CFG(3, 2), .apb_clk = CCM_ROOT_CFG(2, 2) },
+	[DDR_100] = { .valid = true, .alt_clk = CCM_ROOT_CFG(2, 1), .apb_clk = CCM_ROOT_CFG(2, 2) },
+};
+
 void ddrc_phy_load_firmware(void __iomem *phy,
 			    enum ddrc_phy_firmware_offset offset,
 			    const u16 *blob, size_t size)
@@ -102,64 +205,18 @@ int wait_ddrphy_training_complete(void)
 	}
 }
 
-struct dram_bypass_clk_setting {
-	ulong clk;
-	int alt_root_sel;
-	int alt_pre_div;
-	int apb_root_sel;
-	int apb_pre_div;
-};
-
-#define MHZ(x)	(1000000UL * (x))
-
-static struct dram_bypass_clk_setting imx8mq_dram_bypass_tbl[] = {
-	{
-		.clk = MHZ(100),
-		.alt_root_sel = 2,
-		.alt_pre_div = 1 - 1,
-		.apb_root_sel = 2,
-		.apb_pre_div = 2 - 1,
-	} , {
-		.clk = MHZ(250),
-		.alt_root_sel = 3,
-		.alt_pre_div = 2 - 1,
-		.apb_root_sel = 2,
-		.apb_pre_div = 2 - 1,
-	}, {
-		.clk = MHZ(400),
-		.alt_root_sel = 1,
-		.alt_pre_div = 2 - 1,
-		.apb_root_sel = 3,
-		.apb_pre_div = 2 - 1,
-	},
-};
-
-static void dram_enable_bypass(ulong clk_val)
+static void dram_enable_bypass(enum ddr_rate drate)
 {
-	int i;
-	struct dram_bypass_clk_setting *config;
+	const struct imx8m_bypass_config *config = &imx8m_bypass_table[drate];
 
-	for (i = 0; i < ARRAY_SIZE(imx8mq_dram_bypass_tbl); i++) {
-		if (clk_val == imx8mq_dram_bypass_tbl[i].clk)
-			break;
-	}
-
-	if (i == ARRAY_SIZE(imx8mq_dram_bypass_tbl)) {
-		printf("No matched freq table %lu\n", clk_val);
+	if (!config->valid) {
+		printf("No matched freq table entry %u\n", drate);
 		return;
 	}
 
-	config = &imx8mq_dram_bypass_tbl[i];
-
-	imx8m_clock_set_target_val(IMX8M_DRAM_ALT_CLK_ROOT,
-				   IMX8M_CCM_TARGET_ROOTn_ENABLE |
-				   IMX8M_CCM_TARGET_ROOTn_MUX(config->alt_root_sel) |
-				   IMX8M_CCM_TARGET_ROOTn_PRE_DIV(config->alt_pre_div));
-        imx8m_clock_set_target_val(IMX8M_DRAM_APB_CLK_ROOT,
-				   IMX8M_CCM_TARGET_ROOTn_ENABLE |
-				   IMX8M_CCM_TARGET_ROOTn_MUX(config->apb_root_sel) |
-				   IMX8M_CCM_TARGET_ROOTn_PRE_DIV(config->apb_pre_div));
-        imx8m_clock_set_target_val(IMX8M_DRAM_SEL_CFG, IMX8M_CCM_TARGET_ROOTn_ENABLE |
+	imx8m_clock_set_target_val(IMX8M_DRAM_ALT_CLK_ROOT, config->alt_clk);
+	imx8m_clock_set_target_val(IMX8M_DRAM_APB_CLK_ROOT, config->apb_clk);
+	imx8m_clock_set_target_val(IMX8M_DRAM_SEL_CFG, IMX8M_CCM_TARGET_ROOTn_ENABLE |
 				   IMX8M_CCM_TARGET_ROOTn_MUX(1));
 }
 
@@ -174,56 +231,15 @@ static void dram_disable_bypass(void)
 				   IMX8M_CCM_TARGET_ROOTn_PRE_DIV(5 - 1));
 }
 
-struct imx_int_pll_rate_table {
-	u32 rate;
-	u32 r1;
-	u32 r2;
-};
-
-#define MDIV(x) ((x) << 12)
-#define PDIV(x) ((x) << 4)
-#define SDIV(x) ((x) << 0)
-
-#define LOCK_STATUS     BIT(31)
-#define LOCK_SEL_MASK   BIT(29)
-#define CLKE_MASK       BIT(11)
-#define RST_MASK        BIT(9)
-#define BYPASS_MASK     BIT(4)
-
-static struct imx_int_pll_rate_table imx8mm_fracpll_tbl[] = {
-	{ .rate = 1000000000U, .r1 = MDIV(250) | PDIV(3) | SDIV(1), .r2 = 0 },
-	{ .rate = 800000000U,  .r1 = MDIV(300) | PDIV(9) | SDIV(0), .r2 = 0 },
-	{ .rate = 750000000U,  .r1 = MDIV(250) | PDIV(8) | SDIV(0), .r2 = 0 },
-	{ .rate = 650000000U,  .r1 = MDIV(325) | PDIV(3) | SDIV(2), .r2 = 0 },
-	{ .rate = 600000000U,  .r1 = MDIV(300) | PDIV(3) | SDIV(2), .r2 = 0 },
-	{ .rate = 594000000U,  .r1 = MDIV( 99) | PDIV(1) | SDIV(2), .r2 = 0 },
-	{ .rate = 400000000U,  .r1 = MDIV(300) | PDIV(9) | SDIV(1), .r2 = 0 },
-	{ .rate = 266666667U,  .r1 = MDIV(400) | PDIV(9) | SDIV(2), .r2 = 0 },
-	{ .rate = 167000000U,  .r1 = MDIV(334) | PDIV(3) | SDIV(4), .r2 = 0 },
-	{ .rate = 100000000U,  .r1 = MDIV(300) | PDIV(9) | SDIV(3), .r2 = 0 },
-};
-
-static struct imx_int_pll_rate_table *fracpll(u32 freq)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(imx8mm_fracpll_tbl); i++)
-		if (freq == imx8mm_fracpll_tbl[i].rate)
-			return &imx8mm_fracpll_tbl[i];
-
-	return NULL;
-}
-
-static int dram_frac_pll_init(u32 freq)
+static int dram_frac_pll_init(enum ddr_rate drate)
 {
 	volatile int i;
 	u32 tmp;
 	void *pll_base;
-	struct imx_int_pll_rate_table *rate;
+	const struct imx8mm_fracpll_config *config = &imx8mm_fracpll_table[drate];
 
-	rate = fracpll(freq);
-	if (!rate) {
-		printf("No matched freq table %u\n", freq);
+	if (!config->valid) {
+		printf("No matched freq table entry %u\n", drate);
 		return -EINVAL;
 	}
 
@@ -242,8 +258,8 @@ static int dram_frac_pll_init(u32 freq)
 	tmp &= ~RST_MASK;
 	writel(tmp, pll_base);
 
-	writel(rate->r1, pll_base + 4);
-	writel(rate->r2, pll_base + 8);
+	writel(config->r1, pll_base + 4);
+	writel(config->r2, pll_base + 8);
 
 	for (i = 0; i < 1000; i++);
 
@@ -261,25 +277,16 @@ static int dram_frac_pll_init(u32 freq)
 	return 0;
 }
 
-#define SSCG_PLL_LOCK			BIT(31)
-#define SSCG_PLL_DRAM_PLL_CLKE		BIT(9)
-#define SSCG_PLL_PD			BIT(7)
-#define SSCG_PLL_BYPASS1		BIT(5)
-#define SSCG_PLL_BYPASS2		BIT(4)
-
-#define SSCG_PLL_REF_DIVR2_MASK		(0x3f << 19)
-#define SSCG_PLL_REF_DIVR2_VAL(n)	(((n) << 19) & SSCG_PLL_REF_DIVR2_MASK)
-#define SSCG_PLL_FEEDBACK_DIV_F1_MASK	(0x3f << 13)
-#define SSCG_PLL_FEEDBACK_DIV_F1_VAL(n)	(((n) << 13) & SSCG_PLL_FEEDBACK_DIV_F1_MASK)
-#define SSCG_PLL_FEEDBACK_DIV_F2_MASK	(0x3f << 7)
-#define SSCG_PLL_FEEDBACK_DIV_F2_VAL(n)	(((n) << 7) & SSCG_PLL_FEEDBACK_DIV_F2_MASK)
-#define SSCG_PLL_OUTPUT_DIV_VAL_MASK	(0x3f << 1)
-#define SSCG_PLL_OUTPUT_DIV_VAL(n)	(((n) << 1) & SSCG_PLL_OUTPUT_DIV_VAL_MASK)
-
-static int dram_sscg_pll_init(u32 freq)
+static int dram_sscg_pll_init(enum ddr_rate drate)
 {
 	u32 val;
 	void __iomem *pll_base = IOMEM(MX8M_ANATOP_BASE_ADDR) + 0x60;
+	const struct imx8mq_ssgcpll_config *config = &imx8mq_ssgcpll_table[drate];
+
+	if (!config->valid) {
+		printf("No matched freq table entry %u\n", drate);
+		return -EINVAL;
+	}
 
 	/* Bypass */
 	setbits_le32(pll_base, SSCG_PLL_BYPASS1 | SSCG_PLL_BYPASS2);
@@ -289,36 +296,7 @@ static int dram_sscg_pll_init(u32 freq)
 		 SSCG_PLL_FEEDBACK_DIV_F2_MASK |
 		 SSCG_PLL_FEEDBACK_DIV_F1_MASK |
 		 SSCG_PLL_REF_DIVR2_MASK);
-
-	switch (freq) {
-	case MHZ(800):
-		val |= SSCG_PLL_OUTPUT_DIV_VAL(0);
-		val |= SSCG_PLL_FEEDBACK_DIV_F2_VAL(11);
-		val |= SSCG_PLL_FEEDBACK_DIV_F1_VAL(39);
-		val |= SSCG_PLL_REF_DIVR2_VAL(29);
-		break;
-	case MHZ(600):
-		val |= SSCG_PLL_OUTPUT_DIV_VAL(1);
-		val |= SSCG_PLL_FEEDBACK_DIV_F2_VAL(17);
-		val |= SSCG_PLL_FEEDBACK_DIV_F1_VAL(39);
-		val |= SSCG_PLL_REF_DIVR2_VAL(29);
-		break;
-	case MHZ(400):
-		val |= SSCG_PLL_OUTPUT_DIV_VAL(1);
-		val |= SSCG_PLL_FEEDBACK_DIV_F2_VAL(11);
-		val |= SSCG_PLL_FEEDBACK_DIV_F1_VAL(39);
-		val |= SSCG_PLL_REF_DIVR2_VAL(29);
-		break;
-	case MHZ(167):
-		val |= SSCG_PLL_OUTPUT_DIV_VAL(3);
-		val |= SSCG_PLL_FEEDBACK_DIV_F2_VAL(8);
-		val |= SSCG_PLL_FEEDBACK_DIV_F1_VAL(45);
-		val |= SSCG_PLL_REF_DIVR2_VAL(30);
-		break;
-	default:
-		break;
-	}
-
+	val |= config->val;
 	writel(val, pll_base + 0x8);
 
 	/* Clear power down bit */
@@ -337,59 +315,43 @@ static int dram_sscg_pll_init(u32 freq)
 	return 0;
 }
 
-static int dram_pll_init(u32 freq, enum ddrc_type type)
+static int dram_pll_init(enum ddr_rate drate, enum ddrc_type type)
 {
 	switch (type) {
 	case DDRC_TYPE_MQ:
-		return dram_sscg_pll_init(freq);
+		return dram_sscg_pll_init(drate);
 	case DDRC_TYPE_MM:
 	case DDRC_TYPE_MP:
-		return dram_frac_pll_init(freq);
+		return dram_frac_pll_init(drate);
 	default:
 		return -ENODEV;
 	}
 }
 
-void ddrphy_init_set_dfi_clk(unsigned int drate, enum ddrc_type type)
+void ddrphy_init_set_dfi_clk(unsigned int drate_mhz, enum ddrc_type type)
 {
-	switch (drate) {
-	case 4000:
-		dram_pll_init(MHZ(1000), type);
-		dram_disable_bypass();
-		break;
-	case 3200:
-		dram_pll_init(MHZ(800), type);
-		dram_disable_bypass();
-		break;
-	case 3000:
-		dram_pll_init(MHZ(750), type);
-		dram_disable_bypass();
-		break;
-	case 2400:
-		dram_pll_init(MHZ(600), type);
-		dram_disable_bypass();
-		break;
-	case 1600:
-		dram_pll_init(MHZ(400), type);
-		dram_disable_bypass();
-		break;
-	case 1066:
-		dram_pll_init(MHZ(266),type);
-		dram_disable_bypass();
-		break;
-	case 667:
-		dram_pll_init(MHZ(167), type);
-		dram_disable_bypass();
-		break;
-	case 400:
-		dram_enable_bypass(MHZ(400));
-		break;
-	case 100:
-		dram_enable_bypass(MHZ(100));
-		break;
+	enum ddr_rate drate;
+
+	switch (drate_mhz) {
+	case 4000: drate = DDR_4000; break;
+	case 3200: drate = DDR_3200; break;
+	case 3000: drate = DDR_3000; break;
+	case 2400: drate = DDR_2400; break;
+	case 1600: drate = DDR_1600; break;
+	case 1066: drate = DDR_1066; break;
+	case 667: drate = DDR_667; break;
+	case 400: drate = DDR_400; break;
+	case 100: drate = DDR_100; break;
 	default:
 		return;
 	}
+
+	if (drate_mhz > 400) {
+		dram_pll_init(drate, type);
+		dram_disable_bypass();
+	} else {
+		dram_enable_bypass(drate);
+	}
 }
 
 void ddrphy_init_read_msg_block(enum fw_type type)
-- 
2.31.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] ddr: imx8m: Rework ddrphy code and fix DDR-1067 on iMX8MM
  2021-09-24  3:14 [PATCH 1/1] ddr: imx8m: Rework ddrphy code and fix DDR-1067 on iMX8MM Trent Piepho
@ 2021-09-29  9:48 ` Ahmad Fatoum
  2021-10-01 12:14 ` Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2021-09-29  9:48 UTC (permalink / raw)
  To: Trent Piepho, barebox

On 24.09.21 05:14, Trent Piepho wrote:
> This code could not decide if it was using speeds in Hz, or MHz, or just
> constants that identify specific speeds.
> 
> Rework it to use symbolic constants for all speeds.  Use arrays
> for configurations values for all cases.  The DDR DRAM speed constants
> will be the array index, so the linear search for the configuration is
> eliminated.
> 
> For iMX8MM type SoCs using the integr PLL the speed DDR-1067 with a 266⅔
> MHz clock will now work.
> 
> The iM8MQ type SSCG PLL SoCs would previously silently program the PLL
> will zero values if a non-supported DDR rate was used.  Now they will
> generate an error.
> 
> Note that some PLL tables have entries for speeds that the main entry
> point for the code does not support, so they can't actually ever be
> used.  This is not fixed.
> 
> Signed-off-by: Trent Piepho <tpiepho@gmail.com>
> ---

Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

>  drivers/ddr/imx8m/ddrphy_utils.c | 332 ++++++++++++++-----------------
>  1 file changed, 147 insertions(+), 185 deletions(-)
> 
> diff --git a/drivers/ddr/imx8m/ddrphy_utils.c b/drivers/ddr/imx8m/ddrphy_utils.c
> index 9a4e1a22e..a56033f78 100644
> --- a/drivers/ddr/imx8m/ddrphy_utils.c
> +++ b/drivers/ddr/imx8m/ddrphy_utils.c
> @@ -13,6 +13,109 @@
>  #include <mach/imx8m-regs.h>
>  #include <mach/imx8m-ccm-regs.h>
>  
> +/* DDR Transfer rate, bus clock is transfer rate / 2, and the DDRC runs at bus
> + * clock / 2, which is therefor transfer rate / 4.  */
> +enum ddr_rate {
> +	DDR_4000,
> +	DDR_3200,
> +	DDR_3000,
> +	DDR_2600, /* Unused */
> +	DDR_2400,
> +	DDR_2376, /* Unused */
> +	DDR_1600,
> +	DDR_1000, /* Unused */
> +	DDR_1066,
> +	DDR_667,
> +	DDR_400,
> +	DDR_250, /* Unused */
> +	DDR_100,
> +	DDR_NUM_RATES
> +};
> +
> +/* PLL config for IMX8MM type DRAM PLL.  This PLL type isn't documented, but
> + * it looks like it is a basically a fractional PLL:
> + * Frequency = Ref (24 MHz) / P * M / 2^S
> + * Note: Divider is equal to register value
> + */
> +#define MDIV(x) ((x) << 12)
> +#define PDIV(x) ((x) << 4)
> +#define SDIV(x) ((x) << 0)
> +
> +#define LOCK_STATUS     BIT(31)
> +#define LOCK_SEL_MASK   BIT(29)
> +#define CLKE_MASK       BIT(11)
> +#define RST_MASK        BIT(9)
> +#define BYPASS_MASK     BIT(4)
> +
> +static const struct imx8mm_fracpll_config {
> +	uint32_t r1, r2;
> +	bool valid;
> +} imx8mm_fracpll_table[DDR_NUM_RATES] = {
> +	[DDR_4000] = { .valid = true, .r1 = MDIV(250) | PDIV(3) | SDIV(1), .r2 = 0 },
> +	[DDR_3200] = { .valid = true, .r1 = MDIV(300) | PDIV(9) | SDIV(0), .r2 = 0 },
> +	[DDR_3000] = { .valid = true, .r1 = MDIV(250) | PDIV(8) | SDIV(0), .r2 = 0 },
> +	[DDR_2600] = { .valid = true, .r1 = MDIV(325) | PDIV(3) | SDIV(2), .r2 = 0 },
> +	[DDR_2400] = { .valid = true, .r1 = MDIV(300) | PDIV(3) | SDIV(2), .r2 = 0 },
> +	[DDR_2376] = { .valid = true, .r1 = MDIV( 99) | PDIV(1) | SDIV(2), .r2 = 0 },
> +	[DDR_1600] = { .valid = true, .r1 = MDIV(300) | PDIV(9) | SDIV(1), .r2 = 0 },
> +	[DDR_1066] = { .valid = true, .r1 = MDIV(400) | PDIV(9) | SDIV(2), .r2 = 0 },
> +	[DDR_667]  = { .valid = true, .r1 = MDIV(334) | PDIV(3) | SDIV(4), .r2 = 0 },
> +	[DDR_400]  = { .valid = true, .r1 = MDIV(300) | PDIV(9) | SDIV(3), .r2 = 0 },
> +};
> +
> +/* PLL config for IMX8MQ type DRAM PLL.  This is SSCG_PLL:
> + * Frequency = Ref (25 MHz) / divr1 * (2*divf1) / divr2 * divf2 / divq
> + * Note: IMX8MQ RM, §5.1.5.4.4 Fig. 5-8 shows ÷2 on divf2, but this is not true.
> + * Note: divider is register value + 1
> + */
> +#define SSCG_PLL_LOCK			BIT(31)
> +#define SSCG_PLL_DRAM_PLL_CLKE		BIT(9)
> +#define SSCG_PLL_PD			BIT(7)
> +#define SSCG_PLL_BYPASS1		BIT(5)
> +#define SSCG_PLL_BYPASS2		BIT(4)
> +
> +#define SSCG_PLL_REF_DIVR2_MASK		(0x3f << 19)
> +#define SSCG_PLL_REF_DIVR2_VAL(n)	(((n) << 19) & SSCG_PLL_REF_DIVR2_MASK)
> +#define SSCG_PLL_FEEDBACK_DIV_F1_MASK	(0x3f << 13)
> +#define SSCG_PLL_FEEDBACK_DIV_F1_VAL(n)	(((n) << 13) & SSCG_PLL_FEEDBACK_DIV_F1_MASK)
> +#define SSCG_PLL_FEEDBACK_DIV_F2_MASK	(0x3f << 7)
> +#define SSCG_PLL_FEEDBACK_DIV_F2_VAL(n)	(((n) << 7) & SSCG_PLL_FEEDBACK_DIV_F2_MASK)
> +#define SSCG_PLL_OUTPUT_DIV_VAL_MASK	(0x3f << 1)
> +#define SSCG_PLL_OUTPUT_DIV_VAL(n)	(((n) << 1) & SSCG_PLL_OUTPUT_DIV_VAL_MASK)
> +
> +#define SSCG_PLL_CFG2(divf1, divr2, divf2, divq) \
> +	(SSCG_PLL_FEEDBACK_DIV_F1_VAL(divf1) | SSCG_PLL_FEEDBACK_DIV_F2_VAL(divf2) | \
> +	SSCG_PLL_REF_DIVR2_VAL(divr2) | SSCG_PLL_OUTPUT_DIV_VAL(divq))
> +
> +static const struct imx8mq_ssgcpll_config {
> +	uint32_t val;
> +	bool valid;
> +} imx8mq_ssgcpll_table[DDR_NUM_RATES] = {
> +	[DDR_3200] = { .valid = true, .val = SSCG_PLL_CFG2(39, 29, 11, 0) },
> +	[DDR_2400] = { .valid = true, .val = SSCG_PLL_CFG2(39, 29, 17, 1) },
> +	[DDR_1600] = { .valid = true, .val = SSCG_PLL_CFG2(39, 29, 11, 1) },
> +	[DDR_667]  = { .valid = true, .val = SSCG_PLL_CFG2(45, 30, 8, 3) }, /* ~166.935 MHz = 667.74 */
> +};
> +
> +/* IMX8M Bypass clock config.  These configure dram_alt1_clk and the dram apb
> + * clock.  For the bypass config, clock rate = DRAM tranfer rate, rather than
> + * clock = dram / 4
> + */
> +
> +/* prediv is actual divider, register will be set to divider - 1 */
> +#define CCM_ROOT_CFG(mux, prediv) (IMX8M_CCM_TARGET_ROOTn_ENABLE | \
> +	IMX8M_CCM_TARGET_ROOTn_MUX(mux) | IMX8M_CCM_TARGET_ROOTn_PRE_DIV(prediv-1))
> +
> +static const struct imx8m_bypass_config {
> +	uint32_t alt_clk;
> +	uint32_t apb_clk;
> +	bool valid;
> +} imx8m_bypass_table[DDR_NUM_RATES] = {
> +	[DDR_400] = { .valid = true, .alt_clk = CCM_ROOT_CFG(1, 2), .apb_clk = CCM_ROOT_CFG(3, 2) },
> +	[DDR_250] = { .valid = true, .alt_clk = CCM_ROOT_CFG(3, 2), .apb_clk = CCM_ROOT_CFG(2, 2) },
> +	[DDR_100] = { .valid = true, .alt_clk = CCM_ROOT_CFG(2, 1), .apb_clk = CCM_ROOT_CFG(2, 2) },
> +};
> +
>  void ddrc_phy_load_firmware(void __iomem *phy,
>  			    enum ddrc_phy_firmware_offset offset,
>  			    const u16 *blob, size_t size)
> @@ -102,64 +205,18 @@ int wait_ddrphy_training_complete(void)
>  	}
>  }
>  
> -struct dram_bypass_clk_setting {
> -	ulong clk;
> -	int alt_root_sel;
> -	int alt_pre_div;
> -	int apb_root_sel;
> -	int apb_pre_div;
> -};
> -
> -#define MHZ(x)	(1000000UL * (x))
> -
> -static struct dram_bypass_clk_setting imx8mq_dram_bypass_tbl[] = {
> -	{
> -		.clk = MHZ(100),
> -		.alt_root_sel = 2,
> -		.alt_pre_div = 1 - 1,
> -		.apb_root_sel = 2,
> -		.apb_pre_div = 2 - 1,
> -	} , {
> -		.clk = MHZ(250),
> -		.alt_root_sel = 3,
> -		.alt_pre_div = 2 - 1,
> -		.apb_root_sel = 2,
> -		.apb_pre_div = 2 - 1,
> -	}, {
> -		.clk = MHZ(400),
> -		.alt_root_sel = 1,
> -		.alt_pre_div = 2 - 1,
> -		.apb_root_sel = 3,
> -		.apb_pre_div = 2 - 1,
> -	},
> -};
> -
> -static void dram_enable_bypass(ulong clk_val)
> +static void dram_enable_bypass(enum ddr_rate drate)
>  {
> -	int i;
> -	struct dram_bypass_clk_setting *config;
> +	const struct imx8m_bypass_config *config = &imx8m_bypass_table[drate];
>  
> -	for (i = 0; i < ARRAY_SIZE(imx8mq_dram_bypass_tbl); i++) {
> -		if (clk_val == imx8mq_dram_bypass_tbl[i].clk)
> -			break;
> -	}
> -
> -	if (i == ARRAY_SIZE(imx8mq_dram_bypass_tbl)) {
> -		printf("No matched freq table %lu\n", clk_val);
> +	if (!config->valid) {
> +		printf("No matched freq table entry %u\n", drate);
>  		return;
>  	}
>  
> -	config = &imx8mq_dram_bypass_tbl[i];
> -
> -	imx8m_clock_set_target_val(IMX8M_DRAM_ALT_CLK_ROOT,
> -				   IMX8M_CCM_TARGET_ROOTn_ENABLE |
> -				   IMX8M_CCM_TARGET_ROOTn_MUX(config->alt_root_sel) |
> -				   IMX8M_CCM_TARGET_ROOTn_PRE_DIV(config->alt_pre_div));
> -        imx8m_clock_set_target_val(IMX8M_DRAM_APB_CLK_ROOT,
> -				   IMX8M_CCM_TARGET_ROOTn_ENABLE |
> -				   IMX8M_CCM_TARGET_ROOTn_MUX(config->apb_root_sel) |
> -				   IMX8M_CCM_TARGET_ROOTn_PRE_DIV(config->apb_pre_div));
> -        imx8m_clock_set_target_val(IMX8M_DRAM_SEL_CFG, IMX8M_CCM_TARGET_ROOTn_ENABLE |
> +	imx8m_clock_set_target_val(IMX8M_DRAM_ALT_CLK_ROOT, config->alt_clk);
> +	imx8m_clock_set_target_val(IMX8M_DRAM_APB_CLK_ROOT, config->apb_clk);
> +	imx8m_clock_set_target_val(IMX8M_DRAM_SEL_CFG, IMX8M_CCM_TARGET_ROOTn_ENABLE |
>  				   IMX8M_CCM_TARGET_ROOTn_MUX(1));
>  }
>  
> @@ -174,56 +231,15 @@ static void dram_disable_bypass(void)
>  				   IMX8M_CCM_TARGET_ROOTn_PRE_DIV(5 - 1));
>  }
>  
> -struct imx_int_pll_rate_table {
> -	u32 rate;
> -	u32 r1;
> -	u32 r2;
> -};
> -
> -#define MDIV(x) ((x) << 12)
> -#define PDIV(x) ((x) << 4)
> -#define SDIV(x) ((x) << 0)
> -
> -#define LOCK_STATUS     BIT(31)
> -#define LOCK_SEL_MASK   BIT(29)
> -#define CLKE_MASK       BIT(11)
> -#define RST_MASK        BIT(9)
> -#define BYPASS_MASK     BIT(4)
> -
> -static struct imx_int_pll_rate_table imx8mm_fracpll_tbl[] = {
> -	{ .rate = 1000000000U, .r1 = MDIV(250) | PDIV(3) | SDIV(1), .r2 = 0 },
> -	{ .rate = 800000000U,  .r1 = MDIV(300) | PDIV(9) | SDIV(0), .r2 = 0 },
> -	{ .rate = 750000000U,  .r1 = MDIV(250) | PDIV(8) | SDIV(0), .r2 = 0 },
> -	{ .rate = 650000000U,  .r1 = MDIV(325) | PDIV(3) | SDIV(2), .r2 = 0 },
> -	{ .rate = 600000000U,  .r1 = MDIV(300) | PDIV(3) | SDIV(2), .r2 = 0 },
> -	{ .rate = 594000000U,  .r1 = MDIV( 99) | PDIV(1) | SDIV(2), .r2 = 0 },
> -	{ .rate = 400000000U,  .r1 = MDIV(300) | PDIV(9) | SDIV(1), .r2 = 0 },
> -	{ .rate = 266666667U,  .r1 = MDIV(400) | PDIV(9) | SDIV(2), .r2 = 0 },
> -	{ .rate = 167000000U,  .r1 = MDIV(334) | PDIV(3) | SDIV(4), .r2 = 0 },
> -	{ .rate = 100000000U,  .r1 = MDIV(300) | PDIV(9) | SDIV(3), .r2 = 0 },
> -};
> -
> -static struct imx_int_pll_rate_table *fracpll(u32 freq)
> -{
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(imx8mm_fracpll_tbl); i++)
> -		if (freq == imx8mm_fracpll_tbl[i].rate)
> -			return &imx8mm_fracpll_tbl[i];
> -
> -	return NULL;
> -}
> -
> -static int dram_frac_pll_init(u32 freq)
> +static int dram_frac_pll_init(enum ddr_rate drate)
>  {
>  	volatile int i;
>  	u32 tmp;
>  	void *pll_base;
> -	struct imx_int_pll_rate_table *rate;
> +	const struct imx8mm_fracpll_config *config = &imx8mm_fracpll_table[drate];
>  
> -	rate = fracpll(freq);
> -	if (!rate) {
> -		printf("No matched freq table %u\n", freq);
> +	if (!config->valid) {
> +		printf("No matched freq table entry %u\n", drate);
>  		return -EINVAL;
>  	}
>  
> @@ -242,8 +258,8 @@ static int dram_frac_pll_init(u32 freq)
>  	tmp &= ~RST_MASK;
>  	writel(tmp, pll_base);
>  
> -	writel(rate->r1, pll_base + 4);
> -	writel(rate->r2, pll_base + 8);
> +	writel(config->r1, pll_base + 4);
> +	writel(config->r2, pll_base + 8);
>  
>  	for (i = 0; i < 1000; i++);
>  
> @@ -261,25 +277,16 @@ static int dram_frac_pll_init(u32 freq)
>  	return 0;
>  }
>  
> -#define SSCG_PLL_LOCK			BIT(31)
> -#define SSCG_PLL_DRAM_PLL_CLKE		BIT(9)
> -#define SSCG_PLL_PD			BIT(7)
> -#define SSCG_PLL_BYPASS1		BIT(5)
> -#define SSCG_PLL_BYPASS2		BIT(4)
> -
> -#define SSCG_PLL_REF_DIVR2_MASK		(0x3f << 19)
> -#define SSCG_PLL_REF_DIVR2_VAL(n)	(((n) << 19) & SSCG_PLL_REF_DIVR2_MASK)
> -#define SSCG_PLL_FEEDBACK_DIV_F1_MASK	(0x3f << 13)
> -#define SSCG_PLL_FEEDBACK_DIV_F1_VAL(n)	(((n) << 13) & SSCG_PLL_FEEDBACK_DIV_F1_MASK)
> -#define SSCG_PLL_FEEDBACK_DIV_F2_MASK	(0x3f << 7)
> -#define SSCG_PLL_FEEDBACK_DIV_F2_VAL(n)	(((n) << 7) & SSCG_PLL_FEEDBACK_DIV_F2_MASK)
> -#define SSCG_PLL_OUTPUT_DIV_VAL_MASK	(0x3f << 1)
> -#define SSCG_PLL_OUTPUT_DIV_VAL(n)	(((n) << 1) & SSCG_PLL_OUTPUT_DIV_VAL_MASK)
> -
> -static int dram_sscg_pll_init(u32 freq)
> +static int dram_sscg_pll_init(enum ddr_rate drate)
>  {
>  	u32 val;
>  	void __iomem *pll_base = IOMEM(MX8M_ANATOP_BASE_ADDR) + 0x60;
> +	const struct imx8mq_ssgcpll_config *config = &imx8mq_ssgcpll_table[drate];
> +
> +	if (!config->valid) {
> +		printf("No matched freq table entry %u\n", drate);
> +		return -EINVAL;
> +	}
>  
>  	/* Bypass */
>  	setbits_le32(pll_base, SSCG_PLL_BYPASS1 | SSCG_PLL_BYPASS2);
> @@ -289,36 +296,7 @@ static int dram_sscg_pll_init(u32 freq)
>  		 SSCG_PLL_FEEDBACK_DIV_F2_MASK |
>  		 SSCG_PLL_FEEDBACK_DIV_F1_MASK |
>  		 SSCG_PLL_REF_DIVR2_MASK);
> -
> -	switch (freq) {
> -	case MHZ(800):
> -		val |= SSCG_PLL_OUTPUT_DIV_VAL(0);
> -		val |= SSCG_PLL_FEEDBACK_DIV_F2_VAL(11);
> -		val |= SSCG_PLL_FEEDBACK_DIV_F1_VAL(39);
> -		val |= SSCG_PLL_REF_DIVR2_VAL(29);
> -		break;
> -	case MHZ(600):
> -		val |= SSCG_PLL_OUTPUT_DIV_VAL(1);
> -		val |= SSCG_PLL_FEEDBACK_DIV_F2_VAL(17);
> -		val |= SSCG_PLL_FEEDBACK_DIV_F1_VAL(39);
> -		val |= SSCG_PLL_REF_DIVR2_VAL(29);
> -		break;
> -	case MHZ(400):
> -		val |= SSCG_PLL_OUTPUT_DIV_VAL(1);
> -		val |= SSCG_PLL_FEEDBACK_DIV_F2_VAL(11);
> -		val |= SSCG_PLL_FEEDBACK_DIV_F1_VAL(39);
> -		val |= SSCG_PLL_REF_DIVR2_VAL(29);
> -		break;
> -	case MHZ(167):
> -		val |= SSCG_PLL_OUTPUT_DIV_VAL(3);
> -		val |= SSCG_PLL_FEEDBACK_DIV_F2_VAL(8);
> -		val |= SSCG_PLL_FEEDBACK_DIV_F1_VAL(45);
> -		val |= SSCG_PLL_REF_DIVR2_VAL(30);
> -		break;
> -	default:
> -		break;
> -	}
> -
> +	val |= config->val;
>  	writel(val, pll_base + 0x8);
>  
>  	/* Clear power down bit */
> @@ -337,59 +315,43 @@ static int dram_sscg_pll_init(u32 freq)
>  	return 0;
>  }
>  
> -static int dram_pll_init(u32 freq, enum ddrc_type type)
> +static int dram_pll_init(enum ddr_rate drate, enum ddrc_type type)
>  {
>  	switch (type) {
>  	case DDRC_TYPE_MQ:
> -		return dram_sscg_pll_init(freq);
> +		return dram_sscg_pll_init(drate);
>  	case DDRC_TYPE_MM:
>  	case DDRC_TYPE_MP:
> -		return dram_frac_pll_init(freq);
> +		return dram_frac_pll_init(drate);
>  	default:
>  		return -ENODEV;
>  	}
>  }
>  
> -void ddrphy_init_set_dfi_clk(unsigned int drate, enum ddrc_type type)
> +void ddrphy_init_set_dfi_clk(unsigned int drate_mhz, enum ddrc_type type)
>  {
> -	switch (drate) {
> -	case 4000:
> -		dram_pll_init(MHZ(1000), type);
> -		dram_disable_bypass();
> -		break;
> -	case 3200:
> -		dram_pll_init(MHZ(800), type);
> -		dram_disable_bypass();
> -		break;
> -	case 3000:
> -		dram_pll_init(MHZ(750), type);
> -		dram_disable_bypass();
> -		break;
> -	case 2400:
> -		dram_pll_init(MHZ(600), type);
> -		dram_disable_bypass();
> -		break;
> -	case 1600:
> -		dram_pll_init(MHZ(400), type);
> -		dram_disable_bypass();
> -		break;
> -	case 1066:
> -		dram_pll_init(MHZ(266),type);
> -		dram_disable_bypass();
> -		break;
> -	case 667:
> -		dram_pll_init(MHZ(167), type);
> -		dram_disable_bypass();
> -		break;
> -	case 400:
> -		dram_enable_bypass(MHZ(400));
> -		break;
> -	case 100:
> -		dram_enable_bypass(MHZ(100));
> -		break;
> +	enum ddr_rate drate;
> +
> +	switch (drate_mhz) {
> +	case 4000: drate = DDR_4000; break;
> +	case 3200: drate = DDR_3200; break;
> +	case 3000: drate = DDR_3000; break;
> +	case 2400: drate = DDR_2400; break;
> +	case 1600: drate = DDR_1600; break;
> +	case 1066: drate = DDR_1066; break;
> +	case 667: drate = DDR_667; break;
> +	case 400: drate = DDR_400; break;
> +	case 100: drate = DDR_100; break;
>  	default:
>  		return;
>  	}
> +
> +	if (drate_mhz > 400) {
> +		dram_pll_init(drate, type);
> +		dram_disable_bypass();
> +	} else {
> +		dram_enable_bypass(drate);
> +	}
>  }
>  
>  void ddrphy_init_read_msg_block(enum fw_type type)
> 


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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] ddr: imx8m: Rework ddrphy code and fix DDR-1067 on iMX8MM
  2021-09-24  3:14 [PATCH 1/1] ddr: imx8m: Rework ddrphy code and fix DDR-1067 on iMX8MM Trent Piepho
  2021-09-29  9:48 ` Ahmad Fatoum
@ 2021-10-01 12:14 ` Sascha Hauer
  1 sibling, 0 replies; 3+ messages in thread
From: Sascha Hauer @ 2021-10-01 12:14 UTC (permalink / raw)
  To: Trent Piepho; +Cc: barebox

On Thu, Sep 23, 2021 at 08:14:46PM -0700, Trent Piepho wrote:
> This code could not decide if it was using speeds in Hz, or MHz, or just
> constants that identify specific speeds.
> 
> Rework it to use symbolic constants for all speeds.  Use arrays
> for configurations values for all cases.  The DDR DRAM speed constants
> will be the array index, so the linear search for the configuration is
> eliminated.
> 
> For iMX8MM type SoCs using the integr PLL the speed DDR-1067 with a 266⅔
> MHz clock will now work.
> 
> The iM8MQ type SSCG PLL SoCs would previously silently program the PLL
> will zero values if a non-supported DDR rate was used.  Now they will
> generate an error.
> 
> Note that some PLL tables have entries for speeds that the main entry
> point for the code does not support, so they can't actually ever be
> used.  This is not fixed.
> 
> Signed-off-by: Trent Piepho <tpiepho@gmail.com>
> ---
>  drivers/ddr/imx8m/ddrphy_utils.c | 332 ++++++++++++++-----------------
>  1 file changed, 147 insertions(+), 185 deletions(-)

Applied, thanks

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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-01 12:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24  3:14 [PATCH 1/1] ddr: imx8m: Rework ddrphy code and fix DDR-1067 on iMX8MM Trent Piepho
2021-09-29  9:48 ` Ahmad Fatoum
2021-10-01 12:14 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox