[U-Boot] [PATCH V2] i.MX6: mx6q_sabrelite: add SATA bindings

[Stefano Babic]

On 14/03/2012 15:53, Dirk Behme wrote:
> On 13.03.2012 17:59, Eric Nelson wrote:
> ...
>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> ...
>> +    /* Enable sata clock */
>> +    reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
>> +    reg |= MXC_CCM_CCGR5_CG2_MASK;
>> +    writel(reg, &imx_ccm->CCGR5);
> 

Hi Dirk,

> We touch the CCGR5 already in the imximage.cfg. So we could drop this
> code completely and just add the MXC_CCM_CCGR5_CG2_MASK to the
> imximage.cfg.
> 
> What are the advantages/disadvantages of this?
> 
> Advantages:
> 
> Less code, touch the register only once in imximage.cfg.
> 
> Disadvantages:
> 
> Less readability, doing it in setup_sata() instead of imximage.cfg is
> easier to understand and disable (by removing CONFIG_CMD_SATA).
> 
> Opinions?
> 
> It sounds like this results in the basic question: Which registers
> should be touched in imximage.cfg, and which by explicit code in
> drivers/board files?

Of course the board maintainer can decide which is the best for its
custom board. My personal rule of thumb is to put into imximage.cfg only
the initialization of the DRAM controller, so that the SOC is able to
copy u-boot into RAM, while the rest is done in code.

Best regards,
Stefano


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