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

[Niel Fourie]

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

>> Marek originally proposed using the memory size specified in the device tree
>> as an improvement over specifying the size in the defconfig (as in v1 of the
>> patch).
> 
> But then you aren't populating 3 device trees nor making it clear / easy
> to say which module you're on, and then still need to change the config
> for which DT you're picking up.  These SOMs really don't provide any
> run-time method to see which one you're on?  There's no GPIOs to poke?

Agreed, the device tree solution is inferior to autodetection. The SOMs 
manual makes no mention of how different variants can be 
distinguished/detected, and the board specific code in Barebox (written 
by Phytec) does not contain any other detection code (except for the 
RAM), like checking GPIOs. Unfortunately there is no publicly available 
schematic, so I can't be completely sure. So I am going to assume it, 
there is no other way of detection.

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