[U-Boot] [PATCH v3 7/8] imx6: SPL support for iMX6 SabreSD

[John Tobias]

Hi Stefano,

That's fine.. I'll fix it.

Regards,

john

On Wed, Nov 12, 2014 at 2:13 AM, Stefano Babic <sbabic at denx.de> wrote:
> Hi John,
>
> On 11/11/2014 18:15, John Tobias wrote:
>
>>>>> You add a new entry point here (spl_board_mmc_init), but why cannot you
>>>>> do this in the functions already supplied by SPL ? Why not in board_init_f ?
>>>>
>>>> When the spl_mmc_load_image function being called, it call
>>>> mmc_initialize. By default, the mmc_initialize
>>>> call board_mmc_init. By looking the said function, it initialize all mmc ports.
>>>>
>>>> While, in spl_board_mmc_init, only initialize the current mmc port.
>>>
>>> ok - but which is the issue by initializing all ports ? I mean, if
>>> board_mmc_init initialize all ports including what you need in SPL,
>>> which is the reason to exclude the other ones ?
>>>
>>
>> When board_mmc_init initialize all ports, we can issue a command
>> at uboot console to switch to different port.
>> e.g:
>>
>> mmc dev 0 (e.g. SD4)
>> mmc dev 1  (e.g. SD2)
>> mmc dev 2 (e.g. SD3)
>>
>> We can also re-map which is index 0, 1 and 2 by re-arranging the contents of
>>
>> struct fsl_esdhc_cfg usdhc_cfg[3] = {
>>     {USDHC4_BASE_ADDR},
>>     {USDHC2_BASE_ADDR},
>>     {USDHC3_BASE_ADDR},
>> };
>>
>> Assuming the bootstrap is configured to port 3 (USDHC3). The SPL image will
>> call spl_mmc_load_image to load the uboot image. In order to do that,
>> it will call
>> the find_mmc_device(0), based on example above the index 0 is port 4, index 1
>> is port 2 and index 2 is port 3. The find_mmc_device will return with errors
>> because it couldn't initialize the said port.
>>
>> So by default, the user should switch back the bootstrap to port 4 in
>> order to load the
>> uboot correctly. Then, if the user really want to use the different
>> ports, they have
>> to change the arrangement of the contents of usdhc_cfg and compile the
>> u-boot again.
>>
>> In spl_board_mmc_init function, it will read the bootstrap and configure it. The
>> settings are passed to fsl_esdhc_initialize which is to be stored in
>> index 0. So,
>> find_mmc_device(0) could get a correct information.
>>
>> Then, switching to different bootstrap doesn't require to re-compile the image.
>
> This is not enough for adding a new exported weak function, that reamins
> undocumented, and does not avoid using #ifdef, only moved into
> drivers/mmc/mmc.c.
>
> We have other cases where the behavior is different between SPL and
> U-Boot, and this is handled in board file using the same #ifdef
> CONFIG_SPL_BUILD.
>
> The question is: why do you need to change the API (as I said, it
> remains undocumented) instead of doing :
>
> int board_mmc_init(bd_t *bis) {
>
> #ifdef CONFIG_SPL_BUILD
>         <initialization single port based on bootstrap>
> #else
>         <instantiate all ports as usual>
> #endif
>
> 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-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
> =====================================================================