Re: [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op()

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Changhuang Liang <changhuang.liang@starfivetech.com>
Cc: Pratyush Yadav <pratyush@kernel.org>,
	 Michael Walle <mwalle@kernel.org>,
	 Takahiro Kuwano <takahiro.kuwano@infineon.com>,
	Mark Brown <broonie@kernel.org>,
	 Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	 linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	 linux-mtd@lists.infradead.org
Subject: Re: [PATCH v1 2/4] mtd: spi-nor: Support mixed 1S-xD-xD DTR protocols in spi_nor_spimem_setup_op()
Date: Thu, 22 Oct 2026 12:45:03 +0200	[thread overview]
Message-ID: <875wyuqaj4.fsf@bootlin.com> (raw)
In-Reply-To: <20261022102614.75696-3-changhuang.liang@starfivetech.com> (Changhuang Liang's message of "Thu, 22 Oct 2026 03:26:12 -0700")

On 22/10/2026 at 03:26:12 -07, Changhuang Liang <changhuang.liang@starfivetech.com> wrote:

> Currently spi_nor_spimem_setup_op() sets all four phases (cmd, addr,
> dummy, data) to DTR whenever spi_nor_protocol_is_dtr(proto) is true.
> This is too coarse: the 1S-xD-xD DTR protocols (1_1_1_DTR, 1_2_2_DTR,
> 1_4_4_DTR, 1_8_8_DTR) send the command phase in STR on a single line,
> and only the address, dummy and data phases in DTR. SPIMEM supports
> such mixed DTR modes.
>
> Split the handling:
>
>   - For SNOR_PROTO_8_8_8_DTR, keep the existing behavior: set all
>     phases to DTR and use a 2-byte opcode (opcode + extension).
>
>   - For the other DTR protocols, set the command phase to STR and only
>     the address, dummy and data phases to DTR. Double op->dummy.nbytes
>     since DTR transfers 2 bytes per clock cycle, matching how the dummy
>     cycle count is specified for these protocols.
>
> No in-tree flash currently declares SNOR_HWCAPS_READ_1_x_x_DTR, so
> this is not a bug fix but preparation for enabling these protocols.
> A follow-up patch will enable 1S-4D-4D for the MX66L1G45G. The
> 8D-8D-8D path is unchanged.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  drivers/mtd/spi-nor/core.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 90524c58602e..43e506e728ab 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -97,13 +97,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>  	if (op->data.nbytes)
>  		op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
>  
> -	if (spi_nor_protocol_is_dtr(proto)) {
> -		/*
> -		 * SPIMEM supports mixed DTR modes, but right now we can only
> -		 * have all phases either DTR or STR. IOW, SPIMEM can have
> -		 * something like 4S-4D-4D, but SPI NOR can't. So, set all 4
> -		 * phases to either DTR or STR.
> -		 */
> +	if (proto == SNOR_PROTO_8_8_8_DTR) {
>  		op->cmd.dtr = true;
>  		op->addr.dtr = true;
>  		op->dummy.dtr = true;
> @@ -115,6 +109,19 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
>  		ext = spi_nor_get_cmd_ext(nor, op);
>  		op->cmd.opcode = (op->cmd.opcode << 8) | ext;
>  		op->cmd.nbytes = 2;
> +	} else if (spi_nor_protocol_is_dtr(proto)) {
> +		/*
> +		 * 1S-xD-xD protocols: the opcode is sent in STR on a single
> +		 * line, only the address, dummy and data phases are DTR. The
> +		 * opcode stays 1 byte (no command extension).
> +		 */
> +		op->cmd.dtr = false;
> +		op->addr.dtr = true;
> +		op->dummy.dtr = true;
> +		op->data.dtr = true;

Still to coarse IMHO.

What about xS-xS-xD?

As mentioned in the spi controller patch you must make sure these are
refused if they are not supported by your controller because SPI NAND at
least allow them (and I believe SPI NOR might as well)?.

> +
> +		/* 2 bytes per clock cycle in DTR mode. */
> +		op->dummy.nbytes *= 2;
>  	}
>  
>  	if (proto == SNOR_PROTO_8_8_8_DTR && params->flags & SNOR_F_SWAP16)

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