linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Pratyush Yadav <pratyush@kernel.org>
To: Santhosh Kumar K <s-k6@ti.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,  <richard@nod.at>,
	<vigneshr@ti.com>,  <broonie@kernel.org>,
	 <tudor.ambarus@linaro.org>, <pratyush@kernel.org>,
	 <mwalle@kernel.org>,  <p-mantena@ti.com>,
	<linux-spi@vger.kernel.org>,  <linux-mtd@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,  <a-dutta@ti.com>,
	 <u-kumar1@ti.com>, <praneeth@ti.com>
Subject: Re: [RFC PATCH 01/10] spi: spi-mem: Introduce support for tuning controller
Date: Tue, 18 Nov 2025 14:49:13 +0100	[thread overview]
Message-ID: <mafs0ikf74fja.fsf@kernel.org> (raw)
In-Reply-To: <cb04a4ec-c643-4b80-9288-8fd8944cb4f7@ti.com> (Santhosh Kumar K.'s message of "Sat, 20 Sep 2025 23:25:31 +0530")

On Sat, Sep 20 2025, Santhosh Kumar K wrote:

[...]
>> This is actually wrong. Tuning is way more generic than that :)
>> If someone wants to use a chip at a high frequency (50MHz in your case,
>> but whatever, there is a threshold above which additional care must be
>> taken), it must go through the calibration step. It does not matter in
>> which mode you are. Calibration would still be relevant in single SDR
>> mode.
>> This 50MHz bothered Mark because it is too Cadence specific. Maybe this
>> should be a controller parameter? If the spi-mem core (or even the spi
>> core, by extensino) sees that the design allows running at XMHz (due to
>> the SPI peripheral properties or simply the absence of any limitation),
>> and if the controller states that it requires an extra tuning step above
>> YMHz (and X > Y), then it launches the calibration.
>>  From a core perspective, I would like the calibration hook to be as
>> simple as possible, because what "calibration" means is highly
>> controller and chip specific.
>
> I understand the concern here.
>
> Let me point out the options for launching the tuning procedure, along
> with the issues in each approach.
>
> Option 1: Launch tuning as part of spi_mem_exec_op()
>    - After spi_mem_access_start(), introduce a spi_mem_needs_tuning()
> check (a new callback to SPI MEM controller) to check whether the
> current op requires tuning
>    - If yes, we call spi_mem_execute_tuning()
>         - on success, mark tuning complete in a flag within SPI MEM
> Controller private data
>         - on failure, we attempt a fallback by calling
> spi_mem_adjust_op_freq() and drop to a lower supported frequency
>
> Option 2: Launch tuning within spi_controller->exec_op() implementation
>    - Very similar to option 1, except that the spi_mem_execute_tuning()
> is triggered from within the controller's exec_op() implementation
> (no need for spi_mem_needs_tuning())
>
> Drawbacks in option 1 and 2:
>    - Tuning requires multiple reads of a known pattern, but the flash
> may not always be in a state to allow read commands
>    - No fallback on failures, can't make flash-specific adjustments in
> case of a tuning failure
>    - No access to write_op() to write known pattern temporarily to an
> on-die cache. Pattern needs to be always burnt into the flash
>
>    - Plus, in option 2 - we can't call spi_mem_adjust_op_freq()
>
> While the need for tuning is dictated by Controller specific
> characteristics the ops (and state of the chip) required to complete
> tuning is under the control of spi-mem users (spi-nand/spi-nor).
> So, it's impossible to achieve tuning without the help of spi-mem users.
>
> So, Option 3: Launch from SPI MEM clients
>                           (mtd/nand/spi or mtd/spi-nor, etc.,)
>    - Once the spi-mem chip is completely enumerated and best read and
> write ops are chosen call spi_mem_needs_tuning(read_op, write_op) as
> a part of .probe()
>    - If tuning is required, call
> spi_mem_execute_tuning(read_op, write_op)
>         - If only read_op is provided, it implies the tuning pattern is
> pre-flashed to the partition
>    - On tuning failure, retry by re-running spi_mem_needs_tuning() with
> the second best set of ops (max throughput - 1)
>
> With option 3, spi_mem users are limited to calling
> spi_mem_needs_tuning() and spi_mem_execute_tuning(). Rest is hidden
> within the controller drivers. If spi-mem users change read/write ops,
> the above sequence can be re-issued.
>
> The controller can store the read_op and write_op in case of a tuning
> success and periodically re-run tuning, ensuring we always have valid
> tuning parameters.
>
> One concern with option 3 is that we may not be able to make use of
> static data on certain flash as tuning patterns (like reading parameter
> page or SFDP table for tuning instead of controller specific attack
> patterns).

