[U-Boot] [PATCH v1 1/2] OMAP3+: introduce generic ABB support
Andrii Tseglytskyi
andrii.tseglytskyi at ti.com
Mon May 13 17:07:31 CEST 2013
On 05/13/2013 05:58 PM, Nishanth Menon wrote:
> Few minor comments follow:
> On 17:15-20130513, Andrii Tseglytskyi wrote:
> <snip>
>> diff --git a/arch/arm/cpu/armv7/omap-common/abb.c b/arch/arm/cpu/armv7/omap-common/abb.c
>> new file mode 100644
>> index 0000000..7ade110
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv7/omap-common/abb.c
>> @@ -0,0 +1,139 @@
> <snip>
>> + /* - On OMAP5+ silicons some ABB setting are fused
> /*
> *
> please
>> + * in corresponding OPP control registers. Also additional
>> + * setup for LDOVBB is required. Initialization
>> + * sequence contains specific part which handles this.
>> + * If function call fails - return quitely, it means
>> + * no ABB is required for silicon.
>> + *
>> + * - OMAP3 and OMAP4 don't have any fused settings for ABB.
>> + * EFUSE and LDOVBB registers are also not defined for them.
>> + * ABB will be initialized in the common way without
>> + * ldovbb setup.
> OMAP4 does have ABB efuse offset for OPP_TURBO. LDOVBB override is not
> used. please rephrase accordingly.
OK.
>> + */
>> + if (fuse && ldovbb) {
>> + if (abb_setup_ldovbb(fuse, ldovbb))
>> + return;
>> + }
> <snip>
>> +s8 abb_setup_ldovbb(u32 fuse, u32 ldovbb)
>> +{
>> + u32 vset;
>> +
>> + /* ABB parameters must be properly fused
>> + * otherwise ABB should be disabled */
> please fix comment style.
OK
>> + vset = readl(fuse);
>> + if (!(vset & OMAP5_ABB_FUSE_ENABLE_MASK))
>> + return -1;
>> +
>> + /* prepare VSET value for LDOVBB mux register */
>> + vset &= OMAP5_ABB_FUSE_VSET_MASK;
>> + vset >>= ffs(OMAP5_ABB_FUSE_VSET_MASK) - 1;
>> + vset <<= ffs(OMAP5_ABB_LDOVBBMPU_VSET_OUT_MASK) - 1;
>> + vset |= OMAP5_ABB_LDOVBBMPU_MUX_CTRL_MASK;
>> +
>> + /* setup LDOVBB using fused value */
>> + clrsetbits_le32(ldovbb, vset, vset);
>> +
>> + return 0;
>> +}
>> diff --git a/arch/arm/include/asm/arch-omap3/omap3.h b/arch/arm/include/asm/arch-omap3/omap3.h
>> index 2b5e9ae..66361d5 100644
>> --- a/arch/arm/include/asm/arch-omap3/omap3.h
>> +++ b/arch/arm/include/asm/arch-omap3/omap3.h
>> @@ -253,4 +253,13 @@ struct gpio {
>>
>> #define OMAP3_EMU_HAL_START_HAL_CRITICAL 4
>>
>> +/*
>> + * ABB settings
>> + */
> could be one line :)
Yeah it could. The reason why I did this - all comments in this file
have such style.
Anyway, agree with you.
>> +#define OMAP_ABB_SETTLING_TIME 30
>> +#define OMAP_ABB_CLOCK_CYCLES 8
> <snip>
> otherwise looks ok to me.
>
>
Thank you,
Regards,
Andrii
More information about the U-Boot
mailing list