From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Tue, 25 Mar 2008 12:06:00 -0500
Subject: [U-Boot-Users] [PATCH 2/2] Freescale eLBC FCM NAND driver
In-Reply-To: <20080325164936.GA9966@game.jcrosoft.org>
References: <20080324175304.GB1280@ld0162-tx32.am.freescale.net>
	<20080325164936.GA9966@game.jcrosoft.org>
Message-ID: <47E930F8.8010005@freescale.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

Jean-Christophe PLAGNIOL-VILLARD wrote:
>> +#if defined(CONFIG_CMD_NAND) && defined(CONFIG_NAND_FSL_ELBC)
> Please move to the Makefile

OK.

>> +	/* READOOB reads only the OOB because no ECC is performed. */
>> +	case NAND_CMD_READOOB:
>> +		vdbg("fsl_elbc_cmdfunc: NAND_CMD_READOOB, page_addr:"
>> +		     " 0x%x, column: 0x%x.\n", page_addr, column);
>> +
>> +		out_be32(&lbc->fbcr, mtd->oobsize - column);
>> +		set_addr(mtd, column, page_addr, 1);
>> +
>> +		ctrl->read_bytes = mtd->writesize + mtd->oobsize;
>> +
>> +		fsl_elbc_do_read(chip, 1);
>> +		fsl_elbc_run_command(mtd);
>> +		
>    ^^^^^^^^^^^^^
> WhiteSpace please remove

Grr... I keep forgetting to enable the post-commit hook in new git 
repositories.  I wish it could be configured globally in a ~/.gitrc or 
similar.

Of course, I also wish people wouldn't get so bent out of shape about 
blank lines being indented to the level of the code around them.

>> +static void fsl_elbc_select_chip(struct mtd_info *mtd, int chip)
>> +{
>> +	/* The hardware does not seem to support multiple
>> +	 * chips per bank.
>> +	 */
> please use this comment style

Do we really need such trivial style differences with the Linux driver, 
which will only serve as an annoyance when keeping the two in sync, not 
to mention wasting vertical space with no readability benefit because 
there's pretty much always a (mostly) blank line above the comment?

>> +	return i == len && ctrl->status == LTESR_CC ? 0 : -EIO;
> please use something like that to help the reading
> 	return ((i==len) && (ctrl->status==LTESR_CC))? 0 : -EIO;

That's not the style I've generally seen used.

-Scott