[U-Boot-Users] [PATCH 04/10 v2] [ARM] TQMA31: add new board with i.MX31 processor

[Jens Gehrlein]

Hi Jean-Christophe,

Jean-Christophe PLAGNIOL-VILLARD schrieb:

>> +COBJS	:= tqma31.o
> why not use $(BOARD)?
> COBJS	:= $(BOARD).o

Okay.

>> +SOBJS	:= lowlevel_init.o
>> +
>> +SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
>> +OBJS	:= $(addprefix $(obj),$(COBJS))
>> +SOBJS	:= $(addprefix $(obj),$(SOBJS))
>> +
>> +#elif defined CONFIG_MX31_CPU_532_MHZ
>> +	/* Platform clocks */
>> +	REG	CCM_PDR0, PDR0_CSI_PODF(0x1ff)	/* camera sensor interface */		\
>> +			| PDR0_PER_PODF(8-1)	/* periph. if derived from USB clock */	\
>> +			| PDR0_HSP_PODF(4-1)	/* IPU 133 MHz */                      	\
> Please use tab instead of whitespace
>> +			| PDR0_NFC_PODF(7-1)	/* NAND FC 19 MHz */                   	\
> Please use tab instead of whitespace
>> +			| PDR0_IPG_PODF(2-1)	/* peripherals 66.5 MHz */             	\
> Please use tab instead of whitespace
>> +			| PDR0_MAX_PODF(4-1)	/* AHB 133 MHz */                      	\
> Please use tab instead of whitespace
>> +			| PDR0_MCU_PODF(1-1)	/* core 532 MHz */

Okay.

