From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Denk <wd@denx.de>
Date: Thu, 20 Jan 2011 10:41:41 +0100
Subject: [U-Boot] [PATCH V2 11/11] Add support for Freescale's mx35pdk
	board.
In-Reply-To: <1295513194-16158-12-git-send-email-sbabic@denx.de>
References: <1295513194-16158-1-git-send-email-sbabic@denx.de>
	<1295513194-16158-12-git-send-email-sbabic@denx.de>
Message-ID: <20110120094141.DE5E5D301D7@gemini.denx.de>
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

Dear Stefano Babic,

In message <1295513194-16158-12-git-send-email-sbabic@denx.de> you wrote:
> The patch adds suupport for the Freescale's mx35pdk board
> (known as well as mx35_3stack).

Checkpatch says:

	[U-Boot] [PATCH V2 11/11] Add support for Freescale's mx35pdk board.
	total: 0 errors, 1 warnings, 1331 lines checked

(Prototype for smc911x_initialize() is in netdev.h).

...
> +.macro init_clock
> +	ldr r0, =CCM_BASE_ADDR
> +
> +        /* default CLKO to 1/32 of the ARM core*/
> +        ldr r1, [r0, #CLKCTL_COSR]
> +        bic r1, r1, #0x00000FF00
> +        bic r1, r1, #0x0000000FF
> +        mov r2, #0x00006C00
> +        add r2, r2, #0x67
> +        orr r1, r1, r2
> +        str r1, [r0, #CLKCTL_COSR]
> +
> +        ldr r2, =CCM_CCMR_CONFIG
> +        str r2, [r0, #CLKCTL_CCMR]
> +
> +	check_soc_version r1, r2
> +	cmp r1, #CHIP_REV_2_0
> +	ldrhs r3, =CCM_MPLL_532_HZ
> +	bhs 1f
> +	ldr r2, [r0, #CLKCTL_PDR0]
> +        tst r2, #CLKMODE_CONSUMER
> +        ldrne r3, =CCM_MPLL_532_HZ  /* consumer path*/
> +        ldreq r3, =CCM_MPLL_399_HZ  /* auto path*/
etc. etc.

Please use TAB for indentation.


> +int checkboard(void)
> +{
> +	struct ccm_regs *ccm =
> +		(struct ccm_regs *)IMX_CCM_BASE;
> +
> +	puts("Board: MX35 PDK ");
> +
> +	/*
> +	 * Be sure that I2C is initialized to check
> +	 * the board revision
> +	 */
> +	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);

Error checking?

> +	/* Print board revision */
> +	puts(board_detect() ? "2.0" : "1.0");
> + 
> +	/* Print CPU revision */
> +	puts(" i.MX35 ");

I mentioned this before:  If you make board_detect() return 1 or 2, 
you can combine the calls to puts() for example like that:

	printf("Board: MX35 PDK %d.0 i.MX35 ", board_detect());

> +	if (get_cpu_rev() & CHIP_REV_2_0)
> +		puts("2.0 [");
> +	else
> +		puts("1.0 [");

Eventually similar improvment could be done here. [Just a hint - feel
free to post-pone into a later patch if this affects other boards as
well.]



Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Romulan women are not like Vulcan females. We are  not  dedicated  to
pure logic and the sterility of non-emotion.
	-- Romulan Commander, "The Enterprise Incident",
	   stardate 5027.3