[U-Boot] [PATCH v02 1/2] OMAP3+: introduce generic ABB support

[Nishanth Menon]

On Mon, May 20, 2013 at 6:06 AM, Andrii Tseglytskyi
<andrii.tseglytskyi at ti.com> wrote:
> On 05/17/2013 04:11 PM, Nishanth Menon wrote:
>
> [snip]
>>
>> On 19:49-20130513, Andrii Tseglytskyi wrote:
>> [...]
>>>
>>> +       if (fuse && ldovbb) {
>>> +               if (abb_setup_ldovbb(fuse, ldovbb))
>>> +                       return;
>>> +       }
>>> +
>>> +       /* configure timings, based on oscillator value */
>>> +       abb_setup_timings(setup);
>>
>> Still missing txdone clear..
>> If txdone was set on entry, you'd escape a bit waiting for transition
>> completion bit in SR2, However, by clearing TXDONE here, you can just wait
>> for TXDONE.
>
>
> We touch ABB first time here. I can add check/clear txdone here to double
> check that everything is fine,
> but this may be superfluous.

PRM register may be reset OR not depending on type of reset and SoC,
Though this might be the first line of code to execute in bootloader
that touches this register, there is no guarentee that there is no
pending TRANXDONE interrupt in the register. if there is TRANXDONE
interrupt, it is not something we desire. Hence the suggestion to
clear.

>>> +
>>> +       /* select ABB type */
>>> +       clrsetbits_le32(setup,
>>> +                       abb_type_mask | OMAP_ABB_SETUP_SR2EN_MASK,
>>> +                       abb_type_mask | OMAP_ABB_SETUP_SR2EN_MASK);
>>
>> This is no better than set_bits! clearbits (addr, clr, set) -> the clr
>> bits should clear *all* bits of the field. in this example ABB_TYPE
>> should both be cleared, same in OPP_SEL.
>> See the following change on top of this series:
>
> Yep. should be:
>
> clrsetbits_le32(setup,
>                     OMAP_ABB_SETUP_ACTIVE_FBB_SEL_MASK
> |OMAP_ABB_SETUP_ACTIVE_RBB_SEL_MASK | OMAP_ABB_SETUP_SR2EN_MASK,
>                    abb_type_mask | OMAP_ABB_SETUP_SR2EN_MASK)
>
> But I propose to rework this in the following way:
> - at the beginning of setup function clear both ABB registers (setup and
> control),
>     writel(0, setup);
>     writel(0, control);
>
> - use setbits_le32 everywhere.
>
> This guarantees that there will be no trash values in ABB registers and
> increases code readability.
> Your opinion?
>
Case 1) if we have been given control by another bootloader which had
already setup ABB, writing 0 will not impact the current setting as
the transition trigger is OPP_CHANGE
case 2) we are the only bootloader in the system. writing 0 is ok as well.

So, am ok if you reset the registers prior to programming them.
--
Regards,
Nishanth Menon