Hi Brain, > From: Brian Norris [mailto:computersforpeace@xxxxxxxxx] > > On Thu, Oct 17, 2013 at 04:42:23AM +0000, Pekon Gupta wrote: > > > From: Brian Norris [mailto:computersforpeace@xxxxxxxxx] > > > > On Tue, Oct 15, 2013 at 11:19:52AM +0530, Pekon Gupta wrote: [snip] > > So this approach is other way round, where controller is configured > > based on DT binding "nand-bus-width" and then it checks whether > > device actual buswidth matches DT binding value or not. > > If this is the case, then you really don't want to use > NAND_BUSWIDTH_AUTO. The default nand_base.c just *validates* the > pre-selected buswidth and exits with an error, in much the same way that > you're doing here (you're just duplicating the functionality). > NAND_BUSWIDTH_AUTO is useful if you want to turn control of the > buswidth > configuration entirely over to nand_base.c. > > > - GPMC controller is pre-configured to support "x8" or "x16" device based > > on DT binding "nand-bus-width". > > Reference: $LINUX/arch/arm/mach-omap2/gpmc.c > > @@gpmc_probe_nand_child() > > val = of_get_nand_bus_width(child); > > > > - But, bus-width of NAND device is only known during NAND driver > > Probe in nand_scan_ident(). > > > > Going forward when all NAND devices are compliant to ONFI, > > then we can deprecate "nand-bus-width". > > Buswidth auto-detection is not actually restricted to ONFI. nand_base.c > has (corretly, AFAIK) been detecting buswidths for a long time, using > some form of ID decode detection. But it was just that: detection. It > did not actually configure the buswidth, since it left that > responsibility to the low-level driver to get right. > > Another thing to consider: I think this current patch is a regression > from previous behavior. Previously, you would run nand_scan_ident() > twice if the first one failed; once with the platform-configured > buswidth and once with the opposite. But now, you only run > nand_scan_ident() once, and then you quit if it detects differently than > you expected. > Actually having nand_scan_ident() called again if first time it fails, is _not_ the right way, it was a quick-fix work-around. It should not have been approved.. > My opinion: the platform- and device-tree-provided buswidth is > unnecessary; it was previouisly only a suggestion which your driver > would readily discard, and it isn't really needed now. You can probably > get by with just NAND_BUSWIDTH_AUTO and (at most) a warning if the > auto-configured buswidth is different than the platform specified. > > But the real point: you need to clearly communicate what you are > choosing in this patch. Either you want to > > (1) strictly follow the buswidth provided by the platform or > > (2) use the nand_base.c BUSWIDTH_AUTO automatic configuration. > Ideally, I would like to go with (2), but that would need other changes in-order to re-configure GPMC with newly parsed ONFI data, which can be a separate patch-set. Thus, I would drop this patch for now. And go with (1), with following updates in omap_nand_probe(). (a) perform nand_scan_ident() in "x8" mode, so that ONFI params are read and device-info (chip->writesize, chip->oobsize) are populated. (b) Then switch to "x8" or "x16" mode based on "nand-bus-width" as passed from GPMC driver to NAND driver via platform-data. And re-populate mtd->byte, mtd-read_buf, mtd->write_buf. with regards, pekon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