Why not? How else would tuning work? Do you expect controllers to first
flash the tuning pattern and then tune the reads? That is a hard no I
think, since you don't want to over-write user data and I don't think we
will ever have any area of memory we can reliably over-write without
risking that.

I think we should start with the requirement to have the pattern flashed
already and figure out how SPI NOR or SPI NAND can discover that
(perhaps via NVMEM?).

I think SFDP is quite nice for this, but IIRC for spi-candence-quadspi,
that was not a viable option due to some reasons. If you can now make it
work with SFDP, then that would be even better, since we don't have to
deal with the pain of pre-flashing.

Overall, I think option 3 is the most promising. Options 1 and 2 will
likely add so much overhead they will end up being slower than non-PHY
reads, since tuning is usually quite expensive.

>
> Please let me know your thoughts on which of these directions makes the
> most sense.
>
> Thanks,
> Santhosh.
>

-- 
Regards,
Pratyush Yadav

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

  parent reply	other threads:[~2025-11-18 13:49 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11 19:32 [RFC PATCH 00/10] SPINAND PHY Tuning Series Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 01/10] spi: spi-mem: Introduce support for tuning controller Santhosh Kumar K
2025-08-13 20:26   ` Mark Brown
2025-08-14 11:34     ` Santhosh Kumar K
2025-08-14 12:34       ` Mark Brown
2025-08-22  6:05         ` Santhosh Kumar K
2025-09-10  8:07         ` Miquel Raynal
2025-08-24 17:02     ` Miquel Raynal
2025-09-10  8:21   ` Miquel Raynal
2025-09-20 17:55     ` Santhosh Kumar K
2025-10-28 15:41       ` Miquel Raynal
2025-11-05  8:55         ` Santhosh Kumar K
2025-11-05  9:35           ` Miquel Raynal
2025-11-18 13:42             ` Pratyush Yadav
2025-12-03  8:02               ` Santhosh Kumar K
2025-12-03  8:58                 ` Miquel Raynal
2025-12-10 11:33                   ` Santhosh Kumar K
2025-12-12  6:43                   ` Pratyush Yadav
2025-11-18 13:49       ` Pratyush Yadav [this message]
2025-12-03  8:02         ` Santhosh Kumar K
2025-12-03  9:28           ` Michael Walle
2025-12-03  9:50             ` Miquel Raynal
2025-12-03 14:12               ` Michael Walle
2025-12-10 11:36                 ` Santhosh Kumar K
2025-12-10 11:34               ` Santhosh Kumar K
2025-12-11 14:16                 ` Miquel Raynal
2025-12-04 16:54           ` Mahapatra, Amit Kumar
2025-12-10 11:34             ` Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 02/10] spi: spi-mem: Define spi_mem_tuning_params and spi_mem_get_tuning_params() Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 03/10] mtd: nand: spi: Introduce _execute_tuning for mtd devices Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 04/10] mtd: mtdcore: Call mtd_execute_tuning during mtd_register Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 05/10] spi: cadence-quadspi: Move cqspi_readdata_capture() above all operations Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 06/10] spi: cadence-quadspi: Use BIT() macro for CQSPI_REG_READCAPTURE_BYPASS Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 07/10] spi: cadence-quadspi: Enable PHY for aligned DAC reads Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 08/10] spi: cadence-quadspi: Enable PHY for data writes Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 09/10] spi: cadence-quadspi: Implement PHY for higher frequencies in SDR mode Santhosh Kumar K
2025-08-11 19:32 ` [RFC PATCH 10/10] spi: cadence-quadspi: Define cqspi_get_tuning_params() Santhosh Kumar K

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=mafs0ikf74fja.fsf@kernel.org \
    --to=pratyush@kernel.org \
    --cc=a-dutta@ti.com \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=mwalle@kernel.org \
    --cc=p-mantena@ti.com \
    --cc=praneeth@ti.com \
    --cc=richard@nod.at \
    --cc=s-k6@ti.com \
    --cc=tudor.ambarus@linaro.org \
    --cc=u-kumar1@ti.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).