Hi Brian, On Wed, 2 Jul 2014 17:22:37 -0700 Brian Norris <computersforpeace@xxxxxxxxx> wrote: > Hi Lee, > > On Wed, May 28, 2014 at 10:20:05AM +0100, Lee Jones wrote: > > This is a squashed version of the submission to avoid re-sending the > > entire set over and over, essentially clogging up the MLs. > > Thanks. I think I'd prefer to accept your driver in a form like this > too. A few comments below. > > And I'll get one big comment out of the way here: can you abstract your > ST BBT code into its own self-contained portion, preferably in a > separate source file, a la nand_bbt.c? Then, provide a way to optionally > use either your ST BBT or the existing BBT -- perhaps a NAND_BBT_ST flag > for chip->bbt_options, and a matching device tree property. That way, > even though you require a legacy format for bootloader interoperability, > someone can theoretically utilize more mainstream (albeit, not > necessarily better...) BBT support from nand_bbt.c. I think this will > provide the best balance between your existing product support and > upstream-friendly modularity/flexibility. I'm open to other suggestions, > of course. > > > Cc: computersforpeace@xxxxxxxxx > > Cc: Gupta, Pekon" <pekon@xxxxxx> > > Cc: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxxxxxxxxxx> > > Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx > > Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx> > > --- > > Please add versioning to your next patch(es), and describe changes here. > > > Documentation/devicetree/bindings/mtd/stm-nand.txt | 87 + > > See: > > Documentation/devicetree/bindings/submitting-patches.txt > [...] > > + > > + nand_timing0: nand-timing { > > + sig-setup = <10>; > > + sig-hold = <10>; > > + CE-deassert = <0>; > > + WE-to-RBn = <100>; > > + wr-on = <10>; > > + wr-off = <30>; > > + rd-on = <10>; > > + rd-off = <30>; > > + chip-delay = <30>; /* delay in us */ > > + }; > > You didn't document any of this node. And I don't think we want to > specify every single timing parameter in DT; it may make sense to use > Boris Brezillon's approach (I note this further down, in the driver > code) for mapping non-ONFI NAND timings into a compatible ONFI timing > mode. This will greatly simplify the bindings needed, since it's > standardized and auto-detectable in many cases. AFAIR, the NAND timing representation for non-ONFI chips question was left unanswered: https://lkml.org/lkml/2014/5/20/581 I can definitely respin my NAND timings series, but I'd like to be sure this is how you want it done before doing so. Just as a reminder, you and Jason thought NAND timings for non-ONFI chips could be auto detected thanks to READID informations (by storing some sort of "NANDID <-> timings" association table). Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- 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