From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tudor.Ambarus at microchip.com Date: Thu, 17 Oct 2019 13:48:44 +0000 Subject: [U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework In-Reply-To: References: <20191014132752.18534-1-vigneshr@ti.com> <20191014132752.18534-2-vigneshr@ti.com> Message-ID: <48d6029f-2a8b-94f3-477d-48bfe696fff0@microchip.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, Simon, Vignesh, On 10/17/2019 02:20 PM, Simon Goldschmidt wrote: > On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra wrote: >> Current Cadence QSPI driver has few limitations. It assumes all read >> operations to be in Quad mode and thus does not support SFDP parsing. >> Also, adding support for new mode such as Octal mode would not be >> possible with current configuration. Therefore move the driver over to spi-mem >> framework. This has added advantage that driver can be used to support >> SPI NAND memories too. >> Hence, move driver over to new spi-mem APIs. >> >> Please note that this gets rid of mode bit setting done when >> CONFIG_SPL_SPI_XIP is defined as there does not seem to be any user to >> that config option. > I just have tried this on an socfgpa cylone5 board with an mt25ql256a, but > it does not seem to work: when leaving spi-rx-bus-width and spi-tx-bus-width > at 4 in my devicetree, SFDP parsing works, but reading data afterwards > produces invalid results (I haven't tested what's wrong there). > > It works as expected when not parsing SFDP or setting the bus-width to 1. > So the change itself probably works, but SFDP parsing is broken? This can happen if the quad enable method is not correctly set/called. Would you try the patch form below? I don't see much benefit in having those guards, especially that we have SPI_FLASH_SFDP_SUPPORT defined - it trims most of the SFDP logic. More, these #ifdef guards are not scalable and with the addition of flashes that support SFDP the #ifdefs will look uglier and uglier. Cheers, ta diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index e5b9899c64b2..3002f97a7342 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -188,7 +188,6 @@ static int read_fsr(struct spi_nor *nor) * location. Return the configuration register value. * Returns negative if error occurred. */ -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) static int read_cr(struct spi_nor *nor) { int ret; @@ -202,7 +201,6 @@ static int read_cr(struct spi_nor *nor) return val; } -#endif /* * Write status register 1 byte @@ -591,7 +589,6 @@ erase_err: return ret; } -#if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) /* Write status register and ensure bits in mask match written values */ static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask) { @@ -877,7 +874,6 @@ static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len) return stm_is_locked_sr(nor, ofs, len, status); } -#endif /* CONFIG_SPI_FLASH_STMICRO */ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor) { @@ -1116,7 +1112,6 @@ write_err: return ret; } -#ifdef CONFIG_SPI_FLASH_MACRONIX /** * macronix_quad_enable() - set QE bit in Status Register. * @nor: pointer to a 'struct spi_nor' @@ -1153,9 +1148,7 @@ static int macronix_quad_enable(struct spi_nor *nor) return 0; } -#endif -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) /* * Write status Register and configuration register with 2 bytes * The first byte will be written to the status register, while the @@ -1269,7 +1262,6 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor) } #endif /* CONFIG_SPI_FLASH_SFDP_SUPPORT */ -#endif /* CONFIG_SPI_FLASH_SPANSION */ struct spi_nor_read_command { u8 num_mode_clocks; @@ -1787,22 +1779,16 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, case BFPT_DWORD15_QER_NONE: params->quad_enable = NULL; break; -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) case BFPT_DWORD15_QER_SR2_BIT1_BUGGY: case BFPT_DWORD15_QER_SR2_BIT1_NO_RD: params->quad_enable = spansion_no_read_cr_quad_enable; break; -#endif -#ifdef CONFIG_SPI_FLASH_MACRONIX case BFPT_DWORD15_QER_SR1_BIT6: params->quad_enable = macronix_quad_enable; break; -#endif -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) case BFPT_DWORD15_QER_SR2_BIT1: params->quad_enable = spansion_read_cr_quad_enable; break; -#endif default: return -EINVAL; } @@ -2013,20 +1999,16 @@ static int spi_nor_init_params(struct spi_nor *nor, if (params->hwcaps.mask & (SNOR_HWCAPS_READ_QUAD | SNOR_HWCAPS_PP_QUAD)) { switch (JEDEC_MFR(info)) { -#ifdef CONFIG_SPI_FLASH_MACRONIX case SNOR_MFR_MACRONIX: params->quad_enable = macronix_quad_enable; break; -#endif case SNOR_MFR_ST: case SNOR_MFR_MICRON: break; default: -#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) /* Kept only for backward compatibility purpose. */ params->quad_enable = spansion_read_cr_quad_enable; -#endif break; } } @@ -2337,7 +2319,6 @@ int spi_nor_scan(struct spi_nor *nor) mtd->_erase = spi_nor_erase; mtd->_read = spi_nor_read; -#if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) /* NOR protection support for STmicro/Micron chips and similar */ if (JEDEC_MFR(info) == SNOR_MFR_ST || JEDEC_MFR(info) == SNOR_MFR_MICRON || @@ -2347,7 +2328,6 @@ int spi_nor_scan(struct spi_nor *nor) nor->flash_unlock = stm_unlock; nor->flash_is_locked = stm_is_locked; } -#endif #ifdef CONFIG_SPI_FLASH_SST /* sst nor chips use AAI word program */