On Wed, 30 Nov 2016 17:02:16 +0900
Masahiro Yamada <yamada.masah...@socionext.com> wrote:
> 2016-11-28 0:04 GMT+09:00 Boris Brezillon
> > +Andy
> > Hi Masahiro,
> > On Sun, 27 Nov 2016 03:05:46 +0900
> > Masahiro Yamada <yamada.masah...@socionext.com> 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)?
> I want to remove
> The driver should not hard-code timing parameters of Samsung specific
> chips. For ONFI, it is duplicating effort of the core framework.
> 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
> 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).
> 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.
> 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.