[U-Boot] [PATCH 5/5] MX5:MX53: add initial support for MX53EVK board

[Jason Liu]

Hi, Stefano,

2010/12/17 Stefano Babic <sbabic at denx.de>:
> On 12/17/2010 04:05 AM, Jason Liu wrote:
>> There is pretty much different with I.MX51 ROM,  we will not use DCD
>> data file to config the
>> DDR script since ROM has the DCD size limitation and use the advance
>> feature of what we
>> called plug-in,  the plug-in code must be in the first 2K of MMC card
>> from 0x400 offset, that's
>> why we need put this code section before start.S. The plug-in code
>> will be called by boot ROM
>> to do DDR init first and copy u-boot to DDR and jump to _start  to run it.
>
> As I am not understanding what you mean as "plugin", it seems to me you
> need some code able to set-up RAM and copy the u-boot code. This method
> looks like very similar to other SoCs, where a first stage boot loader
> is needed or when we boot from NAND.

Forget about the case about boot From NAND, we are talking about boot
from SD/MMC card here.
Why I call it here"plug-in", it due to it use the plugin feature of ROM.

This section of code is for ROM to load and run, thus it should meet
the ROM boot structure requirement.
The plugin feature of ROM can give more flexibility and it can
overcome some shortcomings of DCD(used on mx51).

By using this plugin we can get around the following issues:
1.	DCD size limitation issue, plugin can be the size of OCRAM free space region.
2.	Safe environment to re-configure PLL1 (without impacting SDRAM) as
the plugin runs from OCRAM.


>
> I do not find any reasonable argument to follow the methods you are
> proposing and to link statically this code to u-boot. In U-Boot we have
> already two mechanisms for do this initial setup, one using special
> images (with .kwb and .imx images when the RBL of the SoC is able to do
> the whole job), the second one with a first stage bootloader as nand_spl
> does and as Albert has already suggested.

I don't know .kwb format, but know .imx format. Here the .imx use the
DCD table to do DDR init
but with plugin, it need run the binary code not the DCD configure
table. So, We need use assembly
code to run the DDR init script.

>
>> Does anyone of expert like you and Albert and Wolfgang tell me how to
>> do it? It's very important for FSL
>> coming many SOCs support, since we will use plug-in feature of boot ROM.
>
> I am really not understanding if this "plugin" feature is really
> different as what we have with other SoCs or when we boot from NAND on
> other SOCs. As I can up now understand, it is similar to other
> approaches we have support already and I do not see a reason to
> implement a new one.

Here, We can talking about SD/MMC boot. It's some different with NAND boot.
It's different with MX51 DCD approach.

>
> Is there some released Freescale's documentation to allow to me to
> better understand what you are meaning ?
>
>> Yes, we need write it in assembly. Please see the comments above.
>> We can't put it into board_init or board_early_init_f, because it does
>> DDR init.
>> It *must* run in IRAM which load by ROM first.
>
> Running in IRAM is not a sufficient reason to do in this way. If I am
> not wrong, we could reach the same behavior on a i.MX51, too, if we set
> the app_code_jump in the header to point to an IRAM address. Then even
> the i.MX51 RBL should copy the code from storage to IRAM and then jump
> to this address, isnt it ?

ROM will only load 2KB data from the beginning of SD/MMC card. The
first 1KB is not used, it means
that only 1KB data is used. We need put ivt table and ddr init
assembly code into this
1KB section. So, we can't put the assembly code into board_init or
board_early_init_f.

>
>> I can't catch your mean. The function just do i2c iomux setup, anything wrong?
>
> My concern is related to the fact that the controllers are normally
> enumerated as "port numbers" and you make the selection based on the
> base address, that could be confusing. I see an approach of the type:
>
> static void setup_i2c(unsigned int port_number)
> {
>
>        switch (port_number) {
>                0:
>                        .....
>                        break;
>                1:
>
>        .........
> }
>
>
> more intuitive to understand.

Yes, agree.


>
>>>> +     /* Set core voltage VDDGP to 1.05V for 800MHZ */
>>>> +     buf[0] = 0x45;
>>>> +     buf[1] = 0x4a;
>>>> +     buf[2] = 0x52;
>>>
>>> Please remove all fixed constants in the file and replaced them with
>>> named constants, defined in a header file. Check vision2.c as reference.
>>> This board uses the MC13892 PMIC controller, and ./include/mc13892.h
>>> contains all required defines.
>>
>> OK,
>
> As further comment: there is already a pmic driver in u-boot
> (fsl_pmic.c), that hides the underlying communication between processor
> and pmic itself. At the moment, only spi is implemented. It is better to
> expand the fsl_pmic code to support i2c, too, and to use the accessors
> pmic_reg_read() and pmic_reg_write() to access to the pmic internal
> registers in the board related code.

OK, I will check that code and see how hard to expand fsl_pmic to support i2c,
which depends on the code scalability.

>
>> No, this is not the case like MX53. The power of FEC is default on of
>> this board.
>> We need change power before we can run CPU at full speed.
>
> Ok, understood.
>
>>>> +#define CONFIG_SYS_I2C_PORT             I2C2_BASE_ADDR
>>>
>>> As stated before: port means an enumeration value (0,1,..N), and it is
>>> set to a physical address.
>>
>> Yes, I will fix it.
>
> This comment is related to the previous one for the setup_i2c()
> function. Sorry if I was not clear enough. As you describe the define
> CONFIG_SYS_I2C_PORT, this should be a port number (0,1,..N) and the
> setup_i2() should accept as parameter this enumeration value, not a base
> address.

OK, I get it. Thanks,

>
> Best regards,
> Stefano Babic
>
>
> --
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> =====================================================================
>