On Wed, 30 Nov 2016 17:02:16 +0900 Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote: > Hi. > > 2016-11-28 0:04 GMT+09:00 Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>: > > +Andy > > > > Hi Masahiro, > > > > On Sun, 27 Nov 2016 03:05:46 +0900 > > Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote: > > > >> As I said in the 1st round series, I am tackling on this driver > >> to use it for my SoCs. > >> > >> The previous series was just cosmetic things, but this series > >> includes *real* changes. > >> > >> After some more cleanups, I will start to add changes that > >> are really necessary. > >> One of the biggest problems I want to solve is a bunch of > >> hard-coded parameters that prevent me from using this driver for > >> my SoCs. > >> > >> I will introduce capability flags that are associated with DT > >> compatible and make platform-dependent parameters overridable. > >> > >> I still have lots of reworks to get done (so probably 3rd round > >> series will come), but I hope it is getting better and > >> I am showing a big picture now. > >> > > > > Thanks for posting this 2nd round of patches, I know have a clearer > > view of what you're trying to achieve. > > Could you be a bit more specific about the remaining rework (your 3rd > > round)? > > > [1] > I want to remove > get_samsung_nand_para() > get_onfi_nand_para() > > The driver should not hard-code timing parameters of Samsung specific > chips. For ONFI, it is duplicating effort of the core framework. Definitely. > > I am thinking if it would be possible to implement > chip->setup_data_interface() in order to set up > timings in a generic way. Indeed, and that'd be really cool to have this driver converted to this new interface. > > [2] > Remove driver-internal bounce buffer. > The current Denali driver allocate DMA_BIDIRECTIONAL buffer > to use it as a driver-internal bounce buffer. > > The hardware transfer page data into the bounce buffer, > then CPU copies from the bounce buffer to a given buf (and oob_poi). > This is not efficient. > > So, I want to set NAND_USE_BOUNCE_BUFFER flag > and do dma_map_single directly for a given buffer. Sounds good. Be careful though, when you use the generic bounce buffer interface you might have to clear the page cache info (->pagebuf = -1). > > [3] > Fix raw and oob callbacks. > > I asked in another thread, > the current driver just puts the physically accessed OOB data > into oob_poi, which is not a collection of ECC data. > Raw write/read() are wrong as well. That's all good things too. > > After fixing those, enable BBT scan by removing the following flag: > /* skip the scan for now until we have OOB read and write support */ > chip->options |= NAND_SKIP_BBTSCAN; > Hm, here you have a problem. The layout you described replaces BBMs by payload data, thus preventing the BBM scan approach (or at least, it won't work with factory BBMs). Some drivers/controllers have an extra 'switch BBM/data bytes' step to restore the BBM at the correct place before flushing the data to the NAND or after reading a page, but I'm not sure this is the case here. > > > > Also, if you don't mind, I'd like to have reviews and testing from intel > > users before applying the series. Can you Cc Andy (and possibly other > > intel maintainers) for the next round. > > Sure. > > Anyway, this series already missed the pull-req for 4.10-rc1, > we have plenty of time until 4.11-rc1. > > Review/test from Intel engineers are very appreciated > because I have no access to their boards. > > -- 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
