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

[Wadim Egorov]

On 07.05.19 16:43, 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
>
>>> 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.

You are right, for our AM335 based SOMs there is no way to detect the
variant. The barebox generates for every possible RAM configuration an
own MLO. The nice thing is that barebox generates all variants with one
build by using its multi image mechanism.

Besides the RAM, the phyCORE can be populated with different AM335x
variants, NANDs, RTC/EEPROM/Eth-PHY(yes/no) and maybe other things I
don't remember now.

I think it would be a good Idea to maintain a list of article numbers
(that fully describes the SOM variant) which are supported by the
current u-boot version.

Regards,
Wadim

>
> Best regards,
> Niel Fourie
>