From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
Date: Thu, 23 May 2013 10:39:31 +0300
Subject: [U-Boot] [PATCH v03 1/2] OMAP3+: introduce generic ABB support
In-Reply-To: <20130522205609.GA5603@kahuna>
References: <1369125729-15571-1-git-send-email-andrii.tseglytskyi@ti.com>
	<1369125729-15571-2-git-send-email-andrii.tseglytskyi@ti.com>
	<20130522205609.GA5603@kahuna>
Message-ID: <519DC7B3.1040906@ti.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 05/22/2013 11:56 PM, Nishanth Menon wrote:
> Hi Andrii,
> We are almost there.. minor comments follow:
> On 11:42-20130521, Andrii Tseglytskyi wrote:
> [...]
>> diff --git a/arch/arm/cpu/armv7/omap5/abb.c b/arch/arm/cpu/armv7/omap5/abb.c
>> new file mode 100644
>> index 0000000..92470be
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv7/omap5/abb.c
>> @@ -0,0 +1,67 @@
> I might introduce this as part of patch #2... but no strong feelings
> about it.
> [...]
>> +s8 abb_setup_ldovbb(u32 fuse, u32 ldovbb)
>> +{
>> +	u32 vset;
>> +
>> +	/*
>> +	 * ABB parameters must be properly fused
>> +	 * otherwise ABB should be disabled
>> +	 */
>> +	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,  OMAP5_ABB_LDOVBBMPU_VSET_OUT_MASK, vset);
> OMAP5_ABB_LDOVBBMPU_MUX_CTRL_MASK wont get set :(

Sorry didn't get... I have

vset |= *OMAP5_ABB_LDOVBBMPU_MUX_CTRL_MASK*;  <-- set here
clrsetbits_le32(ldovbb,  OMAP5_ABB_LDOVBBMPU_VSET_OUT_MASK, vset);


Maybe it would be more clear to rework to:

clrsetbits_le32(ldovbb,  OMAP5_ABB_LDOVBBMPU_VSET_OUT_MASK, vset | *OMAP5_ABB_LDOVBBMPU_MUX_CTRL_MASK*);


Is this your suggestion?

Regards,
Andrii
> [...]
>
> Other than this, I have no further comments.