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

[Liu Hui-R64343]

>-----Original Message-----
>From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de]
>On Behalf Of Stefano Babic
>Sent: Thursday, March 15, 2012 4:28 PM
>To: Dirk Behme
>Cc: u-boot at lists.denx.de; wg at denx.de
>Subject: Re: [U-Boot] [PATCH V2] i.MX6: mx6q_sabrelite: add SATA bindings
>
>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.

As the comments in the imximage.cfg,

# set the default clock gate to save power
DATA 4 0x020c4068 0x00C03F3F
DATA 4 0x020c406c 0x0030FC03
DATA 4 0x020c4070 0x0FFFC000
DATA 4 0x020c4074 0x3FF00000
DATA 4 0x020c4078 0x00FFF300
DATA 4 0x020c407c 0x0F0000C3
DATA 4 0x020c4080 0x000003FF

This is my intention for the default setting to gate clock as much as possible in
order to save power. This also means that when other periph added you need
Turn on the clock explicitly. So, my suggestion is:
- don't change this imximage.cfg,
- turn on the clock you need in the driver code,

Jason Liu
>
>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
>=================================================================
>====
>_______________________________________________
>U-Boot mailing list
>U-Boot at lists.denx.de
>http://lists.denx.de/mailman/listinfo/u-boot