| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <84113017-8ba8-4de3-24d8-719c63f458fe@foss.st.com>
Date: Tue, 18 May 2021 13:50:13 +0200
From: Patrice CHOTARD <patrice.chotard@...s.st.com>
To: Boris Brezillon <boris.brezillon@...labora.com>
CC: Mark Brown <broonie@...nel.org>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Vignesh Raghavendra <vigneshr@...com>,
<linux-mtd@...ts.infradead.org>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
<linux-spi@...r.kernel.org>,
<linux-stm32@...md-mailman.stormreply.com>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <christophe.kerello@...s.st.com>
Subject: Re: [PATCH v3 2/3] mtd: spinand: use the spi-mem poll status APIs
Hi
On 5/18/21 1:28 PM, Boris Brezillon wrote:
> On Tue, 18 May 2021 11:39:50 +0200
> <patrice.chotard@...s.st.com> wrote:
>
>> From: Patrice Chotard <patrice.chotard@...s.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 you were reporting for those
> that don't have a dedicated STATUS poll block logic, which is great!
I will update the commit message by indicating what you mention here.
>
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard@...s.st.com>
>> Signed-off-by: Christophe Kerello <christophe.kerello@...s.st.com>
>> ---
>> 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 | 11 ++++++++-
>> 2 files changed, 42 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
>> index 17f63f95f4a2..ef2a692ab5b6 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_STATUS_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..180c1fa64e62 100644
>> --- a/include/linux/mtd/spinand.h
>> +++ b/include/linux/mtd/spinand.h
>> @@ -169,7 +169,16 @@
>> struct spinand_op;
>> struct spinand_device;
>>
>> -#define SPINAND_MAX_ID_LEN 4
>> +#define SPINAND_MAX_ID_LEN 4
>> +#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
>
> Could you add a comment explaining where those numbers come from?
Sure
>
>> +#define SPINAND_STATUS_TIMEOUT_MS 400
>
> I would name that one SPINAND_WAITRDY_TIMEOUT_MS.
Ok
>
>>
>> /**
>> * struct spinand_id - SPI NAND id structure
>
Thanks
Patrice
Powered by blists - more mailing lists