Hi Brian, > From: Brian Norris [mailto:computersforpeace@xxxxxxxxx] > On Thu, Oct 17, 2013 at 09:00:27PM +0000, Pekon Gupta wrote: > > > From: Brian Norris [mailto:computersforpeace@xxxxxxxxx] > > > > On Thu, Oct 17, 2013 at 04:42:23AM +0000, Pekon Gupta wrote: [...] > > > 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. > > What exactly needs changed to support this? It looks like this omap2 > NAND driver doesn't make assumptions about 8-bit vs. 16-bit until after > nand_scan_ident(). But maybe there is something I missed. > The GPMC driver will be touched by NAND_BUSWIDTH_AUTO change. There are two set of configurations in GPMC controller.. (a) device based configurations: These are static configurations derived on DT bindings for each chip-select (like NAND signal timings, etc). These done on GPMC probe. (b) ecc-scheme based configurations: These are dynamic configurations done in NAND driver in chip->ecc.hwctl() and are refreshed at each NAND accesss. However, 'nand-bus-width' configuration is part of both (1) and (2). Thus, both 'OMAP NAND driver' and 'GPMC driver' need to be consistent in programming 'nand-bus-width' otherwise ecc-engine would not work. Thus, I'm proceeding with (1) DT based parsing of 'nand-bus-width'. > > Thus, I would drop this patch for now. And go with (1), > > OK, but to drop this patch entirely would still not be (1); it would > still leave the possibility of calling nand_scan_ident() twice, and it > puts you in a middle ground between (1) and (2). That's fine if it works > for you, but I just want to acknowledge that now: this driver requires > changes to become either (1) or (2). > I have re-posted [PATCH v10 4/10] with this updates. However, please take into account that some limitation of dual programming require such probe. New patch also call nand_scan_ident() twice, but only on certain pre-determined conditions, not in all failure. Once I convert to NAND_BUSWIDTH_AUTO these should get clean, as I would update both GPMC and NAND driver for this. (Till then this was most optimal solution I could think of).. > Does your series need a rebase if we're removing this patch? Or you're > rewriting/simplifying it to the following two points? (Perhaps it's > best to separate this portion to its own patch (set) and discussion, > since it is independent of your ECC rewrite.) > > > 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. > > OK. > > > (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. > > I'm not sure if switching buswidth after nand_scan_ident() is really > supported, but I'll hold off until I see the code you're proposing. > I have re-posted [PATCH v10 4/10] with this updates. Please accept this ... with regards, pekon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html