From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vignesh R Date: Mon, 28 Jan 2019 17:13:41 +0530 Subject: [U-Boot] [PATCH v2 04/11] mtd: spi: Port SPI NOR framework from Linux In-Reply-To: References: <20181221063836.11429-1-vigneshr@ti.com> <20181221063836.11429-5-vigneshr@ti.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 28/01/19 12:18 AM, Jagan Teki wrote: > Do you have this whole series in some branch in github? I'm unable to > apply it on master. > Here is my tentative v3 branch[1] based on top of today's master. I haven't got to splitting up this patch yet. But have addressed all other comments by you and Simon on this version. [1] https://github.com/r-vignesh/u-boot.git branch: spi-nor-mig-patch-v3 Let me know if there are any additional comments. Thanks for the review! > On Fri, Dec 21, 2018 at 12:15 PM Vignesh R wrote: >> >> Current U-Boot SPI NOR support (sf layer) is quite outdated as it does not >> support 4 byte addressing opcodes, SFDP table parsing and different types of >> quad mode enable sequences. Many newer flashes no longer support BANK >> registers used by sf layer to a access >16MB space. >> Also, many SPI controllers have special MMIO interfaces which provide >> accelerated read/write access but require knowledge of flash parameters >> to make use of it. Recent spi-mem layer provides a way to support such >> flashes but sf layer isn't using that. >> So sync SPI NOR framework from Linux v4.19 and add spi-mem support on top. >> in order to gain 4 byte addressing support, SFDP support and a way to >> support SPI controllers with MMIO flash interface. > > Understand the usage if direct complete sync, however it's difficult > for me to review whole stuff. Can you please break into few patches > like Basic sync, SFDP and other. > Ok, Let me see how that can be done. >> >> Signed-off-by: Vignesh R >> --- >> drivers/mtd/spi/spi-nor-core.c | 2590 ++++++++++++++++++++++++++++++++ >> include/linux/mtd/cfi.h | 32 + >> include/linux/mtd/spi-nor.h | 410 +++++ >> 3 files changed, 3032 insertions(+) >> create mode 100644 drivers/mtd/spi/spi-nor-core.c >> create mode 100644 include/linux/mtd/cfi.h >> create mode 100644 include/linux/mtd/spi-nor.h >> [...] >> +static int spi_nor_sr_ready(struct spi_nor *nor) >> +{ >> + int sr = read_sr(nor); >> + >> + if (sr < 0) >> + return sr; >> + >> +#ifndef CONFIG_SPL_BUILD >> + if (nor->flags & SNOR_F_USE_CLSR && sr & (SR_E_ERR | SR_P_ERR)) { >> + if (sr & SR_E_ERR) >> + dev_dbg(nor->dev, "Erase Error occurred\n"); >> + else >> + dev_dbg(nor->dev, "Programming Error occurred\n"); >> + >> + nor->write_reg(nor, SPINOR_OP_CLSR, NULL, 0); >> + return -EIO; >> + } >> +#endif > > Does it increase SPL size? or do we always assume SPL can't access > flash that would require CLSR? > Code size increase.. But now that we have SPI_FLASH_TINY, will drop this... >> + >> + return !(sr & SR_WIP); >> +} >> + [...] >> +enum spi_nor_read_command_index { >> + SNOR_CMD_READ, >> + SNOR_CMD_READ_FAST, >> + SNOR_CMD_READ_1_1_1_DTR, >> + >> + /* Dual SPI */ >> + SNOR_CMD_READ_1_1_2, >> + SNOR_CMD_READ_1_2_2, >> + SNOR_CMD_READ_2_2_2, >> + SNOR_CMD_READ_1_2_2_DTR, >> + >> + /* Quad SPI */ >> + SNOR_CMD_READ_1_1_4, >> + SNOR_CMD_READ_1_4_4, >> + SNOR_CMD_READ_4_4_4, >> + SNOR_CMD_READ_1_4_4_DTR, >> + >> + /* Octo SPI */ >> + SNOR_CMD_READ_1_1_8, >> + SNOR_CMD_READ_1_8_8, >> + SNOR_CMD_READ_8_8_8, >> + SNOR_CMD_READ_1_8_8_DTR, >> + >> + SNOR_CMD_READ_MAX >> +}; >> + >> +enum spi_nor_pp_command_index { >> + SNOR_CMD_PP, >> + >> + /* Quad SPI */ >> + SNOR_CMD_PP_1_1_4, >> + SNOR_CMD_PP_1_4_4, >> + SNOR_CMD_PP_4_4_4, >> + >> + /* Octo SPI */ >> + SNOR_CMD_PP_1_1_8, >> + SNOR_CMD_PP_1_8_8, >> + SNOR_CMD_PP_8_8_8, >> + >> + SNOR_CMD_PP_MAX >> +}; > > I'm afraid whether we teatsed all thse combinations? or we doing for > the sake of Linux sync? > Code exists for 1_1_1, 1_1_2, 1_1_4 and 4_4_4 modes and has been tested (on par with current U-Boot SF stack). 1-2-2 and 1-4-4 should work with SFDP(haven't tested it on a real hw though). Other modes are not implemented (but exists as a result of sync up with Linux) and enums are dummy definitions with no effect on code (are there for future use). I can drop unused once if you prefer. -- Regards Vignesh