[U-Boot] [PATCH 09/13] board: ti: am574x-idk: Add ddr data support

Tom Rini trini at konsulko.com
Wed Jan 10 14:13:56 UTC 2018 

On Fri, Dec 29, 2017 at 11:47:27AM +0530, Lokesh Vutla wrote:
> 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 <lokeshvutla at ti.com>
> >>>> Signed-off-by: Krunal Bhargav <k-bhargav at 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?

Sorry for the delayed response.  I'm not sure this is a good idea, at
least not without seeing a patch doing it.  While a lot of this data is
fairly opaque, the structs that say field X is value Y is helpful at
times when debugging the odd problem.  And there's always the
programming sequence issues as well.  So, I guess I'd have to see a
patch where moving in this direction really does make life easier /
cleaner to be convinced it's a good idea.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180110/39ef3e69/attachment.sig>

More information about the U-Boot mailing list