[U-Boot] [PATCH v2 7/8] imx: mx6qpsabreauto: Add MX6QP SABREAUTO CPU3 board support

[Peng Fan]

Hi Stefano,

On Mon, Jun 29, 2015 at 09:26:10AM +0200, Stefano Babic wrote:
>Hi Peng,
>
>On 29/06/2015 04:05, Peng Fan wrote:
>
>>> To be sure I have not misunderstood the other patches: this is ok and
>>> this is what I am expecting. Code checks the SOC and provides a
>>> different action for QP or Quad or..
>>>
>>> But if some parts are defined at compile time, the check is losing its
>>> original meaning and the behavior is already defined at build time.
>> 
>> is_mx6dqp is runtime checking, it's behavior is not defined at build time.
>> There are some places which has the macro, #ifdef CONFIG_MX6QP, but all in
>> header files, not in xx.c files.
>
>Exactly, this is what I mean - here there is a runtime check, but this
>can be vanified by some #ifdef in header.
>
>>> The same here
>> 
>> Since this is board header files, different SOCs needs different dtbs.
>> The behavior is defined at build time.
>
>You're wrong. We accepted some code to detect at runtime which is the
>correct DTB to be selected. In fact, one goal is to have a single kernel
>as image, using the correct DTB to select the hardware.
>
>This work if we have the same U-Boot is running on different SOC
>variants. If U-Boot is hardcoded, why do we need to detect the right DTB
>? It could be also hard-coded.
>
>> I think it is not good to add
>> such piece of code in board file:
>> int board_late_init()
>> {
>> 	if (is_mx6dqp())
>> 		setenv(fdt_file, "imx6qp-sabreauto.dtb") /* Just an example */
>
>It must be checked if fdt_file is already set, because the customer can
>decide to use its own, but well, yes, this is done by other boards -
>check in code.
>
>> }
>> So I prefer to use "#if defined CONFIG_MX6QP" in board header file.
>
>There is an important goal: having a single U-Boot image running on
>boards that can have different (pin compatible) SOCs. We get this
>managing the different layout of the IOMUXC, that was maybe the most
>difficult part - why do we go back when we can't ?

Get you.
I saw the code in gw_ventana.c to detect the DTB at runtime.
To imx6q/qp/dl sabreauto board, there is no place to store info such as
ventana_board_info. But this is a good point that gw_ventana use the way
to do runtime check, which can make one image goal for pin compatible SOCs.

To the i.mx6qp board patch, I can use this way to remove the
DTB related #ifdefs, use the way that gw_ventana uses. Will split the current
patch into two, one to refacotr the current code to use runtime DTB detect,
the second one is to add i.mx6qp support.
>
>Best regards,
>Stefano Babic
>
>-- 
>=====================================================================
>DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>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
>=====================================================================

Regards,
Peng.
--