[U-Boot] [PATCH 4/7] ARM: sun6i: Add clock support

[Hans de Goede]

Hi,

On 09/22/2014 02:47 PM, Chen-Yu Tsai wrote:
> On Mon, Sep 22, 2014 at 2:35 AM, Ian Campbell <ijc at hellion.org.uk> wrote:
>> On Mon, 2014-09-08 at 21:28 +0800, Chen-Yu Tsai wrote:
>>
>>> +#ifdef CONFIG_SPL_BUILD
>>
>> Since there is no SPL support this is dead code right now, correct?
> 
> This was part of Hans' attempt to support SPL. It was not finished.
> 
>> I'm wondering whether we should leave it out of mainline until the SPL
>> stuff is done, so SPL will be upstreamed all at once. What do others
>> think?
> 
> Sounds reasonable.
> 
>>> +     /* Set PLL ldo voltage without this PLL6 does not work properly */
>>
>> Is "this" here the doing it 3 times bit? If that's deliberate then
>> please say so explicitly. e.g. "Set PLL LDO voltage 3 times, without ...
>> etc"), if it's not deliberate then please fix ;-)
> 
> Hans, if you could enlighten us? :)

If you take a closer look at the code you will see not all 3 writes are the
same:

        /* Set PLL ldo voltage without this PLL6 does not work properly */
        writel(PRCM_PLL_CTRL_LDO_DIGITAL_EN | PRCM_PLL_CTRL_LDO_ANALOG_EN |
                PRCM_PLL_CTRL_EXT_OSC_EN | PRCM_PLL_CTRL_LDO_OUT_L(1140) |
                PRCM_PLL_CTRL_LDO_KEY, &prcm->pll_ctrl1);
        writel(PRCM_PLL_CTRL_LDO_DIGITAL_EN | PRCM_PLL_CTRL_LDO_ANALOG_EN |
                PRCM_PLL_CTRL_EXT_OSC_EN | PRCM_PLL_CTRL_LDO_OUT_L(1140) |
                PRCM_PLL_CTRL_LDO_KEY, &prcm->pll_ctrl1);
        writel(PRCM_PLL_CTRL_LDO_DIGITAL_EN | PRCM_PLL_CTRL_LDO_ANALOG_EN |
                PRCM_PLL_CTRL_EXT_OSC_EN | PRCM_PLL_CTRL_LDO_OUT_L(1140),
                &prcm->pll_ctrl1);

This register is locked with a so called "key", the first write is to set
the key (and has everything else the same in case the key is already
written). The second write actually makes the changes, and the third write
clears the key bits.

I guess this may need some better comments :)

Regards,

Hans