On Fri, May 2, 2014 at 1:56 PM, Scott Wood <scottw...@freescale.com> wrote:
Hi Scott,
> On Mon, 2014-04-28 at 13:17 -0700, Tim Harvey wrote:
>> +static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
>> + int column, int page_addr)
>> +{
>> + register struct nand_chip *chip = mtd->priv;
>> + int ctrl = NAND_NCE | NAND_CTRL_CLE | NAND_CTRL_CHANGE;
>> + u32 timeo, time_start;
>> +
>> + /* write out the command to the device */
>> + chip->cmd_ctrl(mtd, command, ctrl);
>> +
>> + /* Address cycle, when necessary */
>> + ctrl = NAND_NCE | NAND_CTRL_ALE | NAND_CTRL_CHANGE;
>> + /* Serially input address */
>> + if (column != -1) {
>> + ctrl = NAND_CTRL_ALE | NAND_CTRL_CHANGE;
>
> What happened to NAND_NCE?
>
> It looks like the cmd_ctrl in nand_mxs.c doesn't care about NAND_MCE at
> all, so there's no functional impact, but either use it consistently or
> don't use it at all.
>
>> + chip->cmd_ctrl(mtd, column, ctrl);
>> + ctrl &= ~NAND_CTRL_CHANGE;
>> + chip->cmd_ctrl(mtd, column >> 8, ctrl);
>> + }
>
> Why not pass the ctrl flags directly to cmd_ctrl as you do in most of
> the rest of the function? nand_mxs.c doesn't even look at NAND_CTRL_CHANGE.
I'll simplify using only what nand_mxs.c pays attention to (NAND_ALE,
NAND_CLE), remove the ctrl var and repost
>
>> +int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf)
>> +{
>> + register struct nand_chip *chip;
>
> Why "register"?
cut-n-paste code from nand_base.c - will remove
>
>> + if (!(page % nand_page_per_block)) {
>> + /*
>> + * Yes, new block. See if this block is good. If not,
>> + * loop until we find a good block.
>> + */
>> + while (is_badblock(&mtd, offs, 1)) {
>> + page = page + nand_page_per_block;
>> + /* Check i we've reached the end of flash. */
>> + if (page >= mtd.size >> chip->page_shift)
>> + return -1;
>
> Why -1 rather than a real error value?
probably from the example I used from drivers/mtd/nand/mxc_nand_spl.c.
The calling functions don't appear to do any error checking, but I'll
replace it with -ENOMEM and -ENODEV if mxs_nand_init() fails.
Thanks for the review!
Tim
>
> -Scott
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
