| Message ID | 1414071415-16311-1-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
On 23 October 2014 07:36, Bin Meng <bmeng.cn@gmail.com> wrote: > Currently if SST flash advertises SST_WP flag in the params table > the word program command (ADh) with auto address increment will be > used for the flash write op. However some SPI controllers do not > support the word program command (like the Intel ICH 7), the byte > programm command (02h) has to be used. > > A new TX operation mode SPI_OPM_TX_BP is introduced for such SPI > controller to use byte program op for SST flash. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > drivers/mtd/spi/sf_internal.h | 2 ++ > drivers/mtd/spi/sf_ops.c | 31 +++++++++++++++++++++++++++++++ > drivers/mtd/spi/sf_probe.c | 8 ++++++-- > drivers/spi/ich.c | 9 +++++++-- > include/spi.h | 1 + > 5 files changed, 47 insertions(+), 4 deletions(-) Acked-by: Simon Glass <sjg@chromium.org> Tested on link (which is however ich9) Tested-by: Simon Glass <sjg@chromium.org> There are a few conflicts with the driver model SPI series that just landed. I'll push this to u-boot-x86/testing for now so that Jagan can take a look. But probably this should come through the x86 tree. Regards, Simon
On 23 October 2014 19:06, Bin Meng <bmeng.cn@gmail.com> wrote: > Currently if SST flash advertises SST_WP flag in the params table > the word program command (ADh) with auto address increment will be > used for the flash write op. However some SPI controllers do not > support the word program command (like the Intel ICH 7), the byte > programm command (02h) has to be used. > > A new TX operation mode SPI_OPM_TX_BP is introduced for such SPI > controller to use byte program op for SST flash. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > drivers/mtd/spi/sf_internal.h | 2 ++ > drivers/mtd/spi/sf_ops.c | 31 +++++++++++++++++++++++++++++++ > drivers/mtd/spi/sf_probe.c | 8 ++++++-- > drivers/spi/ich.c | 9 +++++++-- > include/spi.h | 1 + > 5 files changed, 47 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h > index 19d4914..c185e04 100644 > --- a/drivers/mtd/spi/sf_internal.h > +++ b/drivers/mtd/spi/sf_internal.h > @@ -77,6 +77,8 @@ > > int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len, > const void *buf); > +int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, > + const void *buf); > #endif I believe byte write is already been used within the sst_write_wp - please check. > > /* Send a single-byte command to the device and read the response */ > diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c > index 85cf22d..3703acb 100644 > --- a/drivers/mtd/spi/sf_ops.c > +++ b/drivers/mtd/spi/sf_ops.c > @@ -516,4 +516,35 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len, > spi_release_bus(flash->spi); > return ret; > } > + > +int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, > + const void *buf) This function is simply calling existing sst_byte_write(). > +{ > + size_t actual; > + int ret; > + > + ret = spi_claim_bus(flash->spi); > + if (ret) { > + debug("SF: Unable to claim SPI bus\n"); > + return ret; > + } > + > + for (actual = 0; actual < len; actual++) { > + ret = sst_byte_write(flash, offset, buf + actual); > + if (ret) { > + debug("SF: sst byte program failed\n"); > + break; > + } > + offset++; > + } > + > + if (!ret) > + ret = spi_flash_cmd_write_disable(flash); > + > + debug("SF: sst: program %s %zu bytes @ 0x%zx\n", > + ret ? "failure" : "success", len, offset - actual); > + > + spi_release_bus(flash->spi); > + return ret; > +} > #endif > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c > index 4d148d1..1b48955 100644 > --- a/drivers/mtd/spi/sf_probe.c > +++ b/drivers/mtd/spi/sf_probe.c > @@ -138,8 +138,12 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi, > /* Assign spi_flash ops */ > flash->write = spi_flash_cmd_write_ops; > #ifdef CONFIG_SPI_FLASH_SST > - if (params->flags & SST_WP) > - flash->write = sst_write_wp; > + if (params->flags & SST_WP) { > + if (flash->spi->op_mode_tx & SPI_OPM_TX_BP) > + flash->write = sst_write_bp; > + else > + flash->write = sst_write_wp; > + } > #endif > flash->erase = spi_flash_cmd_erase_ops; > flash->read = spi_flash_cmd_read_ops; > diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c > index b356411..16730ec 100644 > --- a/drivers/spi/ich.c > +++ b/drivers/spi/ich.c > @@ -141,9 +141,14 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > ich->slave.max_write_size = ctlr.databytes; > ich->speed = max_hz; > > - /* ICH 7 SPI controller only supports array read command */ > - if (ctlr.ich_version == 7) > + /* > + * ICH 7 SPI controller only supports array read command > + * and byte program command for SST flash > + */ > + if (ctlr.ich_version == 7) { > ich->slave.op_mode_rx = SPI_OPM_RX_AS; > + ich->slave.op_mode_tx = SPI_OPM_TX_BP; > + } SST commands(write) is different than normal flash commands, I don't like to change the controller driver to know the flash command usage. May be this part can think later, try to see above comments. > > return &ich->slave; > } > diff --git a/include/spi.h b/include/spi.h > index ffd6647..a4d3f5f 100644 > --- a/include/spi.h > +++ b/include/spi.h > @@ -34,6 +34,7 @@ > > /* SPI TX operation modes */ > #define SPI_OPM_TX_QPP 1 << 0 > +#define SPI_OPM_TX_BP 1 << 1 > > /* SPI RX operation modes */ > #define SPI_OPM_RX_AS 1 << 0 > -- > 1.8.2.1 > thanks!
Hi Jegan, On Tue, Oct 28, 2014 at 2:00 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > I believe byte write is already been used within the sst_write_wp - > please check. > Yes, byte write is used within sst_write_wp, however that does not mean sst_write_wp is doing the correct thing for byte program (02h). The sst_write_wp() is using the AAI command (ADh) which might not be supported by every spi host controller. >> /* Send a single-byte command to the device and read the response */ >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c >> index 85cf22d..3703acb 100644 >> --- a/drivers/mtd/spi/sf_ops.c >> +++ b/drivers/mtd/spi/sf_ops.c >> @@ -516,4 +516,35 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len, >> spi_release_bus(flash->spi); >> return ret; >> } >> + >> +int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, >> + const void *buf) > > This function is simply calling existing sst_byte_write(). Yes, but with a loop counter of buffer length. > SST commands(write) is different than normal flash commands, I don't like > to change the controller driver to know the flash command usage. > > May be this part can think later, try to see above comments. Yes, SST is different. But since there is already a spi->op_mode_tx field we can make use of it to do similar thing like spi->op_mode_rx (determining which read command to use). Again this might be revisited in the future if this is not that clean. Regards, Bin
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index 19d4914..c185e04 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -77,6 +77,8 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len, const void *buf); +int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, + const void *buf); #endif /* Send a single-byte command to the device and read the response */ diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index 85cf22d..3703acb 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -516,4 +516,35 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len, spi_release_bus(flash->spi); return ret; } + +int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, + const void *buf) +{ + size_t actual; + int ret; + + ret = spi_claim_bus(flash->spi); + if (ret) { + debug("SF: Unable to claim SPI bus\n"); + return ret; + } + + for (actual = 0; actual < len; actual++) { + ret = sst_byte_write(flash, offset, buf + actual); + if (ret) { + debug("SF: sst byte program failed\n"); + break; + } + offset++; + } + + if (!ret) + ret = spi_flash_cmd_write_disable(flash); + + debug("SF: sst: program %s %zu bytes @ 0x%zx\n", + ret ? "failure" : "success", len, offset - actual); + + spi_release_bus(flash->spi); + return ret; +} #endif diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index 4d148d1..1b48955 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -138,8 +138,12 @@ static struct spi_flash *spi_flash_validate_params(struct spi_slave *spi, /* Assign spi_flash ops */ flash->write = spi_flash_cmd_write_ops; #ifdef CONFIG_SPI_FLASH_SST - if (params->flags & SST_WP) - flash->write = sst_write_wp; + if (params->flags & SST_WP) { + if (flash->spi->op_mode_tx & SPI_OPM_TX_BP) + flash->write = sst_write_bp; + else + flash->write = sst_write_wp; + } #endif flash->erase = spi_flash_cmd_erase_ops; flash->read = spi_flash_cmd_read_ops; diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c index b356411..16730ec 100644 --- a/drivers/spi/ich.c +++ b/drivers/spi/ich.c @@ -141,9 +141,14 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, ich->slave.max_write_size = ctlr.databytes; ich->speed = max_hz; - /* ICH 7 SPI controller only supports array read command */ - if (ctlr.ich_version == 7) + /* + * ICH 7 SPI controller only supports array read command + * and byte program command for SST flash + */ + if (ctlr.ich_version == 7) { ich->slave.op_mode_rx = SPI_OPM_RX_AS; + ich->slave.op_mode_tx = SPI_OPM_TX_BP; + } return &ich->slave; } diff --git a/include/spi.h b/include/spi.h index ffd6647..a4d3f5f 100644 --- a/include/spi.h +++ b/include/spi.h @@ -34,6 +34,7 @@ /* SPI TX operation modes */ #define SPI_OPM_TX_QPP 1 << 0 +#define SPI_OPM_TX_BP 1 << 1 /* SPI RX operation modes */ #define SPI_OPM_RX_AS 1 << 0
Currently if SST flash advertises SST_WP flag in the params table the word program command (ADh) with auto address increment will be used for the flash write op. However some SPI controllers do not support the word program command (like the Intel ICH 7), the byte programm command (02h) has to be used. A new TX operation mode SPI_OPM_TX_BP is introduced for such SPI controller to use byte program op for SST flash. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- drivers/mtd/spi/sf_internal.h | 2 ++ drivers/mtd/spi/sf_ops.c | 31 +++++++++++++++++++++++++++++++ drivers/mtd/spi/sf_probe.c | 8 ++++++-- drivers/spi/ich.c | 9 +++++++-- include/spi.h | 1 + 5 files changed, 47 insertions(+), 4 deletions(-)