Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754797AbcK3IRe (ORCPT ); Wed, 30 Nov 2016 03:17:34 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:46721 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751148AbcK3IRZ (ORCPT ); Wed, 30 Nov 2016 03:17:25 -0500 Date: Wed, 30 Nov 2016 09:17:22 +0100 From: Boris Brezillon To: Masahiro Yamada Cc: linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, Linux Kernel Mailing List , Marek Vasut , Brian Norris , Richard Weinberger , David Woodhouse , Cyrille Pitchen , Rob Herring , Mark Rutland , Andy Shevchenko Subject: Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb Message-ID: <20161130091722.2e35f32a@bbrezillon> In-Reply-To: References: <1480183585-592-1-git-send-email-yamada.masahiro@socionext.com> <20161127160459.5894be93@bbrezillon> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3577 Lines: 113 On Wed, 30 Nov 2016 17:02:16 +0900 Masahiro Yamada wrote: > Hi. > > 2016-11-28 0:04 GMT+09:00 Boris Brezillon : > > +Andy > > > > Hi Masahiro, > > > > On Sun, 27 Nov 2016 03:05:46 +0900 > > Masahiro Yamada 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. > >