Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753507AbdF0UKA (ORCPT ); Tue, 27 Jun 2017 16:10:00 -0400 Received: from 1.mo173.mail-out.ovh.net ([178.33.111.180]:37822 "EHLO 1.mo173.mail-out.ovh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753132AbdF0UJw (ORCPT ); Tue, 27 Jun 2017 16:09:52 -0400 Subject: Re: [PATCH v3 0/2] mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables To: Cyrille Pitchen , marek.vasut@gmail.com Cc: boris.brezillon@free-electrons.com, richard@nod.at, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, computersforpeace@gmail.com References: From: Cyrille Pitchen Message-ID: Date: Tue, 27 Jun 2017 22:03:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Ovh-Tracer-Id: 13403838394654087134 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: 0 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelkedrtddtgdduvdelucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenuc Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4245 Lines: 107 Le 26/06/2017 à 15:09, Cyrille Pitchen a écrit : > Hi all, > > a new version of the SFDP patch based on next-20170626 > > tested on sama5d2 xplained with the following QSPI memories: > Macronix > - mx25l25673g > > Spansion/Cypress > - s25fl164 > - s25fl127 > - s25fl512 > > Winbond > - w25q256 > - w25m512 > > SST > - sst26vf064 > > Micron > - m25q128 > - n25q128a > - m25ql512 > - m25ql01g > > For my tests, I used mtd_debug to erase, write then read back some areas > inside data array of the SPI NOR flash memory. > To verify the integrity of the data, I used sha1sum to compare the > original file with the one read from the SPI flash memory. > > For memories with a non-uniform erase map (sst26vf064, s25fl512, ...), I > chose an offset in the data array so the sector size is the one set into > nor->mtd.erasesize. > > CONFIG_MTD_SPI_NOR_USE_4K_SECTORS was defined in my .config file. > Older tests were also run with this macro undefined and with other memory > parts. > Here I'm only talking about tests performed within the last few days. > > For sst26vf064, the Global Unlock Block Protection command was already > sent to the memory by the bootloader so once in Linux, the memory was in a > rw mode, otherwise Sector Erase and Page Program would fail. > > Flash unlock (block protection) and non-uniform erase map are out of the > scope of this patch and would be addressed later in dedicated patches. > > I added/updated few entries in the spi_nor_ids[] array so I could use all > the memory parts listed above with SFDP support. > > My s25fl127 sample is buggy and I don't know whether the issue has been > fixed by Cypress with the later revisions of this part number (s25fl128s): > The SFDP data programmed in my sample claim that the memory is compliant > with JESD216 rev B (version 1.6) however DWORDs 10 to 16 of the Basic > Flash Parameter Table are all 0xFFFFFFFF, only the first 9 DWORDs are > programmed correctly as is the memory was only compliant with JESD216 > (version 1.0). Hence when testing the QER bits in DWORD15, the reserved > value 111b is read. That why I've changed the default case for QER bits > so it now returns -EINVAL when an unexpected value is read. > So the SFDP data are reported as invalid and just ignored. > I didn't use the SPI_NOR_SKIP_SFDP info->flags because s25fl127 and > s25fl128s share the same JEDEC ID and I want to give a chance to use the > SFDP tables of the later revisions of this memory part if the above > issue has been fixed. > > Best regards, > > Cyrille > > > ChangeLog > > v2 > v3: > - add a small patch to fix a conflict when the SPINOR_OP_RDSR2 macro was > defined twice: first in drivers/mtd/devices/serial_flash_cmds.h and > secondly in include/linux/mtd/spi-nor.h. It resulted in a build warning. > - add the missing () in the kernel-doc comments for functions > - add a Return: section in the kernel-doc comments for functions > - improve spi_nor_read_sfdp() to take into account the case where all SFDP > data can't be read in a single nor->read() call. > > v1 -> v2: > - add kernel-doc to the main functions introduced by this patch. > - rename spansion_new_quad_enable() into spansion_read_cr_quad_enable(). > - add spansion_no_read_cr_quad_enable(): the explanation is given in > the kernel-doc. > - take Marek's comments into account (add new lines, remove parenthesis, > keep new error messages on the same line as dev_err(), already exisiting > error messages are left unchanged, use u32 instead of int for >> > operand, use sizeof(u32) instead of sizeof(uint32_t)). > - propagate return code of spi_nor_read_sfdp(). > - handle the default case of the QER bits differently: now returns -EINVAL > > > Cyrille Pitchen (2): > mtd: st_spi_fsm: remove SPINOR_OP_RDSR2 and use SPINOR_OP_RDCR instead > mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables > > drivers/mtd/devices/serial_flash_cmds.h | 1 - > drivers/mtd/devices/st_spi_fsm.c | 4 +- > drivers/mtd/spi-nor/spi-nor.c | 775 +++++++++++++++++++++++++++++++- > include/linux/mtd/spi-nor.h | 6 + > 4 files changed, 770 insertions(+), 16 deletions(-) > series applied to the spi-nor/next branch of l2-mtd