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

[Niel Fourie]

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>
>> +void sdram_init(void)
>> +{
>> +	int ram_type_index = PHYCORE_R2_MT41K128M16JT_256MB;
>> +
>> +	if (fdtdec_setup_mem_size_base())
>> +		gd->ram_size = SZ_256M;
>> +
>> +	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);
>> +}
> 
> This is wrong.  sdram_init() is called by
> arch/arm/mach-omap2/am33xx/board.c::dram_init() which then sets
> gd->ram_size based on what get_ram_size() determines.  So this is all
> just a wrapper around how the various parts of the am33xx generations
> call some form of config_ddr().  And what you have here is a lot of
> unused code about which module provides how much memory.  I assume
> there's some run-time method to determine which module you're on and
> thus determine that correct parameters to pass in for the chip that's in
> use.  If you're not there yet then just make sdram_init() call
> config_ddr(...) with the correct enum for the 256M chip and then update
> this when you have real detection.

Thanks for that input, you are right. I could not find any documented 
way to detect the exact module we are running on, but as you pointed out 
we can use get_ram_size() to find the size of the installed RAM. This is 
in fact exactly what barebox did, I just missed it. How is this for a 
replacement of the above?

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.

Best regards,
Niel Fourie

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-21 Fax: +49-8142-66989-80  Email: lusus at denx.de