Hi Lukas, Sorry for the delayed response.
On Tuesday 19 December 2017 03:04 PM, Lukasz Majewski wrote: > 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 <lokeshvu...@ti.com> >>>> Signed-off-by: Krunal Bhargav <k-bhar...@ti.com> >>>> --- >>>> 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". Agreed. This should be doable. Tom, do you have any comments on this new sequence? On a side node, I will post a v2 for this series with the existing data. Cleanup can be done for the entire supported boards once the above discussion is concluded. Thanks and regards, Lokesh _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot