Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757355AbcCURQj (ORCPT ); Mon, 21 Mar 2016 13:16:39 -0400 Received: from eusmtp01.atmel.com ([212.144.249.242]:32393 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757304AbcCURQh (ORCPT ); Mon, 21 Mar 2016 13:16:37 -0400 Subject: Re: [PATCH 1/1] mtd: spi-nor: add an alternative method to support memory >16MiB To: Brian Norris References: <1458556429-11741-1-git-send-email-cyrille.pitchen@atmel.com> <20160321170107.GG2545@google.com> From: Cyrille Pitchen CC: , , , , Message-ID: <56F02C70.6050106@atmel.com> Date: Mon, 21 Mar 2016 18:16:32 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160321170107.GG2545@google.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.161.30.18] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3433 Lines: 87 Hi Brian, Le 21/03/2016 18:01, Brian Norris a ?crit : > On Mon, Mar 21, 2016 at 11:33:49AM +0100, Cyrille Pitchen wrote: >> This patch provides an alternative mean to support memory above 16MiB >> (128Mib) by replacing 3byte address op codes by their associated 4byte >> address versions. >> >> Using the dedicated 4byte address op codes doesn't change the internal >> state of the SPI NOR memory as opposed to using other means such as >> updating a Base Address Register (BAR) and sending command to enter/leave >> the 4byte mode. >> >> Hence when a CPU reset occurs, early bootloaders don't need to be aware >> of BAR value or 4byte mode being enabled: they can still access the first >> 16MiB of the SPI NOR memory using the regular 3byte address op codes. >> >> Signed-off-by: Cyrille Pitchen > > I understand this reasoning, and that's all well and good. I'd like to > see this happen for all flash that support it, since the stateful 4-byte > modes just seem to break things a lot. But one question below. > >> --- >> >> Hi all, >> >> This patch was tested on a sama5d2 xplained board + Macronix mx25l25635e. >> >> Best regards, >> >> Cyrille >> >> drivers/mtd/spi-nor/Kconfig | 12 +++++ >> drivers/mtd/spi-nor/spi-nor.c | 105 +++++++++++++++++++++++++++++++++--------- >> include/linux/mtd/spi-nor.h | 23 ++++++--- >> 3 files changed, 113 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig >> index d42c98e1f581..7fae36fc8526 100644 >> --- a/drivers/mtd/spi-nor/Kconfig >> +++ b/drivers/mtd/spi-nor/Kconfig >> @@ -29,6 +29,18 @@ config MTD_SPI_NOR_USE_4K_SECTORS >> Please note that some tools/drivers/filesystems may not work with >> 4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum). >> >> +config MTD_SPI_NOR_USE_4B_OPCODES >> + bool "Use 4-byte address op codes" >> + default n >> + help >> + This is an alternative to the "Enter/Exit 4-byte mode" commands and >> + Base Address Register (BAR) updates to support flash size above 16MiB. >> + Using dedicated 4-byte address op codes for (Fast) Read, Page Program >> + and Sector Erase operations avoids changing the internal state of the >> + SPI NOR memory. Hence if a CPU reset occurs, early bootloaders can >> + still use regular 3-byte address op codes and read from the very first >> + 16MiB of the flash memory. >> + > > Why does this need to be a Kconfig option? Can't it just as well be > supported by keeping entries in the ID table, to show which flash > support these opcodes and which don't? Kconfig is really a bad mechanism > for trying to configure your flash. > >> config SPI_FSL_QUADSPI >> tristate "Freescale Quad SPI controller" >> depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE || COMPILE_TEST > > [snip the rest for now, since I don't think it's relevant] > > Brian > Well, the only reason why I've chosen a Kconfig option is to be as safe as possible, if for some reason someone still wants to use the former implementation. Honestly I don't know why one would do so but I'm cautious so I also set "default n". This way no regression is introduced. If you think it's better to use a dedicated flag like SECT_4K or SPI_NOR_QUAD_READ in the spi_nor_ids[] table, I'm perfectly fine with it as well. Just let me know your choice then I'll update my patch accordingly if needed. Best regards, Cyrille