From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from down.free-electrons.com ([37.187.137.238] helo=mail.free-electrons.com) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1afpQ9-0001Ok-Vx for linux-mtd@lists.infradead.org; Tue, 15 Mar 2016 13:54:02 +0000 Date: Tue, 15 Mar 2016 14:53:39 +0100 From: Boris Brezillon To: Jorge Ramirez-Ortiz , computersforpeace@gmail.com Cc: dwmw2@infradead.org, matthias.bgg@gmail.com, robh@kernel.org, daniel.thompson@linaro.org, xiaolei.li@mediatek.com, linux-mtd@lists.infradead.org Subject: Re: [PATCH 2/3] mtd: mediatek: driver for MTK Smart Device Gen1 NAND Message-ID: <20160315145339.28d50175@bbrezillon> In-Reply-To: <56E80C5F.2020406@linaro.org> References: <1456938013-8819-1-git-send-email-jorge.ramirez-ortiz@linaro.org> <1456938013-8819-3-git-send-email-jorge.ramirez-ortiz@linaro.org> <20160308172437.6eccce05@bbrezillon> <56E7FFF5.4000803@linaro.org> <20160315135936.1c924789@bbrezillon> <56E80C5F.2020406@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Tue, 15 Mar 2016 09:21:35 -0400 Jorge Ramirez-Ortiz wrote: > On 03/15/2016 08:59 AM, Boris Brezillon wrote: > > On Tue, 15 Mar 2016 08:28:37 -0400 > > Jorge Ramirez-Ortiz wrote: > > > >> On 03/08/2016 11:24 AM, Boris Brezillon wrote: > >>>> +static int mtk_nfc_write_page(struct mtd_info *mtd, > >>>>> + struct nand_chip *chip, const uint8_t *buf, > >>>>> + int oob_on, int page, int raw) > >>>>> +{ > >>>>> + > >>>>> + struct mtk_nfc_host *host = nand_get_controller_data(chip); > >>>>> + struct completion *nfi = &host->nfi.complete; > >>>>> + struct device *dev = host->dev; > >>>>> + const bool use_ecc = !raw; > >>>>> + void *q = (void *) buf; > >>>>> + dma_addr_t dma_addr; > >>>>> + size_t dmasize; > >>>>> + u32 reg; > >>>>> + int ret; > >>>>> + > >>>>> + dmasize = mtd->writesize + (raw ? mtd->oobsize : 0); > >>>>> + > >>>>> + dma_addr = dma_map_single(dev, q, dmasize, DMA_TO_DEVICE); > >>> buf is not guaranteed to be physically contiguous, so you can't just > >>> use it with DMA without doing a few more verifications. > >>> > >>> In case you're interested in using a generic approach to do this > >>> verification, you can have a look at this series [2]. > >>> > >> unfortunately the internal dma controller does not support scatter gather > >> operations (we need to DMA in/out of memory in a single shot) > >> If we enable NAND_USE_BOUNCE_BUFFER, I think this guarantees that the buffer > >> will be contiguous (since they are allocated with kmalloc) > >> ...although maybe I should have the bounce buffers in the driver and allocate > >> them with devm_get_free_pages instead > >> > >> would either of this would be acceptable? > > Or you could test if the buffer is contiguous, and fallback to using > > a bounce buffer (either an internal one or the generic one) if that's > > not the case. Note that the proposed API can be improved to reject > > non-contiguous buffers... > > wouldn't that check be the same than the one done in the nand interface when > NAND_USE_BOUNCE_BUFFER is enabled? > IIRC virt_addr_valid() guarantees that the buffer is contiguous. Correct, I forgot we had that test, and thought we were always using the bounce buffer when NAND_USE_BOUNCE_BUFFER was set. Still, in some cases, you might have vmallocated buffers that are physically contiguous (say you have NAND page < PAGE_SIZE). Anyway, I guess setting NAND_USE_BOUNCE_BUFFER and assuming the buf passed to ->read/write_page() is always pointing to a physically contiguous region is an acceptable option until we have this mtd_map_buf() API ready. Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com