> From: Scott Wood [mailto:scottw...@freescale.com] > > On Fri, 2013-09-27 at 04:18 +0000, Gupta, Pekon wrote: > > > From: Scott Wood [mailto:scottw...@freescale.com] > > > > > > On Thu, 2013-09-26 at 13:14 +0000, Gupta, Pekon wrote: > > > > > > From: Gupta, Pekon <pe...@ti.com> > > > > > > [snip] > > The changelog that introduced am335x_spl_bch.c says that "custom > read_page implementation is required for proper syndrome generation." > Other than that ECC mode, AFAICT you're already using nand_spl_simple.c. > Oh I missed referring to nand_spl_simple.c. Thanks for pointing out. Yes, we already have a generic SPL driver.
> > > > because SPL is mostly statically configured, and it just does plain NAND > read > > > > (it doesn’t even support NAND write, etc). > > > > But I do not know the hardware configurations tweaks of other SoC, > > > > So at-least have common CONFIG_SYS_NAND_xx which all SPL drivers > can > > > use. > > > > > > Again, are you introducing this symbol for use only in SPL? > > Apart from SPL, CONFIG_SYS_NAND_DEVICE_WIDTH also be useful for > > (1) drivers which do not use CONFIG_SYS_NAND_ONFI_DETECTION, where > > code for reading on-chip ONFI parameters is not enabled in nand_base.c > > (2) non ONFI compatible NAND devices. > > Unlikely, given that they've all managed to work without this so far. > E.g. eLBC and IFC hardcode this information on a per-chip basis in the > #defines that hold values for config registers, and prior to this patch > omap_gpmc had code to read a config register (regardless of where it > originally got set). > (1) drivers/mtd/nand/fsl_ifc_spl.c They are doing same way as OMAP used to. They are also using controller configurations to tell driver about the]NAND bus-width "port_size = (cspr & CSPR_PORT_SIZE_16) ? 16 : 8;" (2) drivers/mtd/nand/fsl_elbc_spl.c They are doing incomplete check. Rather they are not caring for x16 device For a x16 device, badblock marker should be 16-bit (2 bytes) but here they are checking of only 1st byte. "if (i++ < 2 && buf[page_offs + bad_marker] != 0xff)" So CONFIG_SYS_NAND_DEVICE_WIDTH should help them also. right ? So can this new CONFIG_xx be accepted ? > > > It looked > > > like you were removing the code that does dynamic detection, which > would > > > also affect non-SPL. > > > > > > > - /* If we are 16 bit dev, our gpmc config tells us that */ > > > > - if ((readl(&gpmc_cfg->cs[cs].config1) & 0x3000) == 0x1000) > > > > omap_gpmc.c never had dynamic detection support. Above gpmc_config > bit > > which is used to tell whether device is x16 or x8, gets actually hard-coded > > in > > gpmc_init(). Thus it was actually a mechanism to pass hard-coded bus- > width > > information to nand driver. > > Refer: arch/arm/cpu/armv7/am33xx/mem.c : gpmc_init() > > > > So, instead of hacking the gpmc_init() everytime for different devices, > > this patch introduces a generic CONFIG which can be used everywhere. > > It looks like you do more NAND config in gpmc_init() than just setting > this one bit, so I don't think you save anything here. > > BTW, do you not need to set this bit in the config register for the > hardware to work in the SPL case? > Yes, I'm not changing the default configs for GPMC in gpmc_init(), because they are ok for x8 device. I'm just overriding them again during board_nand_init() if CONFIG_SYS_NAND_DEVICE_WIDTH == x16 device. + /* reconfigure GPMC.CONFIG1 register with correct device-width */ + gpmc_config = readl(&gpmc_cfg->cs[cs].config1); + if (nand->options & NAND_BUSWIDTH_16) + gpmc_config |= (0x1 << 12); + else + gpmc_config &= ~(0x1 << 12); + writel(gpmc_config, &gpmc_cfg->cs[cs].config1); + This way im not breaking any backward compatibility, just avoiding the need to hack the board file for x16 devices. with regards, pekon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot