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

Reply via email to