From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver Date: Thu, 24 May 2018 14:27:36 +0200 Message-ID: <20180524142736.2ef157fe@bbrezillon> References: <86fdf19ec92b732709732fb60199f16488b4b727.1526990589.git.stefan@agner.ch> <20180523161810.0ed9fe80@bbrezillon> <2d8107f0e6568512d691e9ea25a1e4e5@agner.ch> <20180524105614.3c51736c@bbrezillon> <7b3cc3991fb054130fd54c6fdfec5097@agner.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <7b3cc3991fb054130fd54c6fdfec5097@agner.ch> Sender: linux-kernel-owner@vger.kernel.org To: Stefan Agner Cc: dwmw2@infradead.org, computersforpeace@gmail.com, marek.vasut@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com, thierry.reding@gmail.com, mturquette@baylibre.com, sboyd@kernel.org, dev@lynxeye.de, miquel.raynal@bootlin.com, richard@nod.at, marcel@ziswiler.com, krzk@kernel.org, digetx@gmail.com, benjamin.lindqvist@endian.se, jonathanh@nvidia.com, pdeschrijver@nvidia.com, pgaikwad@nvidia.com, mirza.krak@gmail.com, linux-mtd@lists.infradead.org, linux-tegra@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org List-Id: devicetree@vger.kernel.org On Thu, 24 May 2018 13:09:53 +0200 Stefan Agner wrote: > On 24.05.2018 10:56, Boris Brezillon wrote: > > On Thu, 24 May 2018 10:46:27 +0200 > > Stefan Agner wrote: > > > >> Hi Boris, > >> > >> Thanks for the initial review! One small question below: > >> > >> On 23.05.2018 16:18, Boris Brezillon wrote: > >> > Hi Stefan, > >> > > >> > On Tue, 22 May 2018 14:07:06 +0200 > >> > Stefan Agner wrote: > >> >> + > >> >> +struct tegra_nand { > >> >> + void __iomem *regs; > >> >> + struct clk *clk; > >> >> + struct gpio_desc *wp_gpio; > >> >> + > >> >> + struct nand_chip chip; > >> >> + struct device *dev; > >> >> + > >> >> + struct completion command_complete; > >> >> + struct completion dma_complete; > >> >> + bool last_read_error; > >> >> + > >> >> + dma_addr_t data_dma; > >> >> + void *data_buf; > >> >> + dma_addr_t oob_dma; > >> >> + void *oob_buf; > >> >> + > >> >> + int cur_chip; > >> >> +}; > >> > > >> > This struct should be split in 2 structures: one representing the NAND > >> > controller and one representing the NAND chip: > >> > > >> > struct tegra_nand_controller { > >> > struct nand_hw_control base; > >> > void __iomem *regs; > >> > struct clk *clk; > >> > struct device *dev; > >> > struct completion command_complete; > >> > struct completion dma_complete; > >> > bool last_read_error; > >> > int cur_chip; > >> > }; > >> > > >> > struct tegra_nand { > >> > struct nand_chip base; > >> > dma_addr_t data_dma; > >> > void *data_buf; > >> > dma_addr_t oob_dma; > >> > void *oob_buf; > >> > }; > >> > >> Is there a particular reason why you would leave DMA buffers in the chip > >> structure? It seems that is more a controller thing... > > > > The size of those buffers is likely to be device dependent, so if you > > have several NANDs connected to the controller, you'll either have to > > have one buffer at the controller level which is max(all-chip-buf-size) > > or a buffer per device. > > > > Also, do you really need these buffers? The core already provide some > > which are suitable for DMA (chip->oob_poi and chip->data_buf). > > > > Good question, I am not sure, that was existing code. > > Are you sure data_buf it is DMA capable? > > nand_scan_tail allocates with kmalloc: > > chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL); Also, you might want to set the NAND_USE_BOUNCE_BUFFER flag in chip->options, so that the core always pass DMA-able buffers to your ->read/write_page[_raw] function. [1]https://elixir.bootlin.com/linux/v4.17-rc6/source/include/linux/mtd/rawnand.h#L202