From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Tue, 19 Dec 2017 10:34:24 +0100 Subject: [U-Boot] [PATCH 09/13] board: ti: am574x-idk: Add ddr data support In-Reply-To: References: <20171218093425.12235-1-lokeshvutla@ti.com> <20171218093425.12235-10-lokeshvutla@ti.com> <20171218121609.0b3cbd85@jawa> Message-ID: <20171219103424.13c59026@jawa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Lokesh, > Hi Lukas, > > On Monday 18 December 2017 04:46 PM, Lukasz Majewski wrote: > > Hi Lokesh, > > > >> AM574x-idk has the following DDR parts attached: > >> EMIF1: MT41K256M16HA (1GB with ECC) > >> EMIF2: MT41K256M16HA (1GB without ECC) > >> > >> Enabling 2GB DDR without interleaving between EMIFs. And > >> enabling ECC on EMIF1. > >> > >> Signed-off-by: Lokesh Vutla > >> Signed-off-by: Krunal Bhargav > >> --- > >> board/ti/am57xx/board.c | 47 > >> ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 > >> insertions(+), 3 deletions(-) > >> > >> diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c > >> index 2d14ae54fe..1377c7b1fe 100644 > >> --- a/board/ti/am57xx/board.c > >> +++ b/board/ti/am57xx/board.c > >> @@ -89,10 +89,18 @@ static const struct dmm_lisa_map_regs > >> am571x_idk_lisa_regs = { .is_ma_present = 0x1 > >> }; > >> > >> +static const struct dmm_lisa_map_regs am574x_idk_lisa_regs = { > >> + .dmm_lisa_map_2 = 0xc0600200, > >> + .dmm_lisa_map_3 = 0x80600100, > >> + .is_ma_present = 0x1 > >> +}; > >> + > >> void emif_get_dmm_regs(const struct dmm_lisa_map_regs > >> **dmm_lisa_regs) { > >> if (board_is_am571x_idk()) > >> *dmm_lisa_regs = &am571x_idk_lisa_regs; > >> + else if (board_is_am574x_idk()) > >> + *dmm_lisa_regs = &am574x_idk_lisa_regs; > >> else > >> *dmm_lisa_regs = &beagle_x15_lisa_regs; > >> } > >> @@ -231,8 +239,8 @@ static const struct emif_regs > >> am571x_emif1_ddr3_666mhz_emif_regs = > >> { .ref_ctrl = > >> 0x0000514d, .ref_ctrl_final = > >> 0x0000144a, .sdram_tim1 = 0xd333887c, > >> - .sdram_tim2 = 0x40b37fe3, > >> - .sdram_tim3 = 0x409f8ada, > >> + .sdram_tim2 = 0x30b37fe3, > >> + .sdram_tim3 = 0x409f8ad8, > >> .read_idle_ctrl = 0x00050000, > >> .zq_config = 0x5007190b, > >> .temp_alert_config = 0x00000000, > >> @@ -249,17 +257,50 @@ static const struct emif_regs > >> am571x_emif1_ddr3_666mhz_emif_regs = > >> { .emif_rd_wr_exec_thresh = 0x00000305 }; > >> > >> +static const struct emif_regs > >> am574x_emif1_ddr3_666mhz_emif_ecc_regs = { > >> + .sdram_config_init = 0x61863332, > >> + .sdram_config = 0x61863332, > >> + .sdram_config2 = 0x08000000, > >> + .ref_ctrl = 0x0000514d, > >> + .ref_ctrl_final = 0x0000144a, > >> + .sdram_tim1 = 0xd333887c, > >> + .sdram_tim2 = 0x30b37fe3, > >> + .sdram_tim3 = 0x409f8ad8, > >> + .read_idle_ctrl = 0x00050000, > >> + .zq_config = 0x5007190b, > >> + .temp_alert_config = 0x00000000, > >> + .emif_ddr_phy_ctlr_1_init = 0x0024400f, > >> + .emif_ddr_phy_ctlr_1 = 0x0e24400f, > >> + .emif_ddr_ext_phy_ctrl_1 = 0x10040100, > >> + .emif_ddr_ext_phy_ctrl_2 = 0x00910091, > >> + .emif_ddr_ext_phy_ctrl_3 = 0x00950095, > >> + .emif_ddr_ext_phy_ctrl_4 = 0x009b009b, > >> + .emif_ddr_ext_phy_ctrl_5 = 0x009e009e, > >> + .emif_rd_wr_lvl_rmp_win = 0x00000000, > >> + .emif_rd_wr_lvl_rmp_ctl = 0x80000000, > >> + .emif_rd_wr_lvl_ctl = 0x00000000, > >> + .emif_rd_wr_exec_thresh = 0x00000305, > >> + .emif_ecc_ctrl_reg = 0xD0000001, > >> + .emif_ecc_address_range_1 = 0x3FFF0000, > >> + .emif_ecc_address_range_2 = 0x00000000 > >> +}; > > > > I'm wondering if it would be possible to: > > > > Embed this memory setup (even as binary blob) to SPL FIT -> Those > > values are generated from TI supplied excel sheet (when memory > > details are provided). > > > > > > Pros: > > ---- > > > > - Since the same EMIF controller is used, one could only adjust the > > binary blob, when new memory (faster, slower, from other > > manufacturer) is used in the product. > > > > - There would be no need to add such code to the board file. > > yeah, ddr is not the only thing that comes in this bucket, PMIC data > as well can be also made in similar way. I mean all the board related > information can be moved out. I think that the EMIF controller configuration is a bit special. As you pointed out - for the whole AM57xx family one EMIF controller type is used. > But then the binary size will still > remain the same. The goal here would be to not make the binary smaller, but reducing the maintanence effort. Example use case - company X has a product. They are using single u-boot (with SPL and dts). The only thing, which they need to change is the data needed for setting up proper memory configuration (DDR2/DDR3, speed - 1500, 1333, ECC enabled/disabled, module size, etc). This all is done in EMIF. With separate EMIF blob configuration they don't need to rebuild u-boot - they only change memory configuration binary data. This data has a separate room in the non-volatile memory (e.g. SPI-NOR flash). > Also, we will need a new driver to parse these new > binary formats. EMIF configuration data is IIRC 128 B at most. It is even possible to copy 1:1 the binary data to EMIF (as it is now done in the u-boot code). Hence, the "driver" would boil down to "memcpy". > > As of now, the ddr excel sheet outputs the data in the $patch format, > so still sticking to this format. As stated before, those Ctrl+C and Ctrl+V data are then copied (in for loop) to the EMIF registers. Why not wrap EMIF binary data to FIT, store on non-volatile memory and on each reset read it and "just" copy this data to EMIF? As fallback we can use the "from excel" structure. > > Yeah, i agree that it would be nice if we can come up with the > separate binary for all board related info(i guess DT can be re used > here). Not all board related info - just EMIF. DT is not an option here - it would require re-flashing u-boot binary. > > Thanks and regards, > Lokesh > > > > >> + > >> void emif_get_reg_dump(u32 emif_nr, const struct emif_regs **regs) > >> { > >> switch (emif_nr) { > >> case 1: > >> if (board_is_am571x_idk()) > >> *regs = > >> &am571x_emif1_ddr3_666mhz_emif_regs; > >> + else if (board_is_am574x_idk()) > >> + *regs = > >> &am574x_emif1_ddr3_666mhz_emif_ecc_regs; else > >> *regs = > >> &beagle_x15_emif1_ddr3_532mhz_emif_regs; break; > >> case 2: > >> - *regs = &beagle_x15_emif2_ddr3_532mhz_emif_regs; > >> + if (board_is_am574x_idk()) > >> + *regs = > >> &am571x_emif1_ddr3_666mhz_emif_regs; > >> + else > >> + *regs = > >> &beagle_x15_emif2_ddr3_532mhz_emif_regs; break; > >> } > >> } > > > > > > > > Best regards, > > > > Lukasz Majewski > > > > -- > > > > DENX Software Engineering GmbH, Managing Director: Wolfgang > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, > > Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: > > wd at denx.de Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 488 bytes Desc: OpenPGP digital signature URL: