From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marek.vasut@gmail.com>
Date: Tue, 18 Jan 2011 20:32:33 +0100
Subject: [U-Boot] [PATCH 2/2] iMX5: EfikaMX: Preliminary board support
In-Reply-To: <4D330CFE.7070804@denx.de>
References: <1294939676-3603-1-git-send-email-marek.vasut@gmail.com>
	<1294939676-3603-2-git-send-email-marek.vasut@gmail.com>
	<4D330CFE.7070804@denx.de>
Message-ID: <201101182032.33444.marek.vasut@gmail.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 Sunday 16 January 2011 16:21:34 Stefano Babic wrote:
> On 01/13/2011 06:27 PM, Marek Vasut wrote:
> > As for the PMIC, other boards don't use any #defines for those constants
> > either, which obviously doens't mean it's right.
> 
> Well, this is not true. You use in yourd board a lot of these constants.
> There are only a few of them that are not used and
> 
> > +
> > +	/* NOTE: if (is_soc_rev(CHIP_REV_2_0) >= 0) */
> 
> You can drop this line
> 
> > +
> > +	/* Set core voltage to 1.1V */
> > +	val = pmic_reg_read(REG_SW_0);
> > +	val = (val & (~0x1F)) | 0x14;
> > +	pmic_reg_write(REG_SW_0, val);
> 
> Values are defined in Table 47 in MC13892.h manual. The low level is 0.6
> Volt, that corresponds to "0x0", and each bit increments the voltage by
> 25mV. I can suggest to add a macro to mc13892.h that return the value
> for register when the desired voltage (in millivolt) is passed.
> 
> > +
> > +	/* Setup VCC (SW2) to 1.25 */
> > +	val = pmic_reg_read(REG_SW_1);
> > +	val = (val & (~0x1F)) | 0x1A;
> > +	pmic_reg_write(REG_SW_1, val);
> 
> ..and use it here, too.
> 
> > +
> > +	/* Setup 1V2_DIG1 (SW3) to 1.25 */
> > +	val = pmic_reg_read(REG_SW_2);
> > +	val = (val & (~0x1F)) | 0x1A;
> > +	pmic_reg_write(REG_SW_2, val);
> > +	udelay(50);
> 
> ..and here.

Fixed
> 
> > +/***********************************************************************
> > *******
> 
> I have not checked before, but this is not the coding style for
> multiline comments.

Well it's more of a section separator, that's why this bold stuff. I'd prefer to 
leave it there for readability's sake.

Cheers