All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pratyush Yadav <pratyush@kernel.org>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Tudor Ambarus <tudor.ambarus@linaro.org>,
	 Pratyush Yadav <pratyush@kernel.org>,
	 Michael Walle <mwalle@kernel.org>,
	 Richard Weinberger <richard@nod.at>,
	 Vignesh Raghavendra <vigneshr@ti.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	 Steam Lin <STLin2@winbond.com>,
	 linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/2] mtd: spi-nor: winbond: Add support for w25q01jv
Date: Wed, 29 Jan 2025 15:39:19 +0000	[thread overview]
Message-ID: <mafs0ikpxk4t4.fsf@kernel.org> (raw)
In-Reply-To: <20250110-winbond-6-12-rc1-nor-volatile-bit-v3-1-735363f8cc7d@bootlin.com> (Miquel Raynal's message of "Fri, 10 Jan 2025 15:49:30 +0100")

On Fri, Jan 10 2025, Miquel Raynal wrote:

> Add support for Winbond w25q01jv spi-nor chip.
>
> This chip is internally made of two dies with linear addressing
> capabilities to make it transparent to the user that two dies were
> used. There is one drawback however, the read status operation is racy
> as the status bit only gives the active die status and not the status of
> the other die. For commands affecting the two dies, it means if another
> command is sent too fast after the first die has returned a valid status
> (deviation can be up to 200us), the chip will get corrupted/in an
> unstable state.
>
> This chip hence requires a better status register read. There are three
> solutions here:
>
> 1- If we assume that the most common situation producing this problem is
> status register writes, maybe we could change the "non-volatile"
> status register write commands to become "volatile" status register
> writes. In practice, what takes time is the write operation of the bits
> themselves, and not the activation of the feature in the internal
> circuitry. Enabling "volatile" status register writes would make the
> writes nearly instant.
>
> This approach, besides probably being the less impacting one, could
> overlook other possible actions where both dies can be used at the same
> time like a chip erase (or any erase over die boundaries in general).
>
> 2- Wait about 200us after getting a first status ready feedback. This
> 200us is about the maximum possible deviation between dies and would
> cover all cases.
>
> 3- We iterate manually over all internal dies (which takes about 30us
> per die) until all are ready. This approach will always be faster than
> a blind delay which represents the maximum deviation, while also being
> totally safe.
>
> This third approach has been adopted. A flash-specific hook for the
> status register read had to be implemented. Testing with the flash_speed
> benchmark shown no difference with the existing performances (using the
> regular status read core function). In practice there are difference in
> the experimental results below, but they are part of the natural
> deviation of the benchmark:
>
> 	> Without the fixup
> 	$ flash_speed /dev/mtd0 -c100 -d
> 	eraseblock write speed is 442 KiB/s
> 	eraseblock read speed is 1606 KiB/s
> 	page write speed is 439 KiB/s
> 	page read speed is 1520 KiB/s
> 	2 page write speed is 441 KiB/s
> 	2 page read speed is 1562 KiB/s
> 	erase speed is 68 KiB/s
>
> 	> With the fixup
> 	$ flash_speed /dev/mtd0 -c100 -d
> 	eraseblock write speed is 428 KiB/s
> 	eraseblock read speed is 1626 KiB/s
> 	page write speed is 426 KiB/s
> 	page read speed is 1538 KiB/s
> 	2 page write speed is 426 KiB/s
> 	2 page read speed is 1574 KiB/s
> 	erase speed is 66 KiB/s
>
> However, the fixup, whatever which one we pick, must be applied on
> multi-die chips, which hence must be properly flagged. The SFDP tables
> implemented give a lot of information but the die details are part of an
> optional table that is not implemented, hence we use a post parsing
> fixup hook to set the params->n_dice value manually.
>
> Link: https://www.winbond.com/resource-files/W25Q01JV%20SPI%20RevE%2003042024%20Plus.pdf
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: Pratyush Yadav <pratyush@kernel.org>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Tudor Ambarus <tudor.ambarus@linaro.org>,
	 Pratyush Yadav <pratyush@kernel.org>,
	 Michael Walle <mwalle@kernel.org>,
	 Richard Weinberger <richard@nod.at>,
	 Vignesh Raghavendra <vigneshr@ti.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	 Steam Lin <STLin2@winbond.com>,
	 linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/2] mtd: spi-nor: winbond: Add support for w25q01jv
