From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752809AbcLGNH4 (ORCPT ); Wed, 7 Dec 2016 08:07:56 -0500 Received: from eusmtp01.atmel.com ([212.144.249.243]:7562 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752008AbcLGNHy (ORCPT ); Wed, 7 Dec 2016 08:07:54 -0500 Subject: Re: [PATCH 1/1] mtd: spi-nor: remove WARN_ONCE() message in spi_nor_write() To: "Krzeminski, Marcin (Nokia - PL/Wroclaw)" , Marek Vasut , Cyrille Pitchen References: <0078578d0f5d2402ac623afabf601d25998f84a9.1481044434.git.cyrille.pitchen@atmel.com> <096f8b4c-b00a-af89-e667-7385226c2e7e@gmail.com> <0b871c04-c105-ec94-441c-481e9773f19e@wedev4u.fr> <7dd7fecc-7f03-659a-ef00-a1ad69daceca@gmail.com> CC: "boris.brezillon@free-electrons.com" , "computersforpeace@gmail.com" , "linux-mtd@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "richard@nod.at" From: Cyrille Pitchen Message-ID: <530a690a-d0df-3e10-2c4d-f363d94f356a@atmel.com> Date: Wed, 7 Dec 2016 14:07:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.145.133.18] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 07/12/2016 à 07:21, Krzeminski, Marcin (Nokia - PL/Wroclaw) a écrit : > Hi Cyrille, > >> -----Original Message----- >> From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf >> Of Marek Vasut >> Sent: Wednesday, December 07, 2016 4:07 AM >> To: Cyrille Pitchen ; Cyrille Pitchen >> >> Cc: boris.brezillon@free-electrons.com; computersforpeace@gmail.com; >> linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; >> richard@nod.at >> Subject: Re: [PATCH 1/1] mtd: spi-nor: remove WARN_ONCE() message in >> spi_nor_write() >> >> On 12/07/2016 12:38 AM, Cyrille Pitchen wrote: >>> Le 06/12/2016 à 20:00, Marek Vasut a écrit : >>>> On 12/06/2016 06:14 PM, Cyrille Pitchen wrote: >>>>> This patch removes the WARN_ONCE() test in spi_nor_write(). >>>>> This macro triggers the display of a warning message almost every >>>>> time we use a UBI file-system because a write operation is performed >>>>> at offset 64, which is in the middle of the SPI NOR memory page. >>>>> This is a valid operation for ubifs. >>>> >>>> Is that a valid operation for all spi nors ? >>>> >>> >>> AFAIK, yes, it is. First we need to erase a sector or a block, then we >>> can send page program commands to write data into the memory. We >>> cannot write more than a page size within a single page program >>> command but you can write less and start in the middle of a page if you >> want. >> > Technically you can, but for some chips this warning is indeed right, and at > least force the user to take a look. See this: > > http://www.macronix.com/Lists/ApplicationNote/Attachments/1606/AN0302V1%20-%20MX25L_G%20Serial%20Flash%20Programming%20Guide.pdf > This Macronix document recommends to align both the start offset and the length to a 16-byte boundary. However the WARN_ONCE() macro only checks the start offset but doesn't test the length. Also it tests a page-size alignment, which is a stronger constraint than a 16-byte alignment. In the case of Macronix mx25l_g memories, when the UBI layer writes at offset 64, the warning is a false positive, isn't it? Also WARN_ONCE() dumps the call stack making people think of a kernel oops, displayed once per boot when mounting a ubifs partition. If the issue exists, printing a warning does not fix it. So what should we do? Best regards, Cyrille > Thanks, > Marcin > >> I wasn't aware this partial and even unaligned programming was available on >> all chips, OK. >> >>> I don't know whether we could cross the page boundary if we start >>> writing from the middle of a page as long as we write less data than a >>> single page size. However spi_nor_write() don't do so, this is why it >>> computes page_remain = min_t(size_t, nor->page_size - page_offset, len >>> - i) >> >> No, I don't think we can, I believe the PP would wrap around and program >> the same page from the beginning. >> >>> Well, now looking at the Spansion S25FL127S datasheet, the address is >>> wrapped if we cross the page boundary: >> >> Yeah, this matches my mental model. >> >>> "Depending on the device configuration, the page size can either be >>> 256 or 512 bytes. Up to a page can be provided on SI after the 3-byte >>> address with instruction 02h or 4-byte address with instruction 12h >>> has been provided. If the 9 least significant address bits (A8-A0) are >>> not all 0, all transmitted data that goes beyond the end of the >>> current page are programmed from the start address of the same page >>> (from the address whose 9 least significant bits (A8-A0) are all 0) >>> i.e. the address wraps within the page aligned address boundaries. >>> This is a result of only requiring the user to enter one single page >>> address to cover the entire page boundary." >>> >>> Then from Adesto AT25DF321A datasheet: >>> "If the starting memory address denoted by A23-A0 does not fall on an >>> even 256-byte page boundary (A7-A0 are not all 0), then special >>> circumstances regarding which memory locations to be programmed will >>> apply. In this situation, any data that is sent to the device that >>> goes beyond the end of the page will wrap around back to the beginning >>> of the same page. For example, if the starting address denoted by >>> A23-A0 is 0000FEh, and three bytes of data are sent to the device, >>> then the first two bytes of data will be programmed at addresses >>> 0000FEh and 0000FFh while the last byte of data will be programmed at >>> address 000000h. The remaining bytes in the page (addresses 000001h >>> through 0000FDh) will not be programmed and will remain in the erased >>> state (FFh). In addition, if more than 256-bytes of data are sent to >>> the device, then only the last 256-bytes sent will be latched into the >> internal buffer." >>> >>> >>> Besides, the wear leveling is handled by the ubi layer I guess, at the >>> spi-nor level we write raw data. Maybe Richard and Boris could tell us >>> more but talking with them I've understood that's it is normal for the >>> ubi layer to write at offset 64. >> >> I'd understand RMW, but pure write seems a bit odd. >> >> >> -- >> Best regards, >> Marek Vasut >> >> ______________________________________________________ >> Linux MTD discussion mailing list >> http://lists.infradead.org/mailman/listinfo/linux-mtd/ >