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

[Stefano Babic]

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
=====================================================================