Re: [U-Boot] [PATCH 08/13 v4] ARM: OMAP3: Add NAND support

On Fri, Oct 17, 2008 at 09:28:50PM +0200, dirk.behme@googlemail.com wrote:

• if (nand_curr_device < 0 ||

• nand_curr_device >= CONFIG_SYS_MAX_NAND_DEVICE ||
  

• !nand_info[nand_curr_device].name) {
  

• printf("Error: Can't switch ecc, no devices available\n");
  

• return;
  

• }

It's much more readable if you align the continuation lines with the beginning of the condition, rather than with the if-body.

• mtd = &nand_info[nand_curr_device];
• nand = mtd->priv;
• /* clean up allocated buffers */
• nand_release(mtd);
• /* mark me unscanned */
• nand->options &= ~NAND_BBT_SCANNED;

Better to set it back to the original value explicitly, in case other internal NAND flags crop up.

• /* Setup the ecc configurations again */
• if (!hardware) {

• nand->ecc.mode = NAND_ECC_SOFT;
  

• /* Use mtd default settings */
  

• nand->ecc.layout = NULL;
  

• } else {

• nand->ecc.mode = NAND_ECC_HW;
  

• nand->ecc.layout = &hw_nand_oob;
  

• nand->ecc.size = 512;
  

• nand->ecc.bytes = 3;
  

• nand->ecc.steps = hw_nand_oob.eccbytes / nand->ecc.bytes;
  

No need to set ecc.steps; nand_scan_tail() will do it.

+/* Small Page x8 NAND device Layout */ +#ifdef GPMC_NAND_ECC_SP_x8_LAYOUT +#define GPMC_NAND_HW_ECC_LAYOUT {\

• .eccbytes = 3,\
• .eccpos = {1, 2, 3},\
• .oobfree = {\

• {.offset = 4,\
  

•  .length = 13 } } \
  

+}

.length = 12

+#endif

+/* Small Page x16 NAND device Layout */ +#ifdef GPMC_NAND_ECC_SP_x16_LAYOUT +#define GPMC_NAND_HW_ECC_LAYOUT {\

• .eccbytes = 3,\
• .eccpos = {2, 3, 4},\
• .oobfree = {\

• {.offset = 4,\
  

•  .length = 12 } } \
  

+}

.offset = 5 .length = 11

Otherwise ACK, if the ARM maintainers are OK with passing integers as addresses to writel().

-Scott