[U-Boot] [PATCH 2/2] fsl_qspi: Improve QSPI driver to incorporate 4 byte commands
Vignesh Raghavendra
vigneshr at ti.com
Fri Apr 26 04:58:45 UTC 2019
On 25/04/19 5:20 PM, Rajat Srivastava wrote:
>
>
>> -----Original Message-----
>> From: Vignesh Raghavendra <vigneshr at ti.com>
>> Sent: Wednesday, April 24, 2019 10:17 PM
>> To: Rajat Srivastava <rajat.srivastava at nxp.com>; u-boot at lists.denx.de;
>> jagan at openedev.com
>> Cc: Ashish Kumar <ashish.kumar at nxp.com>
>> Subject: [EXT] Re: [PATCH 2/2] fsl_qspi: Improve QSPI driver to incorporate 4
>> byte commands
>>
>> Caution: EXT Email
>>
>> On 24-Apr-19 6:10 PM, Rajat Srivastava wrote:
>>> Signed-off-by: Ashish Kumar <ashish.kumar at nxp.com>
>>> Signed-off-by: Rajat Srivastava <rajat.srivastava at nxp.com>
>>
>> Commit message is missing.
>
> I'll add proper commit message in the next patch version.
>
>> But from $patch subject, I infer that $patch is
>> adding new feature and not actually fixing something broken?
>
> Earlier the framework was designed to work for only 3-byte opcodes but our
> controller supports flashes of size greater than 16 MB. As a workaround,
> FSL QSPI driver used to explicitly send 4-byte opcodes for 3-byte opcodes sent by
> framework to the flash. Also there used to exist a temporary patch for framework
> which never got accepted In upstream.
> Now the new framework supports 4-byte opcodes and FSL QSPI driver needs
> correction. I am not introducing any new feature. I am just fixing the driver
> to suit the current framework.
>
With SPL_FLASH_BAR set fsl_qspi driver should never see 4 byte opcodes
and should work like it did before. I guess you are disabling SPI_FLASH_BAR?
Please add all details to the commit message and I recommend you to move
the driver over to spi-mem as soon as possible. Else you would have to
keep handling new opcodes in your driver as and when spi-nor-core changes.
Regards
Vignesh
> Please let me know your feedback.
>
> Regards
> Rajat
>
>> If so, you should move the driver over to use spi-mem APIs instead of adding
>> more features and hard coding more flash specific commands in the driver.
>> This makes driver duplicate more of spi-nor core code. I discourage adding
>> new features w/o moving driver over to spi-mem. IMHO, converting the
>> driver would not be a huge effort. And I believe its already done in kernel.
>>
>> Regards
>> Vignesh
>>
>>> ---
>>> drivers/spi/fsl_qspi.c | 38 +++++++++++++++++++++++++-------------
>>> 1 file changed, 25 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c index
>>> 1598c4f698..1d26c6344b 100644
>>> --- a/drivers/spi/fsl_qspi.c
>>> +++ b/drivers/spi/fsl_qspi.c
>>> @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>> #define TX_BUFFER_SIZE 0x40
>>> #endif
>>>
>>> -#define OFFSET_BITS_MASK GENMASK(23, 0)
>>> +#define OFFSET_BITS_MASK GENMASK(27, 0)
>>>
>>> #define FLASH_STATUS_WEL 0x02
>>>
>>> @@ -754,7 +754,8 @@ static void qspi_op_erase(struct fsl_qspi_priv *priv)
>>> while (qspi_read32(priv->flags, ®s->sr) & QSPI_SR_BUSY_MASK)
>>> ;
>>>
>>> - if (priv->cur_seqid == QSPI_CMD_SE) {
>>> + if ((priv->cur_seqid == QSPI_CMD_SE_4B) ||
>>> + (priv->cur_seqid == QSPI_CMD_SE)) {
>>> qspi_write32(priv->flags, ®s->ipcr,
>>> (SEQID_SE << QSPI_IPCR_SEQID_SHIFT) | 0);
>>> } else if (priv->cur_seqid == QSPI_CMD_BE_4K) { @@ -775,31
>>> +776,40 @@ int qspi_xfer(struct fsl_qspi_priv *priv, unsigned int bitlen,
>>> u32 txbuf;
>>>
>>> WATCHDOG_RESET();
>>> -
>>> if (dout) {
>>> if (flags & SPI_XFER_BEGIN) {
>>> priv->cur_seqid = *(u8 *)dout;
>>> - memcpy(&txbuf, dout, 4);
>>> + if (FSL_QSPI_FLASH_SIZE > SZ_16M && bytes > 4)
>>> + memcpy(&txbuf, dout + 1, 4);
>>> + else
>>> + memcpy(&txbuf, dout, 4);
>>> }
>>>
>>> if (flags == SPI_XFER_END) {
>>> priv->sf_addr = wr_sfaddr;
>>> - qspi_op_write(priv, (u8 *)dout, bytes);
>>> - return 0;
>>> + if (priv->cur_seqid == QSPI_CMD_PP ||
>>> + priv->cur_seqid == QSPI_CMD_PP_4B ||
>>> + priv->cur_seqid == QSPI_CMD_WRAR) {
>>> + qspi_op_write(priv, (u8 *)dout, bytes);
>>> + return 0;
>>> + }
>>> }
>>>
>>> - if (priv->cur_seqid == QSPI_CMD_FAST_READ ||
>>> - priv->cur_seqid == QSPI_CMD_RDAR) {
>>> + if ((priv->cur_seqid == QSPI_CMD_FAST_READ) ||
>>> + (priv->cur_seqid == QSPI_CMD_FAST_READ_4B) ||
>>> + (priv->cur_seqid == QSPI_CMD_RDAR)) {
>>> priv->sf_addr = swab32(txbuf) & OFFSET_BITS_MASK;
>>> } else if ((priv->cur_seqid == QSPI_CMD_SE) ||
>>> - (priv->cur_seqid == QSPI_CMD_BE_4K)) {
>>> + priv->cur_seqid == QSPI_CMD_SE_4B ||
>>> + priv->cur_seqid == QSPI_CMD_BE_4K) {
>>> priv->sf_addr = swab32(txbuf) & OFFSET_BITS_MASK;
>>> qspi_op_erase(priv);
>>> } else if (priv->cur_seqid == QSPI_CMD_PP ||
>>> + priv->cur_seqid == QSPI_CMD_PP_4B ||
>>> priv->cur_seqid == QSPI_CMD_WRAR) {
>>> wr_sfaddr = swab32(txbuf) & OFFSET_BITS_MASK;
>>> } else if ((priv->cur_seqid == QSPI_CMD_BRWR) ||
>>> - (priv->cur_seqid == QSPI_CMD_WREAR)) {
>>> + (priv->cur_seqid == QSPI_CMD_WREAR)) {
>>> #ifdef CONFIG_SPI_FLASH_BAR
>>> wr_sfaddr = 0;
>>> #endif
>>> @@ -807,7 +817,8 @@ int qspi_xfer(struct fsl_qspi_priv *priv, unsigned int
>> bitlen,
>>> }
>>>
>>> if (din) {
>>> - if (priv->cur_seqid == QSPI_CMD_FAST_READ) {
>>> + if ((priv->cur_seqid == QSPI_CMD_FAST_READ) ||
>>> + (priv->cur_seqid == QSPI_CMD_FAST_READ_4B)) {
>>> #ifdef CONFIG_SYS_FSL_QSPI_AHB
>>> qspi_ahb_read(priv, din, bytes); #else @@
>>> -815,10 +826,11 @@ int qspi_xfer(struct fsl_qspi_priv *priv, unsigned
>>> int bitlen, #endif
>>> } else if (priv->cur_seqid == QSPI_CMD_RDAR) {
>>> qspi_op_read(priv, din, bytes);
>>> - } else if (priv->cur_seqid == QSPI_CMD_RDID)
>>> + } else if (priv->cur_seqid == QSPI_CMD_RDID) {
>>> qspi_op_rdid(priv, din, bytes);
>>> - else if (priv->cur_seqid == QSPI_CMD_RDSR)
>>> + } else if (priv->cur_seqid == QSPI_CMD_RDSR) {
>>> qspi_op_rdsr(priv, din, bytes);
>>> + }
>>> #ifdef CONFIG_SPI_FLASH_BAR
>>> else if ((priv->cur_seqid == QSPI_CMD_BRRD) ||
>>> (priv->cur_seqid == QSPI_CMD_RDEAR)) {
>>>
--
Regards
Vignesh
More information about the U-Boot
mailing list