Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754040AbcDYJeV (ORCPT ); Mon, 25 Apr 2016 05:34:21 -0400 Received: from eusmtp01.atmel.com ([212.144.249.243]:55891 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753647AbcDYJeU (ORCPT ); Mon, 25 Apr 2016 05:34:20 -0400 Subject: Re: [PATCH RFC 4/8] mtd: spi-nor: fix support of Dual (x-y-2) and Quad (x-y-4) SPI protocols To: "R, Vignesh" , , References: <571C5454.5040006@ti.com> From: Cyrille Pitchen CC: , , , Message-ID: <571DE496.3000108@atmel.com> Date: Mon, 25 Apr 2016 11:34:14 +0200 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: <571C5454.5040006@ti.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: 4228 Lines: 99 Hi Vignesh, Le 24/04/2016 07:06, R, Vignesh a ?crit : > Hi Cyrille, > > On 4/13/2016 10:53 PM, Cyrille Pitchen wrote: > [...] >> @@ -1451,51 +1599,42 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode) >> if (np) { >> /* If we were instantiated by DT, use it */ >> if (of_property_read_bool(np, "m25p,fast-read")) >> - nor->flash_read = SPI_NOR_FAST; >> + modes->rd_modes |= SNOR_MODE_1_1_1; >> else >> - nor->flash_read = SPI_NOR_NORMAL; >> + modes->rd_modes &= ~SNOR_MODE_1_1_1; >> } else { >> /* If we weren't instantiated by DT, default to fast-read */ >> - nor->flash_read = SPI_NOR_FAST; >> + modes->rd_modes |= SNOR_MODE_1_1_1; >> } >> >> /* Some devices cannot do fast-read, no matter what DT tells us */ >> if (info->flags & SPI_NOR_NO_FR) >> - nor->flash_read = SPI_NOR_NORMAL; >> + modes->rd_modes &= ~SNOR_MODE_1_1_1; >> >> - /* Quad/Dual-read mode takes precedence over fast/normal */ >> - if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) { >> - ret = set_quad_mode(nor, info); >> - if (ret) { >> - dev_err(dev, "quad mode not supported\n"); > > Now that the set_quad_mode() is removed, could you explain how spansion > flash enters SNOR_MODE_1_1_4 mode? Is it bootloader's responsibility to > flash's set quad enable bit? > The proposed mechanics relies on the struct spi_nor_basic_flash_parameter. This structure provides the same kind of parameters that those we could find in JEDEC SFDP (Serial Flash Discoverable Parameters): - (Fast) Read op codes and protocols supported by the memory and the associated number of dummy cycles. More precisely the number of mode clock and wait state cycle. - Page Program op code and protocols. - Sector/Block Erase op codes and the associated erase size. - The "Quad Enable Requirements (QER)" which tells us "whether the device contains a Quad Enable (QE) bit used to enable 1-1-4 and 1-4-4 quad read or quad program operations" as found in the JESD216B specification and implemented here through the optional .enable_quad_io() hook. - The "4-4-4 mode enable and disable sequences", implemented by the optional .enable_4_4_4() hook. - The optional .enable_2_2_2() hook: the JESD216B specification misses to describe this procedure, only used by Micron AFAIK. For this RFC series, I've only provided a patch (patch 6) for Micron as an example of the new way to support QSPI memories. I've already written another patch for the support of Macronix memories but I didn't publish it yet. I was waiting for comments on the series to know whether I'm on the good way before spending much time on writing support to other manufacturers. For Spansion, the .enable_4_4_4() and .enable_2_2_2() would not be implemented as Spansion QSPI memories only supports 1-1-2, 1-2-2, 1-1-4 and 1-4-4 protocols. Hence only the .enable_quad_io() hook is needed. This hook should be implemented using something very closed to the spansion_quad_enable() function. So at one side the struct spi_nor_basic_flash_parameter describes the SPI memory hardware capabilities and SPI protocols it supports. On the other side, the struct spi_nor_modes allows the caller of spi_nor_scan() to describe the hardware capabilities of the SPI controller; which SPI protocols the controller supports and which SPI protocols the user wants to allow or disable. As an example, if a QSPI memory has already entered its 4-4-4 mode, you can still probe the memory from Linux by reading the JEDEC ID as long as you set the SNOR_MODE_4_4_4 bit in modes.id_modes. Then, if you don't set the SNOR_MODE_4_4_4 bit in modes.rd_modes but only the SNOR_MODE_1_1_4 bit, spi_nor_scan() first makes the SPI exit its 4-4-4 mode then select the SPI 1-1-4 (if supported by the memory) for Fast Read operations. You can do so if for some reason you'd rather use the SPI 1-1-4 protocol instead of the 4-4-4. So it's now up to the caller of spi_nor_scan() to tell the framework what protocols it supports and what protocols it wants to use. The spi-nor framework will select the best match between the memory and controller hardware capabilities. Anyway, thanks for the review! :) > Regards > Vignesh > Best regards, Cyrille