[U-Boot] [PATCH v1 1/2] OMAP3+: introduce generic ABB support
Nishanth Menon
nm at ti.com
Mon May 13 16:58:01 CEST 2013
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.
> + */
> + 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.
> + 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 :)
> +#define OMAP_ABB_SETTLING_TIME 30
> +#define OMAP_ABB_CLOCK_CYCLES 8
<snip>
otherwise looks ok to me.
--
Regards,
Nishanth Menon
More information about the U-Boot
mailing list