[U-Boot] [EXT] Re: [PATCH 2/2] fsl_qspi: Improve QSPI driver to incorporate 4 byte commands
Rajat Srivastava
rajat.srivastava at nxp.com
Thu Apr 25 11:50:03 UTC 2019
> -----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.
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)) {
> >
More information about the U-Boot
mailing list