[U-Boot] [PATCH v5] ARM: am335x: Add phyCORE AM335x R2 support

[Tom Rini]

On Tue, May 07, 2019 at 04:43:04PM +0200, Niel Fourie wrote:
> Hi Tom,
> 
> On 5/7/19 3:19 PM, Tom Rini wrote:
> >On Tue, May 07, 2019 at 11:39:12AM +0200, Niel Fourie wrote:
> >>Hi Tom,
> >>
> >>On 5/6/19 7:24 PM, Tom Rini wrote:
> >>>On Mon, May 06, 2019 at 06:44:48PM +0200, Niel Fourie wrote:
> >>>>Hi Tom,
> >>>>
> >>>>On 5/6/19 4:18 PM, Tom Rini wrote:
> >>>>>On Mon, May 06, 2019 at 04:02:53PM +0200, Niel Fourie wrote:
> >>>>>
> >>>>>>Support for Phytech phyCORE AM335x R2 SOM (PCL060) on the Phytec
> >>>>>>phyBOARD-Wega AM335x.
> >>>>>>
> >>>>>>CPU  : AM335X-GP rev 2.1
> >>>>>>Model: Phytec AM335x phyBOARD-WEGA
> >>>>>>DRAM:  256 MiB
> >>>>>>NAND:  256 MiB
> >>>>>>MMC:   OMAP SD/MMC: 0
> >>>>>>eth0: ethernet at 4a100000
> >>>>>>
> >>>>>>Working:
> >>>>>>  - Eth0
> >>>>>>  - i2C
> >>>>>>  - MMC/SD
> >>>>>>  - NAND
> >>>>>>  - UART
> >>>>>>  - USB (host)
> >>>>>>
> >>>>>>Device trees were taken from Linux mainline:
> >>>>>>commit 37624b58542f ("Linux 5.1-rc7")
> >>>>>>
> >>>>>>Signed-off-by: Niel Fourie <lusus at denx.de>
> >>[snip]
> >>>>
> >>>>void sdram_init(void)
> >>>>{
> >>>>	/* Configure memory to maximum supported size for detection */
> >>>>	int ram_type_index = PHYCORE_R2_MT41K512M16HA125IT_1024MB;
> >>>>	config_ddr(DDR_CLK_MHZ, &ioregs,
> >>>>		   &physom_timings[ram_type_index].ddr3_data,
> >>>>		   &ddr3_cmd_ctrl_data,
> >>>>		   &physom_timings[ram_type_index].ddr3_emif_reg_data,
> >>>>		   0);
> >>>>
> >>>>	/* Detect memory physically present */
> >>>>	gd->ram_size = get_ram_size((void *)CONFIG_SYS_SDRAM_BASE,
> >>>>				    CONFIG_MAX_RAM_BANK_SIZE);
> >>>>
> >>>>	/* Reconfigure memory for actual detected size */
> >>>>	switch (gd->ram_size) {
> >>>>	case SZ_1G:
> >>>>		ram_type_index = PHYCORE_R2_MT41K512M16HA125IT_1024MB;
> >>>>		break;
> >>>>	case SZ_512M:
> >>>>		ram_type_index = PHYCORE_R2_MT41K256M16TW107IT_512MB;
> >>>>		break;
> >>>>	case SZ_256M:
> >>>>	default:
> >>>>		ram_type_index = PHYCORE_R2_MT41K128M16JT_256MB;
> >>>>		break;
> >>>>	}
> >>>>	config_ddr(DDR_CLK_MHZ, &ioregs,
> >>>>		   &physom_timings[ram_type_index].ddr3_data,
> >>>>		   &ddr3_cmd_ctrl_data,
> >>>>		   &physom_timings[ram_type_index].ddr3_emif_reg_data,
> >>>>		   0);
> >>>>}
> >>>>
> >>>>The ugliest part of this is, as you pointed out, that directly after this is
> >>>>called, get_ram_size() will be called again from sdram_init(). But it at
> >>>>least noninvasive, and no longer requires the device tree.
> >>>
> >>>I don't think it's safe to call config_ddr twice, especially with the
> >>>possibly wrong parameters.  What's barebox doing in this case, being
> >>>told the presumably correct DDR size in the device tree?
> >>
> >>Good point. Barebox uses the above mechanism to detect the memory size, and
> >>I could find no equivalent memory size specified in its internal device
> >>tree.
> >
> >Configure for 1GB and then see how much we can actually talk to?
> 
> Yes. If you are interested, you can see their implementation here [1], where
> get_minimal_timings() returns the configuration for 1GiB; everything is in
> that file. (I did credit the author.)
> 
> [1] https://git.pengutronix.de/cgit/barebox/tree/arch/arm/boards/phytec-som-am335x/lowlevel.c#n167

Thinking back to my time at TI, that leaves me a little worried, but if
the vendor is OK with that approach, I'll leave it be, and yes, we can
mirror that approach here.  Thanks for the links!

-- 
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/20190507/1c400ee3/attachment.sig>