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

[Nishanth Menon <nm@ti.com>]

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