[PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Mukesh Kumar Savaliya]

There is no support for protocol drivers to specify whether a transfer
should use single or dual transfer mode. As a result, the SPI controller
has no way to determine this information from the user, leading to
potential limitations in transfer capabilities.

This change introduces a new field `dtr_mode` in the `spi_transfer`
structure. The `dtr_mode` field allows protocol drivers to indicate if
Double Transfer Rate (DTR) mode is supported for a given transfer. When
`dtr_mode` is set to true, the SPI controller will use DTR mode
otherwise, it will default to single transfer mode.

The QSPI controller driver uses this flag and configures single or double
transfer rate using the controller register.

The existing spi-mem driver helps configure the DTR mode but is limited
to memory devices. There is no support available to set DTR mode for
non-memory devices, e.g., touch or any generic SPI sensor. This change
is backward compatible and doesn't break existing SPI or QSPI drivers.

Changes include:
- Addition of `dtr_mode` field in `spi_transfer` structure.
- Documentation updates to reflect the new `dtr_mode` field.

Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
---
 include/linux/spi/spi.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 0ba5e49bace4..4e1152de82cd 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -998,6 +998,7 @@ struct spi_res {
  *	processed the word, i.e. the "pre" timestamp should be taken before
  *	transmitting the "pre" word, and the "post" timestamp after receiving
  *	transmit confirmation from the controller for the "post" word.
+ * @dtr_mode: true if supports double transfer rate.
  * @timestamped: true if the transfer has been timestamped
  * @error: Error status logged by SPI controller driver.
  *
@@ -1049,6 +1050,9 @@ struct spi_res {
  * two should both be set. User can set transfer mode with SPI_NBITS_SINGLE(1x)
  * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three transfer.
  *
+ * User may also set dtr_mode to true to use dual transfer mode if desired. if
+ * not, default considered as single transfer mode.
+ *
  * The code that submits an spi_message (and its spi_transfers)
  * to the lower layers is responsible for managing its memory.
  * Zero-initialize every field you don't set up explicitly, to
@@ -1083,6 +1087,7 @@ struct spi_transfer {
 	unsigned	tx_nbits:4;
 	unsigned	rx_nbits:4;
 	unsigned	timestamped:1;
+	bool		dtr_mode;
 #define	SPI_NBITS_SINGLE	0x01 /* 1-bit transfer */
 #define	SPI_NBITS_DUAL		0x02 /* 2-bit transfer */
 #define	SPI_NBITS_QUAD		0x04 /* 4-bit transfer */
-- 
2.25.1

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Jyothi Kumar Seerapu]

Tested-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>

On 3/26/2025 2:09 PM, Mukesh Kumar Savaliya wrote:
> There is no support for protocol drivers to specify whether a transfer
> should use single or dual transfer mode. As a result, the SPI controller
> has no way to determine this information from the user, leading to
> potential limitations in transfer capabilities.
> 
> This change introduces a new field `dtr_mode` in the `spi_transfer`
> structure. The `dtr_mode` field allows protocol drivers to indicate if
> Double Transfer Rate (DTR) mode is supported for a given transfer. When
> `dtr_mode` is set to true, the SPI controller will use DTR mode
> otherwise, it will default to single transfer mode.
> 
> The QSPI controller driver uses this flag and configures single or double
> transfer rate using the controller register.
> 
> The existing spi-mem driver helps configure the DTR mode but is limited
> to memory devices. There is no support available to set DTR mode for
> non-memory devices, e.g., touch or any generic SPI sensor. This change
> is backward compatible and doesn't break existing SPI or QSPI drivers.
> 
> Changes include:
> - Addition of `dtr_mode` field in `spi_transfer` structure.
> - Documentation updates to reflect the new `dtr_mode` field.
> 
> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
> ---
>   include/linux/spi/spi.h | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 0ba5e49bace4..4e1152de82cd 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -998,6 +998,7 @@ struct spi_res {
>    *	processed the word, i.e. the "pre" timestamp should be taken before
>    *	transmitting the "pre" word, and the "post" timestamp after receiving
>    *	transmit confirmation from the controller for the "post" word.
> + * @dtr_mode: true if supports double transfer rate.
>    * @timestamped: true if the transfer has been timestamped
>    * @error: Error status logged by SPI controller driver.
>    *
> @@ -1049,6 +1050,9 @@ struct spi_res {
>    * two should both be set. User can set transfer mode with SPI_NBITS_SINGLE(1x)
>    * SPI_NBITS_DUAL(2x) and SPI_NBITS_QUAD(4x) to support these three transfer.
>    *
> + * User may also set dtr_mode to true to use dual transfer mode if desired. if
> + * not, default considered as single transfer mode.
> + *
>    * The code that submits an spi_message (and its spi_transfers)
>    * to the lower layers is responsible for managing its memory.
>    * Zero-initialize every field you don't set up explicitly, to
> @@ -1083,6 +1087,7 @@ struct spi_transfer {
>   	unsigned	tx_nbits:4;
>   	unsigned	rx_nbits:4;
>   	unsigned	timestamped:1;
> +	bool		dtr_mode;
>   #define	SPI_NBITS_SINGLE	0x01 /* 1-bit transfer */
>   #define	SPI_NBITS_DUAL		0x02 /* 2-bit transfer */
>   #define	SPI_NBITS_QUAD		0x04 /* 4-bit transfer */

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Konrad Dybcio]

On 3/26/25 10:21 AM, Jyothi Kumar Seerapu wrote:
> Tested-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>

It doesn't look like there's much to test here, you may want to
leave an a-b or r-b instead

Konrad

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Wed, Mar 26, 2025 at 02:09:54PM +0530, Mukesh Kumar Savaliya wrote:

> This change introduces a new field `dtr_mode` in the `spi_transfer`
> structure. The `dtr_mode` field allows protocol drivers to indicate if
> Double Transfer Rate (DTR) mode is supported for a given transfer. When
> `dtr_mode` is set to true, the SPI controller will use DTR mode
> otherwise, it will default to single transfer mode.

> The QSPI controller driver uses this flag and configures single or double
> transfer rate using the controller register.

We should have a flag in the controller indicating if it supports this,
and code in the core which returns an error if a driver attempts to use
it when the controller doesn't support it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Mukesh Kumar Savaliya]

Hi Mark, thanks for your comment.

On 3/26/2025 6:34 PM, Mark Brown wrote:
> On Wed, Mar 26, 2025 at 02:09:54PM +0530, Mukesh Kumar Savaliya wrote:
> 
>> This change introduces a new field `dtr_mode` in the `spi_transfer`
>> structure. The `dtr_mode` field allows protocol drivers to indicate if
>> Double Transfer Rate (DTR) mode is supported for a given transfer. When
>> `dtr_mode` is set to true, the SPI controller will use DTR mode
>> otherwise, it will default to single transfer mode.
> 
>> The QSPI controller driver uses this flag and configures single or double
>> transfer rate using the controller register.
> 
> We should have a flag in the controller indicating if it supports this,
> and code in the core which returns an error if a driver attempts to use
> it when the controller doesn't support it.
Have added below in spi.h which can be set by client and controller 
driver should be using it to decide mode.

+ bool        dtr_mode;

since default it's false, should continue with SDR.
I believe for QSPI, it supports SDR or DDR, but it's not applicable to 
standard SPI right ? So not sure in which case we should return an error ?

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

On Wed, Mar 26, 2025 at 07:55:05PM +0530, Mukesh Kumar Savaliya wrote:
> On 3/26/2025 6:34 PM, Mark Brown wrote:

> > We should have a flag in the controller indicating if it supports this,
> > and code in the core which returns an error if a driver attempts to use
> > it when the controller doesn't support it.

> Have added below in spi.h which can be set by client and controller driver
> should be using it to decide mode.

> + bool        dtr_mode;

> since default it's false, should continue with SDR.
> I believe for QSPI, it supports SDR or DDR, but it's not applicable to
> standard SPI right ? So not sure in which case we should return an error ?

Standard SPI is the main thing I'm thinking of here, or possibly some
limited QSPI controller that doesn't support DTR.  It's not something
that should actually come up really, it's more error handling if things
aren't set up properly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Mukesh Kumar Savaliya]

Thanks Mark !

On 3/26/2025 8:12 PM, Mark Brown wrote:
> On Wed, Mar 26, 2025 at 07:55:05PM +0530, Mukesh Kumar Savaliya wrote:
>> On 3/26/2025 6:34 PM, Mark Brown wrote:
> 
>>> We should have a flag in the controller indicating if it supports this,
>>> and code in the core which returns an error if a driver attempts to use
>>> it when the controller doesn't support it.
> 
>> Have added below in spi.h which can be set by client and controller driver
>> should be using it to decide mode.
> 
>> + bool        dtr_mode;
> 
>> since default it's false, should continue with SDR.
>> I believe for QSPI, it supports SDR or DDR, but it's not applicable to
>> standard SPI right ? So not sure in which case we should return an error ?
> 
> Standard SPI is the main thing I'm thinking of here, or possibly some
> limited QSPI controller that doesn't support DTR.  It's not something
> that should actually come up really, it's more error handling if things
> aren't set up properly.

IIUC, It comes to the point of first identifying if it's in context of 
QSPI controller or SPI controller right ?

Identify if SPI/QSPI controller has this capability using dtr_caps = 
true/false. Then check if it's supporting SDR/DDR mode. Can we then 
introduce below struct to first mark capability as true/false and then 
process dtr_mode ?

if not supported (dtr_caps = false), then don't care dtr_mode.
struct spi_caps {
	bool dtr_mode;
	bool dtr_caps;
};

OR we can have an API spi_controller_dtr_caps() which returns this flag 
set by individual SPI/QSPI controller. We can use above struct OR 
separate  struct spi_controller_dtr_caps { bool dtr_caps };.

Please confirm if i am correct OR suggest more. Else you may approve one 
of the above option. (Sorry if i am missing anything ).

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

On Thu, Mar 27, 2025 at 03:33:15PM +0530, Mukesh Kumar Savaliya wrote:

> IIUC, It comes to the point of first identifying if it's in context of QSPI
> controller or SPI controller right ?

> Identify if SPI/QSPI controller has this capability using dtr_caps =
> true/false. Then check if it's supporting SDR/DDR mode. Can we then
> introduce below struct to first mark capability as true/false and then
> process dtr_mode ?

> if not supported (dtr_caps = false), then don't care dtr_mode.

We should complain if the device requsted anything the controller can't
support.

> struct spi_caps {
> 	bool dtr_mode;
> 	bool dtr_caps;
> };

The controller capabilities are currently mainly advertised via the
SPI_CONTROLLER_ flags but adding some bools also works, some things
already do use that.  I'm not sure we need to wrap things in a further
struct though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Mukesh Kumar Savaliya]

Thanks Mark !

On 3/27/2025 7:37 PM, Mark Brown wrote:
> On Thu, Mar 27, 2025 at 03:33:15PM +0530, Mukesh Kumar Savaliya wrote:
> 
>> IIUC, It comes to the point of first identifying if it's in context of QSPI
>> controller or SPI controller right ?
> 
>> Identify if SPI/QSPI controller has this capability using dtr_caps =
>> true/false. Then check if it's supporting SDR/DDR mode. Can we then
>> introduce below struct to first mark capability as true/false and then
>> process dtr_mode ?
> 
>> if not supported (dtr_caps = false), then don't care dtr_mode.
> 
> We should complain if the device requsted anything the controller can't
> support.
> 
Yes, makes sense. i have added dtr_caps into controller data structure 
and validating it inside the __spi_validate().
>> struct spi_caps {
>> 	bool dtr_mode;
>> 	bool dtr_caps;
>> };
> 
> The controller capabilities are currently mainly advertised via the
> SPI_CONTROLLER_ flags but adding some bools also works, some things
> already do use that.  I'm not sure we need to wrap things in a further
> struct though.
Uploaded Patch V2. Please help review and suggest next. it may require 
still some fine stunning.

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Tudor Ambarus]

On 3/26/25 2:25 PM, Mukesh Kumar Savaliya wrote:
> Hi Mark, thanks for your comment.
> 
> On 3/26/2025 6:34 PM, Mark Brown wrote:
>> On Wed, Mar 26, 2025 at 02:09:54PM +0530, Mukesh Kumar Savaliya wrote:
>>
>>> This change introduces a new field `dtr_mode` in the `spi_transfer`
>>> structure. The `dtr_mode` field allows protocol drivers to indicate if
>>> Double Transfer Rate (DTR) mode is supported for a given transfer. When
>>> `dtr_mode` is set to true, the SPI controller will use DTR mode
>>> otherwise, it will default to single transfer mode.
>>
>>> The QSPI controller driver uses this flag and configures single or
>>> double
>>> transfer rate using the controller register.
>>
>> We should have a flag in the controller indicating if it supports this,
>> and code in the core which returns an error if a driver attempts to use
>> it when the controller doesn't support it.
> Have added below in spi.h which can be set by client and controller
> driver should be using it to decide mode.
> 
> + bool        dtr_mode;
> 
> since default it's false, should continue with SDR.
> I believe for QSPI, it supports SDR or DDR, but it's not applicable to
> standard SPI right ? So not sure in which case we should return an error ?
> 

Please check how spimem is dealing with DTR, same ideas shall be applied
for spi transfers.

Cheers,
ta

Re: [PATCH V1] spi: Add support for Double Transfer Rate (DTR) mode

[Mukesh Kumar Savaliya]

Thanks Tudor !

On 3/26/2025 8:18 PM, Tudor Ambarus wrote:
> 
> 
> On 3/26/25 2:25 PM, Mukesh Kumar Savaliya wrote:
>> Hi Mark, thanks for your comment.
>>
>> On 3/26/2025 6:34 PM, Mark Brown wrote:
>>> On Wed, Mar 26, 2025 at 02:09:54PM +0530, Mukesh Kumar Savaliya wrote:
>>>
>>>> This change introduces a new field `dtr_mode` in the `spi_transfer`
>>>> structure. The `dtr_mode` field allows protocol drivers to indicate if
>>>> Double Transfer Rate (DTR) mode is supported for a given transfer. When
>>>> `dtr_mode` is set to true, the SPI controller will use DTR mode
>>>> otherwise, it will default to single transfer mode.
>>>
>>>> The QSPI controller driver uses this flag and configures single or
>>>> double
>>>> transfer rate using the controller register.
>>>
>>> We should have a flag in the controller indicating if it supports this,
>>> and code in the core which returns an error if a driver attempts to use
>>> it when the controller doesn't support it.
>> Have added below in spi.h which can be set by client and controller
>> driver should be using it to decide mode.
>>
>> + bool        dtr_mode;
>>
>> since default it's false, should continue with SDR.
>> I believe for QSPI, it supports SDR or DDR, but it's not applicable to
>> standard SPI right ? So not sure in which case we should return an error ?
>>
> 
> Please check how spimem is dealing with DTR, same ideas shall be applied
> for spi transfers.
> 
Yes, i just got it. Have kept my proposal aligning to this. Looks 
similar to spi_mem_controller_is_capable(). Please review reply to Mark 
if that is matching expectations OR have more suggestion/corrections.
> Cheers,
> ta