>> +	/* MCU PLL */
>> +	REG	CCM_MPCTL, PLL_PD(1-1) | PLL_MFD(52-1) | PLL_MFI(10) | PLL_MFN(12)
>> +#else
>> +#error Please define CPU core frequency (CONFIG_CPU_399_MHZ or CONFIG_CPU_532_MHZ)!
>> +#endif /* CONFIG_CPU_xxx_MHZ */
>> +
>> +	/* Serial PLL. Default: 1, 4, 12, 1 */
>> +	REG	CCM_SPCTL, PLL_PD(2-1) | PLL_MFD(5-1) | PLL_MFI(12) | PLL_MFN(1)
>> +.endm /* init_sdram */
>> +
>> +
>> +/* setup SDRAM on CSD0 */
>> +.macro init_sdram
>> +	/*
>> +	 * Enhanced SDRAM Miscellaneous Register
>> +	 *	Latency Hiding Enable
>> +	 *	LPDDR delay line measure unit is enabled
>> +	 *	LPDDR Delay Line is not reset
>> +	 *	Enable Mobile DDR SDRAM operation
>> +	 *	Soft Reset is disabled
>> +	 */
>> +	REG	ESDMISC, 0x00000004
>> +
>> +	/*
>> +	 * Enhanced SDRAM Configuration Register 0
>> +	 *	tXP 4 clock delay before new COMMAND issued to LPDDR
>> +	 *	tWTR tLPDDR WRITE to READ Command Delay = 2 clock
>> +	 *	tRP SDRAM Row Precharge Delay = 3 clock
>> +	 *	tMRD–SDRAM Load Mode Register to ACTIVE Command = 3 clock
> you have a nice char here '<96>' in vim

Uups. Well catched. Got this code from my colleague and missed that. 
Thank you.


>> +
>> +
>> +  	/*
> Please use tab instead of whitespace
>> +  	 * Enhanced SDRAM Control Register 0
> Please use tab instead of whitespace
>> +	 *	Enhanced SDRAM Controller Enable = 1
>> +	 *	SDRAM Controller Operating Mode = 010 Auto-Refresh Command
>> +	 *	Supervisor Protect = 0 User mode accesses are allowed to this
>> +	 *		chip select region
>> +	 *	ROW Row Address Width. = 011 14 Row Addresses
>> +	 *	COL Column Address Width = 01 9 Col Addresses
>> +	 *	DSIZ SDRAM Memory Data Width = 00 00 16-bit memory width
>> +	 *		aligned to D[31:16]
>> +	 *	SREFR SDRAM Refresh Rate  = 000 Refresh Disabled
>> +	 *		(bit field reset value)
>> +	 *	PWDT Power Down = 00 Disabled (bit field reset value) Run Mode
>> +	 *	FP Full Page = 0 0 Burst Length of the external memory device
>> +	 *		is not set to Full Page
>> +	 *	BL Burst Length = 0
>> +	 *	PRCT Precharge Timer 000000 Disabled (Bit field reset value)
>> +	 */
>> +	REG	ESDCTL0, 0xa2200000
>> +	REG	0x80000000, 0x12344321
>> +	REG	0x80000000, 0x12344321
>> +
>> +	/*
>> +	 * Enhanced SDRAM Control Register 0
>> +	 * 	Enhanced SDRAM Controller Enable = 1
> Please use tab instead of whitespace
>> +	 * 	SDRAM Controller Operating Mode = 001 Precharge Command
> Please use tab instead of whitespace
>> +	 * 	Supervisor Protect = 0 User mode accesses are allowed to this
> Please use tab instead of whitespace
>> +	 *		chip select region
>> +	 * 	ROW Row Address Width. = 011 14 Row Addresses
> Please use tab instead of whitespace
>> +	 * 	COL Column Address Width = 01 9 Col Addresses
> Please use tab instead of whitespace
>> +	 * 	DSIZ SDRAM Memory Data Width = 00 00 16-bit memory width
> Please use tab instead of whitespace
>> +	 *		aligned to D[31:16]
>> +	 * 	SREFR SDRAM Refresh Rate  = 000 Refresh Disabled
> Please use tab instead of whitespace
>> +	 *		(bit field reset value)
>> +	 * 	PWDT Power Down = 00 Disabled (bit field reset value) Run Mode
> Please use tab instead of whitespace
>> +	 * 	FP Full Page = 0 0 Burst Length of the external memory device
> Please use tab instead of whitespace
>> +	 *		is not set to Full Page.
>> +	 * 	BL Burst Length = 0
> Please use tab instead of whitespace
>> +	 * 	PRCT Precharge Timer 000000 Disabled (Bit field reset value)
> Please use tab instead of whitespace

Okay.

>> +	 */
>> +	REG	ESDCTL0, 0xb2200000
>> +	REG8	0x80000033, 0xda
>> +	REG8	0x81000000, 0xff
>> +
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +
>> +	return 0;
>> +}
>> +
>> +int board_init (void)
>> +{
>> +	/* WEIM burst clock runs all the time */
>> +	__REG (WCR) = WCR_BCM_1;
>> +
>> +	setup_chipselects ();
>> +	setup_iomux ();
>> +
>> +	/*
>> +	 * FIXME:
>> +	 * Replace machine type ID by symbolic define MACH_TYPE_TQMA31 as soon
>> +	 * as updated file mach-types.h is available in U-Boot tree.
>> +	 * Machine type ID 1810 for TQMA31 has been registered on 2008-06-25.
>> +	 */
>> +	/* Machine type ID for ARM linux */
>> +	gd->bd->bi_arch_number = 1810;
> No please update the mach-types.h

I thought this file gets updated from time to time by copying from the 
kernel. But after having a look into the git history of this file, it is 
not so. I will update directly.

>> +
>> +	/* Address of boot parameters */
>> +	gd->bd->bi_boot_params = 0x80000100;
>> +
>  +
>> +/*
>> + * Bank start addresses.
>> + * 4 x 32 MiB is the maximum for user and U-Boot code. Please adapt the list
>> + * as well as CFG_MAX_FLASH_BANKS to your TQM. Also consider dual die chips
>> + * or single die chips, e.g. one dual die chip represents two banks.
>> + */
>> +#define CFG_FLASH_BANKS_LIST		{CFG_FLASH_BASE, 		\
> Please use tab instead of whitespace

Okay.

>> +					CFG_FLASH_BASE + 32*1024*1024,	\
> 							please add space
> 							between operators

Okay.


Thank you very much for catching all these silly mistakes.

A last question:
When I want to "reply all", your e-mail address doesn't appear in the 
to-field. I have to add the address manually. Is this intended?

Kind regards,
Jens