Date: Wed, 29 Jan 2025 15:39:19 +0000	[thread overview]
Message-ID: <mafs0ikpxk4t4.fsf@kernel.org> (raw)
In-Reply-To: <20250110-winbond-6-12-rc1-nor-volatile-bit-v3-1-735363f8cc7d@bootlin.com> (Miquel Raynal's message of "Fri, 10 Jan 2025 15:49:30 +0100")

On Fri, Jan 10 2025, Miquel Raynal wrote:

> Add support for Winbond w25q01jv spi-nor chip.
>
> This chip is internally made of two dies with linear addressing
> capabilities to make it transparent to the user that two dies were
> used. There is one drawback however, the read status operation is racy
> as the status bit only gives the active die status and not the status of
> the other die. For commands affecting the two dies, it means if another
> command is sent too fast after the first die has returned a valid status
> (deviation can be up to 200us), the chip will get corrupted/in an
> unstable state.
>
> This chip hence requires a better status register read. There are three
> solutions here:
>
> 1- If we assume that the most common situation producing this problem is
> status register writes, maybe we could change the "non-volatile"
> status register write commands to become "volatile" status register
> writes. In practice, what takes time is the write operation of the bits
> themselves, and not the activation of the feature in the internal
> circuitry. Enabling "volatile" status register writes would make the
> writes nearly instant.
>
> This approach, besides probably being the less impacting one, could
> overlook other possible actions where both dies can be used at the same
> time like a chip erase (or any erase over die boundaries in general).
>
> 2- Wait about 200us after getting a first status ready feedback. This
> 200us is about the maximum possible deviation between dies and would
> cover all cases.
>
> 3- We iterate manually over all internal dies (which takes about 30us
> per die) until all are ready. This approach will always be faster than
> a blind delay which represents the maximum deviation, while also being
> totally safe.
>
> This third approach has been adopted. A flash-specific hook for the
> status register read had to be implemented. Testing with the flash_speed
> benchmark shown no difference with the existing performances (using the
> regular status read core function). In practice there are difference in
> the experimental results below, but they are part of the natural
> deviation of the benchmark:
>
> 	> Without the fixup
> 	$ flash_speed /dev/mtd0 -c100 -d
> 	eraseblock write speed is 442 KiB/s
> 	eraseblock read speed is 1606 KiB/s
> 	page write speed is 439 KiB/s
> 	page read speed is 1520 KiB/s
> 	2 page write speed is 441 KiB/s
> 	2 page read speed is 1562 KiB/s
> 	erase speed is 68 KiB/s
>
> 	> With the fixup
> 	$ flash_speed /dev/mtd0 -c100 -d
> 	eraseblock write speed is 428 KiB/s
> 	eraseblock read speed is 1626 KiB/s
> 	page write speed is 426 KiB/s
> 	page read speed is 1538 KiB/s
> 	2 page write speed is 426 KiB/s
> 	2 page read speed is 1574 KiB/s
> 	erase speed is 66 KiB/s
>
> However, the fixup, whatever which one we pick, must be applied on
> multi-die chips, which hence must be properly flagged. The SFDP tables
> implemented give a lot of information but the die details are part of an
> optional table that is not implemented, hence we use a post parsing
> fixup hook to set the params->n_dice value manually.
>
> Link: https://www.winbond.com/resource-files/W25Q01JV%20SPI%20RevE%2003042024%20Plus.pdf
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

-- 
Regards,
Pratyush Yadav

  reply	other threads:[~2025-01-29 15:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-10 14:49 [PATCH v3 0/2] mtd: spi-nor: winbond: Add support for flashes with several dies Miquel Raynal
2025-01-10 14:49 ` Miquel Raynal
2025-01-10 14:49 ` [PATCH v3 1/2] mtd: spi-nor: winbond: Add support for w25q01jv Miquel Raynal
2025-01-10 14:49   ` Miquel Raynal
2025-01-29 15:39   ` Pratyush Yadav [this message]
2025-01-29 15:39     ` Pratyush Yadav
2025-01-10 14:49 ` [PATCH v3 2/2] mtd: spi-nor: winbond: Add support for w25q02jv Miquel Raynal
2025-01-10 14:49   ` Miquel Raynal
2025-01-29 15:39   ` Pratyush Yadav
2025-01-29 15:39     ` Pratyush Yadav
2025-02-03 14:28 ` [PATCH v3 0/2] mtd: spi-nor: winbond: Add support for flashes with several dies Pratyush Yadav
2025-02-03 14:28   ` Pratyush Yadav

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mafs0ikpxk4t4.fsf@kernel.org \
    --to=pratyush@kernel.org \
    --cc=STLin2@winbond.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=mwalle@kernel.org \
    --cc=richard@nod.at \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tudor.ambarus@linaro.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.