[U-Boot] [PATCH 2/2] iMX5: EfikaMX: Preliminary board support

[Marek Vasut]

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