[U-Boot] [PATCH 1/2] spi: cadence_qspi: Move to spi-mem framework

[Tudor.Ambarus at microchip.com]

Hi, Simon, Vignesh,

On 10/17/2019 02:20 PM, Simon Goldschmidt wrote:
> On Mon, Oct 14, 2019 at 3:27 PM Vignesh Raghavendra <vigneshr at ti.com> 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 */