| Message ID | 20210518134332.17826-1-patrice.chotard@foss.st.com |
|---|---|
| Headers | show |
| Series | MTD: spinand: Add spi_mem_poll_status() support | expand |
On Tue, 18 May 2021 15:43:31 +0200 <patrice.chotard@foss.st.com> wrote: > From: Patrice Chotard <patrice.chotard@foss.st.com> > > Make use of spi-mem poll status APIs to let advanced controllers > optimize wait operations. > This should also fix the high CPU usage for system that don't have > a dedicated STATUS poll block logic. > > Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> > Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > Changes in v4: > - Update commit message. > - Add comment which explains how delays has been calculated. > - Rename SPINAND_STATUS_TIMEOUT_MS to SPINAND_WAITRDY_TIMEOUT_MS. > > Changes in v3: > - Add initial_delay_us and polling_delay_us parameters to spinand_wait() > - Add SPINAND_READ/WRITE/ERASE/RESET_INITIAL_DELAY_US and > SPINAND_READ/WRITE/ERASE/RESET_POLL_DELAY_US defines. > > Changes in v2: > - non-offload case is now managed by spi_mem_poll_status() > > drivers/mtd/nand/spi/core.c | 45 ++++++++++++++++++++++++++----------- > include/linux/mtd/spinand.h | 22 ++++++++++++++++++ > 2 files changed, 54 insertions(+), 13 deletions(-) > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c > index 17f63f95f4a2..3131fae0c715 100644 > --- a/drivers/mtd/nand/spi/core.c > +++ b/drivers/mtd/nand/spi/core.c > @@ -473,20 +473,26 @@ static int spinand_erase_op(struct spinand_device *spinand, > return spi_mem_exec_op(spinand->spimem, &op); > } > > -static int spinand_wait(struct spinand_device *spinand, u8 *s) > +static int spinand_wait(struct spinand_device *spinand, > + unsigned long initial_delay_us, > + unsigned long poll_delay_us, > + u8 *s) > { > - unsigned long timeo = jiffies + msecs_to_jiffies(400); > + struct spi_mem_op op = SPINAND_GET_FEATURE_OP(REG_STATUS, > + spinand->scratchbuf); > u8 status; > int ret; > > - do { > - ret = spinand_read_status(spinand, &status); > - if (ret) > - return ret; > + ret = spi_mem_poll_status(spinand->spimem, &op, STATUS_BUSY, 0, > + initial_delay_us, > + poll_delay_us, > + SPINAND_WAITRDY_TIMEOUT_MS); > + if (ret) > + return ret; > > - if (!(status & STATUS_BUSY)) > - goto out; > - } while (time_before(jiffies, timeo)); > + status = *spinand->scratchbuf; > + if (!(status & STATUS_BUSY)) > + goto out; > > /* > * Extra read, just in case the STATUS_READY bit has changed > @@ -526,7 +532,10 @@ static int spinand_reset_op(struct spinand_device *spinand) > if (ret) > return ret; > > - return spinand_wait(spinand, NULL); > + return spinand_wait(spinand, > + SPINAND_RESET_INITIAL_DELAY_US, > + SPINAND_RESET_POLL_DELAY_US, > + NULL); > } > > static int spinand_lock_block(struct spinand_device *spinand, u8 lock) > @@ -549,7 +558,10 @@ static int spinand_read_page(struct spinand_device *spinand, > if (ret) > return ret; > > - ret = spinand_wait(spinand, &status); > + ret = spinand_wait(spinand, > + SPINAND_READ_INITIAL_DELAY_US, > + SPINAND_READ_POLL_DELAY_US, > + &status); > if (ret < 0) > return ret; > > @@ -585,7 +597,10 @@ static int spinand_write_page(struct spinand_device *spinand, > if (ret) > return ret; > > - ret = spinand_wait(spinand, &status); > + ret = spinand_wait(spinand, > + SPINAND_WRITE_INITIAL_DELAY_US, > + SPINAND_WRITE_POLL_DELAY_US, > + &status); > if (!ret && (status & STATUS_PROG_FAILED)) > return -EIO; > > @@ -768,7 +783,11 @@ static int spinand_erase(struct nand_device *nand, const struct nand_pos *pos) > if (ret) > return ret; > > - ret = spinand_wait(spinand, &status); > + ret = spinand_wait(spinand, > + SPINAND_ERASE_INITIAL_DELAY_US, > + SPINAND_ERASE_POLL_DELAY_US, > + &status); > + > if (!ret && (status & STATUS_ERASE_FAILED)) > ret = -EIO; > > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h > index 6bb92f26833e..6988956b8492 100644 > --- a/include/linux/mtd/spinand.h > +++ b/include/linux/mtd/spinand.h > @@ -170,6 +170,28 @@ struct spinand_op; > struct spinand_device; > > #define SPINAND_MAX_ID_LEN 4 > +/* > + * For erase, write and read operation, we got the following timings : > + * tBERS (erase) 1ms to 4ms > + * tPROG 300us to 400us > + * tREAD 25us to 100us > + * In order to minimize latency, the min value is divided by 4 for the > + * initial delay, and dividing by 20 for the poll delay. > + * For reset, 5us/10us/500us if the device is respectively > + * reading/programming/erasing when the RESET occurs. Since we always > + * issue a RESET when the device is IDLE, 5us is selected for both initial > + * and poll delay. > + */ > +#define SPINAND_READ_INITIAL_DELAY_US 6 > +#define SPINAND_READ_POLL_DELAY_US 5 > +#define SPINAND_RESET_INITIAL_DELAY_US 5 > +#define SPINAND_RESET_POLL_DELAY_US 5 > +#define SPINAND_WRITE_INITIAL_DELAY_US 75 > +#define SPINAND_WRITE_POLL_DELAY_US 15 > +#define SPINAND_ERASE_INITIAL_DELAY_US 250 > +#define SPINAND_ERASE_POLL_DELAY_US 50 > + > +#define SPINAND_WAITRDY_TIMEOUT_MS 400 > > /** > * struct spinand_id - SPI NAND id structure
On 5/18/21 4:18 PM, Boris Brezillon wrote: > On Tue, 18 May 2021 15:43:31 +0200 > <patrice.chotard@foss.st.com> wrote: > >> From: Patrice Chotard <patrice.chotard@foss.st.com> >> >> Make use of spi-mem poll status APIs to let advanced controllers >> optimize wait operations. >> This should also fix the high CPU usage for system that don't have >> a dedicated STATUS poll block logic. >> >> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> >> Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com> >> --- >> Changes in v4: >> - Update commit message. >> - Add comment which explains how delays has been calculated. >> - Rename SPINAND_STATUS_TIMEOUT_MS to SPINAND_WAITRDY_TIMEOUT_MS. >> >> Changes in v3: >> - Add initial_delay_us and polling_delay_us parameters to spinand_wait() >> - Add SPINAND_READ/WRITE/ERASE/RESET_INITIAL_DELAY_US and >> SPINAND_READ/WRITE/ERASE/RESET_POLL_DELAY_US defines. >> >> Changes in v2: >> - non-offload case is now managed by spi_mem_poll_status() >> >> drivers/mtd/nand/spi/core.c | 45 ++++++++++++++++++++++++++----------- >> include/linux/mtd/spinand.h | 22 ++++++++++++++++++ >> 2 files changed, 54 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c >> index 17f63f95f4a2..3131fae0c715 100644 >> --- a/drivers/mtd/nand/spi/core.c >> +++ b/drivers/mtd/nand/spi/core.c >> @@ -473,20 +473,26 @@ static int spinand_erase_op(struct spinand_device *spinand, >> return spi_mem_exec_op(spinand->spimem, &op); >> } >> >> -static int spinand_wait(struct spinand_device *spinand, u8 *s) >> +static int spinand_wait(struct spinand_device *spinand, >> + unsigned long initial_delay_us, >> + unsigned long poll_delay_us, >> + u8 *s) >> { >> - unsigned long timeo = jiffies + msecs_to_jiffies(400); >> + struct spi_mem_op op = SPINAND_GET_FEATURE_OP(REG_STATUS, >> + spinand->scratchbuf); >> u8 status; >> int ret; >> >> - do { >> - ret = spinand_read_status(spinand, &status); >> - if (ret) >> - return ret; >> + ret = spi_mem_poll_status(spinand->spimem, &op, STATUS_BUSY, 0, >> + initial_delay_us, >> + poll_delay_us, >> + SPINAND_WAITRDY_TIMEOUT_MS); >> + if (ret) >> + return ret; >> >> - if (!(status & STATUS_BUSY)) >> - goto out; >> - } while (time_before(jiffies, timeo)); >> + status = *spinand->scratchbuf; >> + if (!(status & STATUS_BUSY)) >> + goto out; > > Looks like you expect the driver to not only wait for a status change > but also fill the data buffer with the last status value. I think that > should be documented in the SPI mem API. Right, i will update the API. Thanks Patrice > >> /* >> * Extra read, just in case the STATUS_READY bit has changed >> @@ -526,7 +532,10 @@ static int spinand_reset_op(struct spinand_device *spinand) >> if (ret) >> return ret; >> >> - return spinand_wait(spinand, NULL); >> + return spinand_wait(spinand, >> + SPINAND_RESET_INITIAL_DELAY_US, >> + SPINAND_RESET_POLL_DELAY_US, >> + NULL); >> } >> >> static int spinand_lock_block(struct spinand_device *spinand, u8 lock) >> @@ -549,7 +558,10 @@ static int spinand_read_page(struct spinand_device *spinand, >> if (ret) >> return ret; >> >> - ret = spinand_wait(spinand, &status); >> + ret = spinand_wait(spinand, >> + SPINAND_READ_INITIAL_DELAY_US, >> + SPINAND_READ_POLL_DELAY_US, >> + &status); >> if (ret < 0) >> return ret; >> >> @@ -585,7 +597,10 @@ static int spinand_write_page(struct spinand_device *spinand, >> if (ret) >> return ret; >> >> - ret = spinand_wait(spinand, &status); >> + ret = spinand_wait(spinand, >> + SPINAND_WRITE_INITIAL_DELAY_US, >> + SPINAND_WRITE_POLL_DELAY_US, >> + &status); >> if (!ret && (status & STATUS_PROG_FAILED)) >> return -EIO; >> >> @@ -768,7 +783,11 @@ static int spinand_erase(struct nand_device *nand, const struct nand_pos *pos) >> if (ret) >> return ret; >> >> - ret = spinand_wait(spinand, &status); >> + ret = spinand_wait(spinand, >> + SPINAND_ERASE_INITIAL_DELAY_US, >> + SPINAND_ERASE_POLL_DELAY_US, >> + &status); >> + >> if (!ret && (status & STATUS_ERASE_FAILED)) >> ret = -EIO; >> >> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h >> index 6bb92f26833e..6988956b8492 100644 >> --- a/include/linux/mtd/spinand.h >> +++ b/include/linux/mtd/spinand.h >> @@ -170,6 +170,28 @@ struct spinand_op; >> struct spinand_device; >> >> #define SPINAND_MAX_ID_LEN 4 >> +/* >> + * For erase, write and read operation, we got the following timings : >> + * tBERS (erase) 1ms to 4ms >> + * tPROG 300us to 400us >> + * tREAD 25us to 100us >> + * In order to minimize latency, the min value is divided by 4 for the >> + * initial delay, and dividing by 20 for the poll delay. >> + * For reset, 5us/10us/500us if the device is respectively >> + * reading/programming/erasing when the RESET occurs. Since we always >> + * issue a RESET when the device is IDLE, 5us is selected for both initial >> + * and poll delay. >> + */ >> +#define SPINAND_READ_INITIAL_DELAY_US 6 >> +#define SPINAND_READ_POLL_DELAY_US 5 >> +#define SPINAND_RESET_INITIAL_DELAY_US 5 >> +#define SPINAND_RESET_POLL_DELAY_US 5 >> +#define SPINAND_WRITE_INITIAL_DELAY_US 75 >> +#define SPINAND_WRITE_POLL_DELAY_US 15 >> +#define SPINAND_ERASE_INITIAL_DELAY_US 250 >> +#define SPINAND_ERASE_POLL_DELAY_US 50 >> + >> +#define SPINAND_WAITRDY_TIMEOUT_MS 400 >> >> /** >> * struct spinand_id - SPI NAND id structure >
On 5/18/21 4:37 PM, Boris Brezillon wrote: > On Tue, 18 May 2021 15:43:32 +0200 > <patrice.chotard@foss.st.com> wrote: > >> +static int stm32_qspi_poll_status(struct spi_mem *mem, const struct spi_mem_op *op, >> + u16 mask, u16 match, >> + unsigned long initial_delay_us, >> + unsigned long polling_rate_us, >> + unsigned long timeout_ms) >> +{ >> + struct stm32_qspi *qspi = spi_controller_get_devdata(mem->spi->master); >> + int ret; >> + > > The spi_mem_supports_op() call is still missing. Yes, i forgot it Thanks Patrice > >> + ret = pm_runtime_get_sync(qspi->dev); >> + if (ret < 0) { >> + pm_runtime_put_noidle(qspi->dev); >> + return ret; >> + } >> + >> + mutex_lock(&qspi->lock); >> + >> + writel_relaxed(mask, qspi->io_base + QSPI_PSMKR); >> + writel_relaxed(match, qspi->io_base + QSPI_PSMAR); >> + qspi->fmode = CCR_FMODE_APM; >> + qspi->status_timeout = timeout_ms; >> + >> + ret = stm32_qspi_send(mem, op); >> + mutex_unlock(&qspi->lock); >> + >> + pm_runtime_mark_last_busy(qspi->dev); >> + pm_runtime_put_autosuspend(qspi->dev); >> + >> + return ret; >> +}
From: Patrice Chotard <patrice.chotard@foss.st.com> This series adds support for the spi_mem_poll_status() spinand interface. Some QSPI controllers allows to poll automatically memory status during operations (erase, read or write). This allows to offload the CPU for this task. STM32 QSPI is supporting this feature, driver update are also part of this series. Changes in v4: - Remove init_completion() from spi_mem_probe() added in v2. - Add missing static for spi_mem_read_status(). - Check if operation in spi_mem_poll_status() is a READ. - Update patch 2 commit message. - Add comment which explains how delays has been calculated. - Rename SPINAND_STATUS_TIMEOUT_MS to SPINAND_WAITRDY_TIMEOUT_MS. Chnages in v3: - Add spi_mem_read_status() which allows to read 8 or 16 bits status. - Add initial_delay_us and polling_delay_us parameters to spi_mem_poll_status(). and also to poll_status() callback. - Move spi_mem_supports_op() in SW-based polling case. - Add delay before invoquing read_poll_timeout(). - Remove the reinit/wait_for_completion() added in v2. - Add initial_delay_us and polling_delay_us parameters to spinand_wait(). - Add SPINAND_READ/WRITE/ERASE/RESET_INITIAL_DELAY_US and SPINAND_READ/WRITE/ERASE/RESET_POLL_DELAY_US defines. - Remove spi_mem_finalize_op() API added in v2. Changes in v2: - Indicates the spi_mem_poll_status() timeout unit - Use 2-byte wide status register - Add spi_mem_supports_op() call in spi_mem_poll_status() - Add completion management in spi_mem_poll_status() - Add offload/non-offload case management in spi_mem_poll_status() - Optimize the non-offload case by using read_poll_timeout() - mask and match stm32_qspi_poll_status()'s parameters are 2-byte wide - Make usage of new spi_mem_finalize_op() API in stm32_qspi_wait_poll_status() Patrice Chotard (3): spi: spi-mem: add automatic poll status functions mtd: spinand: use the spi-mem poll status APIs spi: stm32-qspi: add automatic poll status feature drivers/mtd/nand/spi/core.c | 45 +++++++++++++------ drivers/spi/spi-mem.c | 85 ++++++++++++++++++++++++++++++++++++ drivers/spi/spi-stm32-qspi.c | 83 +++++++++++++++++++++++++++++++---- include/linux/mtd/spinand.h | 22 ++++++++++ include/linux/spi/spi-mem.h | 14 ++++++ 5 files changed, 228 insertions(+), 21 deletions(-)