virtio-spi: add the device specification

Hi Haixu,

Quoted 3 lines

+The \field{word_delay} defines how long to wait between words within one SPI transfer,
+in ns unit.

I'm a little surprised to see a word_delay but no frame_delay or transfer_delay.

For example, many SPI peripherals require a delay after CS is asserted, but before the first SCLK edge, allowing them to prepare to send data. (E.g. an ADC might begin taking a sample.)

The linux struct spi_transfer has three delay fields:

* @cs_change_delay: delay between cs deassert and assert when *      @cs_change is set and @spi_transfer is not the last in *      @spi_message
* @delay: delay to be introduced after this transfer before *    (optionally) changing the chipselect status, then starting the *    next transfer or completing this @spi_message.
* @word_delay: inter word delay to be introduced after each word size *    (set by bits_per_word) transmission.

The userspace spidev.h API has only two:

* @delay_usecs: If nonzero, how long to delay after the last bit *       transfer before optionally deselecting the device before the *       next transfer.
* @word_delay_usecs: If nonzero, how long to wait between words within *       one transfer. This property needs explicit support in the SPI *       controller, otherwise it is silently ignored.

The NXP i.MX RT500 SPI (master) controller, for example, has four configurable delays:

- PRE_DELAY: After CS assertion, before first SCLK edge.
- POST_DELAY: After a transfer, before CS deassertion.
- FRAME_DELAY: Between frames (which are arbitrarily defined by software).
- TRANSFER_DELAY: Minimum CS deassertion time between transfers.

The NVIDIA Tegra114 SPI controller has:

- nvidia,cs-setup-clk-count: CS setup timing parameter.
- nvidia,cs-hold-clk-count: CS hold timing parameter.

I understand that accurately representing all possible delays might be difficult or futile, but I'm curious to understand why, of all the possible delays, inter-word delay was chosen for inclusion.

In a microcontroller, delays around CS (de)assertion can be customized by using a GPIO -- rather than the hardware SPI block -- to drive the CS signal. This way, delays can be inserted where needed. To do so with a virtualized SPI controller might prove difficult however, as the virtual channel carrying a CS GPIO signal might not be synchronized to the channel carrying the SPI data.

Curious to hear your thoughts.

Thanks,
Jonathon Reinhart

Hi @[email protected],
Thank you very much for your helpful comments.

I missed the delay and cs_change_delay parameters. I will add both
of them, although cs_change_delay can not be set from userspace, but can be set in kernel space.

For CS delays such as PRE_DELAY/POST_DELAY, it seems that they are
defined in structure spi_device:

@cs_setup: delay to be introduced by the controller after CS is
asserted.

@cs_hold: delay to be introduced by the controller before CS is
deasserted.

@cs_inactive: delay to be introduced by the controller after CS is
deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

They show the SPI controller ability to control the CS
assertion/deassertion timing and should not be changed for each transfer (because thay can be updated by setting structure spi_transfer or structure spi_ioc_transfer). I think it better to define these parameter in host OS rather than in guest OS since it's the host OS to operate the hardware SPI controller directly. Besides, it can also avoid passing the same values from guest to host time after time.

What's your opinion on this topic? Again, thank you very much.

Best Regards
Haixu Cui

Quoted 64 lines · On 6/28/2023 1:06 AM, [email protected] wrote:

Hi Haixu,

Quoted 3 lines

+The \field{word_delay} defines how long to wait between words within one SPI transfer,
+in ns unit.

I'm a little surprised to see a word_delay but no frame_delay or transfer_delay.

For example, many SPI peripherals require a delay after CS is asserted, but before the first SCLK edge, allowing them to prepare to send data. (E.g. an ADC might begin taking a sample.)

The linux struct spi_transfer has three delay fields:

* @cs_change_delay: delay between cs deassert and assert when
* @cs_change is set and @spi_transfer is not the last in
* @spi_message
* @delay: delay to be introduced after this transfer before
* (optionally) changing the chipselect status, then starting the
* next transfer or completing this @spi_message.
* @word_delay: inter word delay to be introduced after each word size
* (set by bits_per_word) transmission.

The userspace spidev.h API has only two:

* @delay_usecs: If nonzero, how long to delay after the last bit
* transfer before optionally deselecting the device before the
* next transfer.
* @word_delay_usecs: If nonzero, how long to wait between words within
* one transfer. This property needs explicit support in the SPI
* controller, otherwise it is silently ignored.

The NXP i.MX RT500 SPI (master) controller, for example, has four configurable delays:

- PRE_DELAY: After CS assertion, before first SCLK edge.
- POST_DELAY: After a transfer, before CS deassertion.
- FRAME_DELAY: Between frames (which are arbitrarily defined by software).
- TRANSFER_DELAY: Minimum CS deassertion time between transfers.

The NVIDIA Tegra114 SPI controller has:

- nvidia,cs-setup-clk-count: CS setup timing parameter.
- nvidia,cs-hold-clk-count: CS hold timing parameter.

I understand that accurately representing all possible delays might be difficult or futile, but I'm curious to understand why, of all the possible delays, inter-word delay was chosen for inclusion.

In a microcontroller, delays around CS (de)assertion can be customized by using a GPIO -- rather than the hardware SPI block -- to drive the CS signal. This way, delays can be inserted where needed. To do so with a virtualized SPI controller might prove difficult however, as the virtual channel carrying a CS GPIO signal might not be synchronized to the channel carrying the SPI data.

Curious to hear your thoughts.

Thanks,
Jonathon Reinhart

Quoted 29 lines

Hi @[email protected],
Thank you very much for your helpful comments.

I missed the delay and cs_change_delay parameters. I will add both
of them, although cs_change_delay can not be set from userspace, but can be set in kernel space.

For CS delays such as PRE_DELAY/POST_DELAY, it seems that they are
defined in structure spi_device:

@cs_setup: delay to be introduced by the controller after CS is
asserted.

@cs_hold: delay to be introduced by the controller before CS is
deasserted.

@cs_inactive: delay to be introduced by the controller after CS is
deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

They show the SPI controller ability to control the CS
assertion/deassertion timing and should not be changed for each transfer (because thay can be updated by setting structure spi_transfer or structure spi_ioc_transfer). I think it better to define these parameter in host OS rather than in guest OS since it's the host OS to operate the hardware SPI controller directly. Besides, it can also avoid passing the same values from guest to host time after time.

Quoted 1 line

What's your opinion on this topic? Again, thank you very much.

Hi Haixu,

I took another look at the Linux structures and attempted to map up the delay parameters to the various points in a SPI sequence. Please correct me if I'm wrong about any of this.

Consider this diagram where CS# is an active-low chip select, and SCLK is the serial clock:

       ___.                                                   ._____.
   CS#    |___________________________________________________|     |___
          .                                                   .     .
          .                                                   .     .
  SCLK ________NNNN___NNNN___NNNN_______NNNN___NNNN___NNNN______________
          .   .    . .           .     .                  .   .     .
Delays:  +-A-+    +B+           +--C--+                  +-D-+--E--+
          .   .    . .           .     .                  .   .     .
          0   1    2 3           4     5                  6   7     8
          .   .    .             .     .                  .   .
Terms:   .   +Word+             .     .                  .   .
          .   .                  .     .                  .   .
          .   +-------Frame------+     +-------Frame------+   .
          .                                                   .
          +-----------------Transfer--------------------------+

Using NXP terminology:

From 1 to 2 is a "word" (with e.g. 8 bits).
From 1 to 4 is a "frame" (with 3 words).
From 1 to 6 is a "transfer" (with 3 frames).

Linux does not have the concept of a frame, only a series of transfers (a spi_message), each of which can specify whether CS changes or not.

I've identified these various timing points and attempted to match them to the Linux spi_device and spi_transfer parameters:

A - CS Setup: Delay after CS asserted before clock starts.
spi_device.cs_setup

B - Word Delay: Delay between words.
spi_transfer.word_delay

C - Frame Delay: Delay between frames.
(Linux does not have the concept of a SPI "frame".)

D - CS Hold: Delay after clock stops, and before CS deasserted.
spi_device.cs_hold (+ spi_transfer.delay ??)

E - CS Inactive: Delay after CS deasserted, before asserting again.
spi_device.cs_inactive + spi_transfer.cs_change_delay

While I agree with you that some of these timings are unlikely to change from transfer-to-transfer, the subset of which should be specified at the device level vs. per-transfer seems somewhat arbitrary. As you can see there is some overlap between them.

If I understand correctly, it appears that the CS hold time *can* be controlled on a per-transfer bases, using spi_transfer.delay ("after this transfer before changing the chipselect status").

That leaves CS setup as the only parameter that cannot be influenced on a per-transfer basis.

Theoretically, one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus). In that case, one must set those spi_device parameters to the worst-case (maximum) values. However, this is already a Linux limitation, so I'm not sure it needs to be improved here.

I think you've achieved parity with what the Linux kernel API allows, and that's probably good enough. As you said, anything else can be adjusted in the host OS -- I'm not sure how the guest would go about achieving that, though. You're not proposing the use of configuration space for virtio-spi, are you?

Regards,
Jonathon Reinhart

Quoted 69 lines

Best Regards
Haixu Cui

Quoted 64 lines · On 6/28/2023 1:06 AM, [email protected] wrote:

Hi Haixu,

Quoted 3 lines

+The \field{word_delay} defines how long to wait between words within one SPI transfer,
+in ns unit.

I'm a little surprised to see a word_delay but no frame_delay or transfer_delay.

For example, many SPI peripherals require a delay after CS is asserted, but before the first SCLK edge, allowing them to prepare to send data. (E.g. an ADC might begin taking a sample.)

The linux struct spi_transfer has three delay fields:

* @cs_change_delay: delay between cs deassert and assert when
* @cs_change is set and @spi_transfer is not the last in
* @spi_message
* @delay: delay to be introduced after this transfer before
* (optionally) changing the chipselect status, then starting the
* next transfer or completing this @spi_message.
* @word_delay: inter word delay to be introduced after each word size
* (set by bits_per_word) transmission.

The userspace spidev.h API has only two:

* @delay_usecs: If nonzero, how long to delay after the last bit
* transfer before optionally deselecting the device before the
* next transfer.
* @word_delay_usecs: If nonzero, how long to wait between words within
* one transfer. This property needs explicit support in the SPI
* controller, otherwise it is silently ignored.

The NXP i.MX RT500 SPI (master) controller, for example, has four configurable delays:

- PRE_DELAY: After CS assertion, before first SCLK edge.
- POST_DELAY: After a transfer, before CS deassertion.
- FRAME_DELAY: Between frames (which are arbitrarily defined by software).
- TRANSFER_DELAY: Minimum CS deassertion time between transfers.

The NVIDIA Tegra114 SPI controller has:

- nvidia,cs-setup-clk-count: CS setup timing parameter.
- nvidia,cs-hold-clk-count: CS hold timing parameter.

I understand that accurately representing all possible delays might be difficult or futile, but I'm curious to understand why, of all the possible delays, inter-word delay was chosen for inclusion.

In a microcontroller, delays around CS (de)assertion can be customized by using a GPIO -- rather than the hardware SPI block -- to drive the CS signal. This way, delays can be inserted where needed. To do so with a virtualized SPI controller might prove difficult however, as the virtual channel carrying a CS GPIO signal might not be synchronized to the channel carrying the SPI data.

Curious to hear your thoughts.

Thanks,
Jonathon Reinhart

Hi Jonathon Reinhart,

Thank you so much for all your helpful advice and info!

I took a look at latest Linux SPI driver and found, for
cs_setup/cs_hold/cs_inactive set in device-tree, there are following two conditions:
1) if SPI controller supports CS timing configuration and CS is not
using a GPIO line, then the SPI driver set the cs_setup/cs_hold/ca_inactive values into the corresponding SPI CS registers;
2) if CS is using a GPIO line, or SPI controller doesn't support CS
timing configuration, it is the software to perform the cs_setup/cs_hold/cs_inactive delays, which is implemented in function spi_set_cs in driver/spi/spi.c.

So I agree it is necessary to pass cs_setup/cs_hold/cs_inactive to
the host for each transfer.

For condition 1, host should set these values into the CS timing
registers. And as you mentioned "one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus)", if so, host need to overwrite the CS timing registers between the two transfers talking to different salve_id.

For condition 2, SPI driver running in the host performing the
cs_setup/cs_hold/cs_inactive delays seems more accurate, compared with performing delays in guest.

     And the latest virtio spi transfer structure is defined as follows:

      struct spi_transfer_head {
         u8 slave_id;
         u8 bits_per_word;
         u8 cs_change;
         u8 tx_nbits;
        u8 rx_nbits;
        u8 reserved[3];
         u32 mode;

         u32 freq;
         u32 word_delay_ns;
         u32 delay_ns;
         u32 cs_change_delay_ns;
         u32 cs_setup_ns;
         u32 cs_hold_ns;
         u32 cs_inactive_ns;
      };

@slave_id: chipselect index the SPI transfer used @bits_per_word: the number of bits in each SPI transfer word @cs_change: True to deselect device before starting the next transfer @tx_nbits: number of bits used for writing

@rx_nbits: number of bits used for reading
@reserved: reserved for future use
@mode: the spi mode defines how data is clocked out and in @freq: transfer speed
@word_delay_ns: delay to be inserted between consecutive words of
a transfer, in ns unit
@cs_setup_ns: delay to be introduced by the controller after CS is
asserted, in ns unit

@delay_ns: delay to be introduced after this transfer before
(optionally) changing the chipselect status, in ns unit
@cs_hold_ns: delay to be introduced by the controller before CS is
deasserted, in ns unit

@cs_change_delay_ns: delay between cs deassert and assert when
cs_change is set, in ns unit
@cs_inactive_ns: delay to be introduced by the controller after CS
is deasserted, in ns unit

Could you please help check if this new structure contains enough
information. Really appreciate for kind help.

Best Regards
Haixu Cui

Quoted 181 lines · Jonathon Reinhart

Quoted 29 lines

Hi @[email protected],
Thank you very much for your helpful comments.

I missed the delay and cs_change_delay parameters. I will add both
of them, although cs_change_delay can not be set from userspace, but can be set in kernel space.

For CS delays such as PRE_DELAY/POST_DELAY, it seems that they are
defined in structure spi_device:

@cs_setup: delay to be introduced by the controller after CS is
asserted.

@cs_hold: delay to be introduced by the controller before CS is
deasserted.

@cs_inactive: delay to be introduced by the controller after CS is
deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

They show the SPI controller ability to control the CS
assertion/deassertion timing and should not be changed for each transfer (because thay can be updated by setting structure spi_transfer or structure spi_ioc_transfer). I think it better to define these parameter in host OS rather than in guest OS since it's the host OS to operate the hardware SPI controller directly. Besides, it can also avoid passing the same values from guest to host time after time.

Quoted 1 line

What's your opinion on this topic? Again, thank you very much.

Hi Haixu,

I took another look at the Linux structures and attempted to map up the delay parameters to the various points in a SPI sequence. Please correct me if I'm wrong about any of this.

Consider this diagram where CS# is an active-low chip select, and SCLK is the serial clock:

       ___.                                                   ._____.
   CS#    |___________________________________________________|     |___
          .                                                   .     .
          .                                                   .     .
  SCLK ________NNNN___NNNN___NNNN_______NNNN___NNNN___NNNN______________
          .   .    . .           .     .                  .   .     .
Delays:  +-A-+    +B+           +--C--+                  +-D-+--E--+
          .   .    . .           .     .                  .   .     .
          0   1    2 3           4     5                  6   7     8
          .   .    .             .     .                  .   .
Terms:   .   +Word+             .     .                  .   .
          .   .                  .     .                  .   .
          .   +-------Frame------+     +-------Frame------+   .
          .                                                   .
          +-----------------Transfer--------------------------+

Using NXP terminology:

From 1 to 2 is a "word" (with e.g. 8 bits).
From 1 to 4 is a "frame" (with 3 words).
From 1 to 6 is a "transfer" (with 3 frames).

Linux does not have the concept of a frame, only a series of transfers (a spi_message), each of which can specify whether CS changes or not.

I've identified these various timing points and attempted to match them to the Linux spi_device and spi_transfer parameters:

A - CS Setup: Delay after CS asserted before clock starts.
spi_device.cs_setup

B - Word Delay: Delay between words.
spi_transfer.word_delay

C - Frame Delay: Delay between frames.
(Linux does not have the concept of a SPI "frame".)

D - CS Hold: Delay after clock stops, and before CS deasserted.
spi_device.cs_hold (+ spi_transfer.delay ??)

E - CS Inactive: Delay after CS deasserted, before asserting again.
spi_device.cs_inactive + spi_transfer.cs_change_delay

While I agree with you that some of these timings are unlikely to change from transfer-to-transfer, the subset of which should be specified at the device level vs. per-transfer seems somewhat arbitrary. As you can see there is some overlap between them.

If I understand correctly, it appears that the CS hold time *can* be controlled on a per-transfer bases, using spi_transfer.delay ("after this transfer before changing the chipselect status").

That leaves CS setup as the only parameter that cannot be influenced on a per-transfer basis.

Theoretically, one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus). In that case, one must set those spi_device parameters to the worst-case (maximum) values. However, this is already a Linux limitation, so I'm not sure it needs to be improved here.

I think you've achieved parity with what the Linux kernel API allows, and that's probably good enough. As you said, anything else can be adjusted in the host OS -- I'm not sure how the guest would go about achieving that, though. You're not proposing the use of configuration space for virtio-spi, are you?

Regards,
Jonathon Reinhart

Quoted 69 lines

Best Regards
Haixu Cui

Quoted 64 lines · On 6/28/2023 1:06 AM, [email protected] wrote:

Hi Haixu,

Quoted 3 lines

+The \field{word_delay} defines how long to wait between words within one SPI transfer,
+in ns unit.

I'm a little surprised to see a word_delay but no frame_delay or transfer_delay.

For example, many SPI peripherals require a delay after CS is asserted, but before the first SCLK edge, allowing them to prepare to send data. (E.g. an ADC might begin taking a sample.)

The linux struct spi_transfer has three delay fields:

* @cs_change_delay: delay between cs deassert and assert when
* @cs_change is set and @spi_transfer is not the last in
* @spi_message
* @delay: delay to be introduced after this transfer before
* (optionally) changing the chipselect status, then starting the
* next transfer or completing this @spi_message.
* @word_delay: inter word delay to be introduced after each word size
* (set by bits_per_word) transmission.

The userspace spidev.h API has only two:

* @delay_usecs: If nonzero, how long to delay after the last bit
* transfer before optionally deselecting the device before the
* next transfer.
* @word_delay_usecs: If nonzero, how long to wait between words within
* one transfer. This property needs explicit support in the SPI
* controller, otherwise it is silently ignored.

The NXP i.MX RT500 SPI (master) controller, for example, has four configurable delays:

- PRE_DELAY: After CS assertion, before first SCLK edge.
- POST_DELAY: After a transfer, before CS deassertion.
- FRAME_DELAY: Between frames (which are arbitrarily defined by software).
- TRANSFER_DELAY: Minimum CS deassertion time between transfers.

The NVIDIA Tegra114 SPI controller has:

- nvidia,cs-setup-clk-count: CS setup timing parameter.
- nvidia,cs-hold-clk-count: CS hold timing parameter.

I understand that accurately representing all possible delays might be difficult or futile, but I'm curious to understand why, of all the possible delays, inter-word delay was chosen for inclusion.

In a microcontroller, delays around CS (de)assertion can be customized by using a GPIO -- rather than the hardware SPI block -- to drive the CS signal. This way, delays can be inserted where needed. To do so with a virtualized SPI controller might prove difficult however, as the virtual channel carrying a CS GPIO signal might not be synchronized to the channel carrying the SPI data.

Curious to hear your thoughts.

Thanks,
Jonathon Reinhart

Quoted 40 lines · Haixu Cui

Hi Jonathon Reinhart,

Thank you so much for all your helpful advice and info!

I took a look at latest Linux SPI driver and found, for
cs_setup/cs_hold/cs_inactive set in device-tree, there are following two conditions:
1) if SPI controller supports CS timing configuration and CS is not
using a GPIO line, then the SPI driver set the cs_setup/cs_hold/ca_inactive values into the corresponding SPI CS registers;
2) if CS is using a GPIO line, or SPI controller doesn't support CS
timing configuration, it is the software to perform the cs_setup/cs_hold/cs_inactive delays, which is implemented in function spi_set_cs in driver/spi/spi.c.

So I agree it is necessary to pass cs_setup/cs_hold/cs_inactive to
the host for each transfer.

For condition 1, host should set these values into the CS timing
registers. And as you mentioned "one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus)", if so, host need to overwrite the CS timing registers between the two transfers talking to different salve_id.

For condition 2, SPI driver running in the host performing the
cs_setup/cs_hold/cs_inactive delays seems more accurate, compared with performing delays in guest.

      And the latest virtio spi transfer structure is defined as follows:

       struct spi_transfer_head {
          u8 slave_id;
          u8 bits_per_word;
          u8 cs_change;
          u8 tx_nbits;
         u8 rx_nbits;
         u8 reserved[3];
          u32 mode;

Quoted 9 lines

          u32 freq;
          u32 word_delay_ns;
          u32 delay_ns;
          u32 cs_change_delay_ns;
          u32 cs_setup_ns;
          u32 cs_hold_ns;
          u32 cs_inactive_ns;
       };

Hello Haixu Cui,

I think there may be some unnecessary redundancy in these fields. See below.

Quoted 4 lines

@slave_id: chipselect index the SPI transfer used @bits_per_word: the number of bits in each SPI transfer word @cs_change: True to deselect device before starting the next transfer @tx_nbits: number of bits used for writing

Quoted 8 lines

@rx_nbits: number of bits used for reading
@reserved: reserved for future use
@mode: the spi mode defines how data is clocked out and in @freq: transfer speed
@word_delay_ns: delay to be inserted between consecutive words of
a transfer, in ns unit
@cs_setup_ns: delay to be introduced by the controller after CS is
asserted, in ns unit

(I am reordering these in my quoted text to be grouped appropriately.)

Quoted 4 lines

@delay_ns: delay to be introduced after this transfer before
(optionally) changing the chipselect status, in ns unit
@cs_hold_ns: delay to be introduced by the controller before CS is
deasserted, in ns unit

Aren't @delay_ns and @cs_hold_ns specifying the same thing?

Linux spi_transfer defines @delay as:
delay to be introduced after this transfer before (optionally) changing the chipselect status, then starting the next transfer or completing this spi_message.

...and spi_device @cs_hold as: delay to be introduced by the
controller before CS is deasserted.

(This is the period labeled "D" in my diagram.)

I guess the only difference is if you have a series of transfers, where only the last transfer has @cs_change=true, and all preceding messages have @cs_change=false. In this case, @delay_ns would apply between each transfer, and after the last transfer @delay_ns + @cs_hold would apply before CS is deasserted:

     Transfer[0] (cs_change=false)
     delay(@delay_ns)
     Transfer[1] (cs_change=false)
     delay(@delay_ns)
     Transfer[3] (cs_change=true)
     delay(@delay_ns) + delay(@cs_hold_ns)
     CS deasserted

Quoted 4 lines

@cs_change_delay_ns: delay between cs deassert and assert when
cs_change is set, in ns unit
@cs_inactive_ns: delay to be introduced by the controller after CS
is deasserted, in ns unit

Similarly, aren't @cs_change_delay_ns and @cs_inactive_ns applied at the same point in the cycle?

(This is the period labeled "E" in my diagram.)

Linux spi_device defines @cs_inactive as: delay to be introduced by the
controller after CS is deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

...and spi_transfer defines @cs_change_delay as: delay between cs
deassert and assert when @cs_change is set and @spi_transfer is not the last in @spi_message.

It even says they will be added up.

The only difference seems to be that @cs_change_delay applies only when the transfer is not the last in a list (spi_message, which we don't have here.)

CS asserted
Transfer[0] (cs_change=true)
CS deasserted
delay(@cs_inactive) + delay(@cs_change_delay) CS asserted
Transfer[1] (cs_change=true)
CS deasserted
delay(@cs_inactive)

In both cases of redundancy above, the difference in semantics between the spi_device and spi_transfer only seem to apply when a series of transfers (spi_message or SPI_IOC_MESSAGE) are involved. Since (AFAICT) you aren't proposing that construct here, maybe the separate fields are not necessary? IOW, maybe @delay_ns and @cs_change_delay can be eliminated?

Please let me know if I've misunderstood anything.

Jonathon

Quoted 189 lines

Could you please help check if this new structure contains enough
information. Really appreciate for kind help.

Best Regards
Haixu Cui

Quoted 181 lines · Jonathon Reinhart

Quoted 29 lines

Hi @[email protected],
Thank you very much for your helpful comments.

I missed the delay and cs_change_delay parameters. I will add both
of them, although cs_change_delay can not be set from userspace, but can be set in kernel space.

For CS delays such as PRE_DELAY/POST_DELAY, it seems that they are
defined in structure spi_device:

@cs_setup: delay to be introduced by the controller after CS is
asserted.

@cs_hold: delay to be introduced by the controller before CS is
deasserted.

@cs_inactive: delay to be introduced by the controller after CS is
deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

They show the SPI controller ability to control the CS
assertion/deassertion timing and should not be changed for each transfer (because thay can be updated by setting structure spi_transfer or structure spi_ioc_transfer). I think it better to define these parameter in host OS rather than in guest OS since it's the host OS to operate the hardware SPI controller directly. Besides, it can also avoid passing the same values from guest to host time after time.

Quoted 1 line

What's your opinion on this topic? Again, thank you very much.

Hi Haixu,

I took another look at the Linux structures and attempted to map up the delay parameters to the various points in a SPI sequence. Please correct me if I'm wrong about any of this.

Consider this diagram where CS# is an active-low chip select, and SCLK is the serial clock:

        ___.                                                   ._____.
    CS#    |___________________________________________________|     |___
           .                                                   .     .
           .                                                   .     .
   SCLK ________NNNN___NNNN___NNNN_______NNNN___NNNN___NNNN______________
           .   .    . .           .     .                  .   .     .
Delays:  +-A-+    +B+           +--C--+                  +-D-+--E--+
           .   .    . .           .     .                  .   .     .
           0   1    2 3           4     5                  6   7     8
           .   .    .             .     .                  .   .
Terms:   .   +Word+             .     .                  .   .
           .   .                  .     .                  .   .
           .   +-------Frame------+     +-------Frame------+   .
           .                                                   .
           +-----------------Transfer--------------------------+

Using NXP terminology:

From 1 to 2 is a "word" (with e.g. 8 bits).
From 1 to 4 is a "frame" (with 3 words).
From 1 to 6 is a "transfer" (with 3 frames).

Linux does not have the concept of a frame, only a series of transfers (a spi_message), each of which can specify whether CS changes or not.

I've identified these various timing points and attempted to match them to the Linux spi_device and spi_transfer parameters:

A - CS Setup: Delay after CS asserted before clock starts.
spi_device.cs_setup

B - Word Delay: Delay between words.
spi_transfer.word_delay

C - Frame Delay: Delay between frames.
(Linux does not have the concept of a SPI "frame".)

D - CS Hold: Delay after clock stops, and before CS deasserted.
spi_device.cs_hold (+ spi_transfer.delay ??)

E - CS Inactive: Delay after CS deasserted, before asserting again.
spi_device.cs_inactive + spi_transfer.cs_change_delay

While I agree with you that some of these timings are unlikely to change from transfer-to-transfer, the subset of which should be specified at the device level vs. per-transfer seems somewhat arbitrary. As you can see there is some overlap between them.

If I understand correctly, it appears that the CS hold time *can* be controlled on a per-transfer bases, using spi_transfer.delay ("after this transfer before changing the chipselect status").

That leaves CS setup as the only parameter that cannot be influenced on a per-transfer basis.

Theoretically, one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus). In that case, one must set those spi_device parameters to the worst-case (maximum) values. However, this is already a Linux limitation, so I'm not sure it needs to be improved here.

I think you've achieved parity with what the Linux kernel API allows, and that's probably good enough. As you said, anything else can be adjusted in the host OS -- I'm not sure how the guest would go about achieving that, though. You're not proposing the use of configuration space for virtio-spi, are you?

Regards,
Jonathon Reinhart

Quoted 69 lines

Best Regards
Haixu Cui

Quoted 64 lines · On 6/28/2023 1:06 AM, [email protected] wrote:

Hi Haixu,

Quoted 3 lines

+The \field{word_delay} defines how long to wait between words within one SPI transfer,
+in ns unit.

I'm a little surprised to see a word_delay but no frame_delay or transfer_delay.

For example, many SPI peripherals require a delay after CS is asserted, but before the first SCLK edge, allowing them to prepare to send data. (E.g. an ADC might begin taking a sample.)

The linux struct spi_transfer has three delay fields:

* @cs_change_delay: delay between cs deassert and assert when
* @cs_change is set and @spi_transfer is not the last in
* @spi_message
* @delay: delay to be introduced after this transfer before
* (optionally) changing the chipselect status, then starting the
* next transfer or completing this @spi_message.
* @word_delay: inter word delay to be introduced after each word size
* (set by bits_per_word) transmission.

The userspace spidev.h API has only two:

* @delay_usecs: If nonzero, how long to delay after the last bit
* transfer before optionally deselecting the device before the
* next transfer.
* @word_delay_usecs: If nonzero, how long to wait between words within
* one transfer. This property needs explicit support in the SPI
* controller, otherwise it is silently ignored.

The NXP i.MX RT500 SPI (master) controller, for example, has four configurable delays:

- PRE_DELAY: After CS assertion, before first SCLK edge.
- POST_DELAY: After a transfer, before CS deassertion.
- FRAME_DELAY: Between frames (which are arbitrarily defined by software).
- TRANSFER_DELAY: Minimum CS deassertion time between transfers.

The NVIDIA Tegra114 SPI controller has:

- nvidia,cs-setup-clk-count: CS setup timing parameter.
- nvidia,cs-hold-clk-count: CS hold timing parameter.

I understand that accurately representing all possible delays might be difficult or futile, but I'm curious to understand why, of all the possible delays, inter-word delay was chosen for inclusion.

In a microcontroller, delays around CS (de)assertion can be customized by using a GPIO -- rather than the hardware SPI block -- to drive the CS signal. This way, delays can be inserted where needed. To do so with a virtualized SPI controller might prove difficult however, as the virtual channel carrying a CS GPIO signal might not be synchronized to the channel carrying the SPI data.

Curious to hear your thoughts.

Thanks,
Jonathon Reinhart

Hi Jonathon Reinhart,
Thank you very much for your comments.

Quoted 75 lines · Jonathon Reinhart

Quoted 40 lines · Haixu Cui

Hi Jonathon Reinhart,

Thank you so much for all your helpful advice and info!

I took a look at latest Linux SPI driver and found, for
cs_setup/cs_hold/cs_inactive set in device-tree, there are following two conditions:
1) if SPI controller supports CS timing configuration and CS is not
using a GPIO line, then the SPI driver set the cs_setup/cs_hold/ca_inactive values into the corresponding SPI CS registers;
2) if CS is using a GPIO line, or SPI controller doesn't support CS
timing configuration, it is the software to perform the cs_setup/cs_hold/cs_inactive delays, which is implemented in function spi_set_cs in driver/spi/spi.c.

So I agree it is necessary to pass cs_setup/cs_hold/cs_inactive to
the host for each transfer.

For condition 1, host should set these values into the CS timing
registers. And as you mentioned "one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus)", if so, host need to overwrite the CS timing registers between the two transfers talking to different salve_id.

For condition 2, SPI driver running in the host performing the
cs_setup/cs_hold/cs_inactive delays seems more accurate, compared with performing delays in guest.

      And the latest virtio spi transfer structure is defined as follows:

       struct spi_transfer_head {
          u8 slave_id;
          u8 bits_per_word;
          u8 cs_change;
          u8 tx_nbits;
         u8 rx_nbits;
         u8 reserved[3];
          u32 mode;

Quoted 9 lines

          u32 freq;
          u32 word_delay_ns;
          u32 delay_ns;
          u32 cs_change_delay_ns;
          u32 cs_setup_ns;
          u32 cs_hold_ns;
          u32 cs_inactive_ns;
       };

Hello Haixu Cui,

I think there may be some unnecessary redundancy in these fields. See below.

Quoted 4 lines

@slave_id: chipselect index the SPI transfer used @bits_per_word: the number of bits in each SPI transfer word @cs_change: True to deselect device before starting the next transfer @tx_nbits: number of bits used for writing

Quoted 8 lines

@rx_nbits: number of bits used for reading
@reserved: reserved for future use
@mode: the spi mode defines how data is clocked out and in @freq: transfer speed
@word_delay_ns: delay to be inserted between consecutive words of
a transfer, in ns unit
@cs_setup_ns: delay to be introduced by the controller after CS is
asserted, in ns unit

(I am reordering these in my quoted text to be grouped appropriately.)

Quoted 4 lines

@delay_ns: delay to be introduced after this transfer before
(optionally) changing the chipselect status, in ns unit
@cs_hold_ns: delay to be introduced by the controller before CS is
deasserted, in ns unit

Quoted 1 line

Aren't @delay_ns and @cs_hold_ns specifying the same thing?

I think they are not the same thing, delay_ns is the spi transfer property while cs_hold_ns is the spi device property， although they take effect at the same stage.

I list 4 conditions, here transfer cs delay including spi_transfer->delay and spi_transfer->cs_change_delay, and device cs delay including spi_device->cs_hold, spi_device->cs_setup and spi_device->cs_inactive.

condition 1:
virtio-spi controller only has transfer_one interface but no
transfer_one_message interface, and spi_transfer_one_message is the default transfer_one_message interface of virtio-spi controller.
Hardware SPI controller doesn't support CS timing configuration.

In this case, both transfer cs delay and device cs delay are
executed by software in spi_transfer_one_message function in guest Linux OS. So guest doesn't need pass these cs timing parameters to host.

condition 2:
virtio-spi controller only has transfer_one interface but no
transfer_one_message interface, and spi_transfer_one_message is the default transfer_one_message interface of virtio-spi controller.
Hardware SPI controller supports device CS timing configuration,
which means it has registers to hold these configuration values.

In this case, transfer cs delay is executed in
spi_transfer_one_message function, and guest need pass device cs delay to host. Because one SPI number may have more than one slave, these slaves also may have different device cs delay values, so for each transfer, guest should pass device cs delay along with the slave_id to host to overwrite the corresponding registers.

condition 3:
virtio-spi controller has transfer_one_message interface but no
transfer_one interface.
Hardware SPI controller doesn't support CS timing configuration.

In this case, host SPI driver should implement all cs delay
operations with the transfer cs delay and device cs delay received from the guest.

condition 4:
virtio-spi controller has transfer_one_message interface but no
transfer_one interface.
Hardware SPI controller supports CS timing configuration.

In this case, guest pass transfer cs delay and device cs delay to
guest, and guest SPI driver should implement logic to execute transfer cs delay then overwrite the corresponding register with the device cs delay values.

Except for condition 1, in other three conditions, guest should pass both transfer cs delay and device cs delay to host.

Best Regards
Haixu Cui

Quoted 260 lines

Linux spi_transfer defines @delay as:
delay to be introduced after this transfer before (optionally) changing the chipselect status, then starting the next transfer or completing this spi_message.

...and spi_device @cs_hold as: delay to be introduced by the
controller before CS is deasserted.

(This is the period labeled "D" in my diagram.)

I guess the only difference is if you have a series of transfers, where only the last transfer has @cs_change=true, and all preceding messages have @cs_change=false. In this case, @delay_ns would apply between each transfer, and after the last transfer @delay_ns + @cs_hold would apply before CS is deasserted:

     Transfer[0] (cs_change=false)
     delay(@delay_ns)
     Transfer[1] (cs_change=false)
     delay(@delay_ns)
     Transfer[3] (cs_change=true)
     delay(@delay_ns) + delay(@cs_hold_ns)
     CS deasserted

Quoted 4 lines

@cs_change_delay_ns: delay between cs deassert and assert when
cs_change is set, in ns unit
@cs_inactive_ns: delay to be introduced by the controller after CS
is deasserted, in ns unit

Similarly, aren't @cs_change_delay_ns and @cs_inactive_ns applied at the same point in the cycle?

(This is the period labeled "E" in my diagram.)

Linux spi_device defines @cs_inactive as: delay to be introduced by the
controller after CS is deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

...and spi_transfer defines @cs_change_delay as: delay between cs
deassert and assert when @cs_change is set and @spi_transfer is not the last in @spi_message.

It even says they will be added up.

The only difference seems to be that @cs_change_delay applies only when the transfer is not the last in a list (spi_message, which we don't have here.)

CS asserted
Transfer[0] (cs_change=true)
CS deasserted
delay(@cs_inactive) + delay(@cs_change_delay) CS asserted
Transfer[1] (cs_change=true)
CS deasserted
delay(@cs_inactive)

In both cases of redundancy above, the difference in semantics between the spi_device and spi_transfer only seem to apply when a series of transfers (spi_message or SPI_IOC_MESSAGE) are involved. Since (AFAICT) you aren't proposing that construct here, maybe the separate fields are not necessary? IOW, maybe @delay_ns and @cs_change_delay can be eliminated?

Please let me know if I've misunderstood anything.

Jonathon

Quoted 189 lines

Could you please help check if this new structure contains enough
information. Really appreciate for kind help.

Best Regards
Haixu Cui

Quoted 181 lines · Jonathon Reinhart

Quoted 29 lines

Hi @[email protected],
Thank you very much for your helpful comments.

I missed the delay and cs_change_delay parameters. I will add both
of them, although cs_change_delay can not be set from userspace, but can be set in kernel space.

For CS delays such as PRE_DELAY/POST_DELAY, it seems that they are
defined in structure spi_device:

@cs_setup: delay to be introduced by the controller after CS is
asserted.

@cs_hold: delay to be introduced by the controller before CS is
deasserted.

@cs_inactive: delay to be introduced by the controller after CS is
deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

They show the SPI controller ability to control the CS
assertion/deassertion timing and should not be changed for each transfer (because thay can be updated by setting structure spi_transfer or structure spi_ioc_transfer). I think it better to define these parameter in host OS rather than in guest OS since it's the host OS to operate the hardware SPI controller directly. Besides, it can also avoid passing the same values from guest to host time after time.

Quoted 1 line

What's your opinion on this topic? Again, thank you very much.

Hi Haixu,

I took another look at the Linux structures and attempted to map up the delay parameters to the various points in a SPI sequence. Please correct me if I'm wrong about any of this.

Consider this diagram where CS# is an active-low chip select, and SCLK is the serial clock:

        ___.                                                   ._____.
    CS#    |___________________________________________________|     |___
           .                                                   .     .
           .                                                   .     .
   SCLK ________NNNN___NNNN___NNNN_______NNNN___NNNN___NNNN______________
           .   .    . .           .     .                  .   .     .
Delays:  +-A-+    +B+           +--C--+                  +-D-+--E--+
           .   .    . .           .     .                  .   .     .
           0   1    2 3           4     5                  6   7     8
           .   .    .             .     .                  .   .
Terms:   .   +Word+             .     .                  .   .
           .   .                  .     .                  .   .
           .   +-------Frame------+     +-------Frame------+   .
           .                                                   .
           +-----------------Transfer--------------------------+

Using NXP terminology:

From 1 to 2 is a "word" (with e.g. 8 bits).
From 1 to 4 is a "frame" (with 3 words).
From 1 to 6 is a "transfer" (with 3 frames).

Linux does not have the concept of a frame, only a series of transfers (a spi_message), each of which can specify whether CS changes or not.

I've identified these various timing points and attempted to match them to the Linux spi_device and spi_transfer parameters:

A - CS Setup: Delay after CS asserted before clock starts.
spi_device.cs_setup

B - Word Delay: Delay between words.
spi_transfer.word_delay

C - Frame Delay: Delay between frames.
(Linux does not have the concept of a SPI "frame".)

D - CS Hold: Delay after clock stops, and before CS deasserted.
spi_device.cs_hold (+ spi_transfer.delay ??)

E - CS Inactive: Delay after CS deasserted, before asserting again.
spi_device.cs_inactive + spi_transfer.cs_change_delay

While I agree with you that some of these timings are unlikely to change from transfer-to-transfer, the subset of which should be specified at the device level vs. per-transfer seems somewhat arbitrary. As you can see there is some overlap between them.

If I understand correctly, it appears that the CS hold time *can* be controlled on a per-transfer bases, using spi_transfer.delay ("after this transfer before changing the chipselect status").

That leaves CS setup as the only parameter that cannot be influenced on a per-transfer basis.

Theoretically, one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus). In that case, one must set those spi_device parameters to the worst-case (maximum) values. However, this is already a Linux limitation, so I'm not sure it needs to be improved here.

I think you've achieved parity with what the Linux kernel API allows, and that's probably good enough. As you said, anything else can be adjusted in the host OS -- I'm not sure how the guest would go about achieving that, though. You're not proposing the use of configuration space for virtio-spi, are you?

Regards,
Jonathon Reinhart

Quoted 69 lines

Best Regards
Haixu Cui

Quoted 64 lines · On 6/28/2023 1:06 AM, [email protected] wrote:

Hi Haixu,

Quoted 3 lines

+The \field{word_delay} defines how long to wait between words within one SPI transfer,
+in ns unit.

I'm a little surprised to see a word_delay but no frame_delay or transfer_delay.

For example, many SPI peripherals require a delay after CS is asserted, but before the first SCLK edge, allowing them to prepare to send data. (E.g. an ADC might begin taking a sample.)

The linux struct spi_transfer has three delay fields:

* @cs_change_delay: delay between cs deassert and assert when
* @cs_change is set and @spi_transfer is not the last in
* @spi_message
* @delay: delay to be introduced after this transfer before
* (optionally) changing the chipselect status, then starting the
* next transfer or completing this @spi_message.
* @word_delay: inter word delay to be introduced after each word size
* (set by bits_per_word) transmission.

The userspace spidev.h API has only two:

* @delay_usecs: If nonzero, how long to delay after the last bit
* transfer before optionally deselecting the device before the
* next transfer.
* @word_delay_usecs: If nonzero, how long to wait between words within
* one transfer. This property needs explicit support in the SPI
* controller, otherwise it is silently ignored.

The NXP i.MX RT500 SPI (master) controller, for example, has four configurable delays:

- PRE_DELAY: After CS assertion, before first SCLK edge.
- POST_DELAY: After a transfer, before CS deassertion.
- FRAME_DELAY: Between frames (which are arbitrarily defined by software).
- TRANSFER_DELAY: Minimum CS deassertion time between transfers.

The NVIDIA Tegra114 SPI controller has:

- nvidia,cs-setup-clk-count: CS setup timing parameter.
- nvidia,cs-hold-clk-count: CS hold timing parameter.

I understand that accurately representing all possible delays might be difficult or futile, but I'm curious to understand why, of all the possible delays, inter-word delay was chosen for inclusion.

In a microcontroller, delays around CS (de)assertion can be customized by using a GPIO -- rather than the hardware SPI block -- to drive the CS signal. This way, delays can be inserted where needed. To do so with a virtualized SPI controller might prove difficult however, as the virtual channel carrying a CS GPIO signal might not be synchronized to the channel carrying the SPI data.

Curious to hear your thoughts.

Thanks,
Jonathon Reinhart

Quoted 80 lines · Haixu Cui

Hi Jonathon Reinhart,
Thank you very much for your comments.

Quoted 75 lines · Jonathon Reinhart

Quoted 40 lines · Haixu Cui

Hi Jonathon Reinhart,

Thank you so much for all your helpful advice and info!

I took a look at latest Linux SPI driver and found, for
cs_setup/cs_hold/cs_inactive set in device-tree, there are following two conditions:
1) if SPI controller supports CS timing configuration and CS is not
using a GPIO line, then the SPI driver set the cs_setup/cs_hold/ca_inactive values into the corresponding SPI CS registers;
2) if CS is using a GPIO line, or SPI controller doesn't support CS
timing configuration, it is the software to perform the cs_setup/cs_hold/cs_inactive delays, which is implemented in function spi_set_cs in driver/spi/spi.c.

So I agree it is necessary to pass cs_setup/cs_hold/cs_inactive to
the host for each transfer.

For condition 1, host should set these values into the CS timing
registers. And as you mentioned "one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus)", if so, host need to overwrite the CS timing registers between the two transfers talking to different salve_id.

For condition 2, SPI driver running in the host performing the
cs_setup/cs_hold/cs_inactive delays seems more accurate, compared with performing delays in guest.

       And the latest virtio spi transfer structure is defined as follows:

        struct spi_transfer_head {
           u8 slave_id;
           u8 bits_per_word;
           u8 cs_change;
           u8 tx_nbits;
          u8 rx_nbits;
          u8 reserved[3];
           u32 mode;

Quoted 9 lines

           u32 freq;
           u32 word_delay_ns;
           u32 delay_ns;
           u32 cs_change_delay_ns;
           u32 cs_setup_ns;
           u32 cs_hold_ns;
           u32 cs_inactive_ns;
        };

Hello Haixu Cui,

I think there may be some unnecessary redundancy in these fields. See below.

Quoted 4 lines

@slave_id: chipselect index the SPI transfer used @bits_per_word: the number of bits in each SPI transfer word @cs_change: True to deselect device before starting the next transfer @tx_nbits: number of bits used for writing

Quoted 8 lines

@rx_nbits: number of bits used for reading
@reserved: reserved for future use
@mode: the spi mode defines how data is clocked out and in @freq: transfer speed
@word_delay_ns: delay to be inserted between consecutive words of
a transfer, in ns unit
@cs_setup_ns: delay to be introduced by the controller after CS is
asserted, in ns unit

(I am reordering these in my quoted text to be grouped appropriately.)

Quoted 4 lines

@delay_ns: delay to be introduced after this transfer before
(optionally) changing the chipselect status, in ns unit
@cs_hold_ns: delay to be introduced by the controller before CS is
deasserted, in ns unit

Quoted 5 lines

Quoted 1 line

Aren't @delay_ns and @cs_hold_ns specifying the same thing?

I think they are not the same thing, delay_ns is the spi transfer property while cs_hold_ns is the spi device property， although they take effect at the same stage.

I see. I think we have differing perspectives here.

I assume that you are looking at this from the perspective of a Linux guest on a Linux host, where virtio-spi is connecting a virtual spi driver in the guest to a hardware spi driver on the host. In this case, it makes sense to have parity between the fields in Linux spi_device / spi_transfer and the fields in the virtio spi_transfer_head. It makes the implementation easier if there is a simple 1:1 mapping between them, even if you have to copy some spi_transfer_head fields to spi_device (like the cs_ fields).

My interest in virtio-spi is actually a bit different. We are looking to connect a virtual spi driver in a guest running Linux to another instance of QEMU emulating a baremetal firmware image. For a use-case like mine, the redundancy in the fields complicates things slightly. Because there is no physical SPI controller represented by the spi_device, all fields on spi_transfer_head have to be implemented in software -- including adding together the delays when appropriate (@delay_ns + @cs_hold_ns, and @cs_change_delay_ns + @cs_inactive_ns).

It's not a deal-breaker, but I wanted you to consider the non-Linux perspective. From a purely protocol-centric view, I think the fields can still be seen as redundant.

Quoted 315 lines

I list 4 conditions, here transfer cs delay including spi_transfer->delay and spi_transfer->cs_change_delay, and device cs delay including spi_device->cs_hold, spi_device->cs_setup and spi_device->cs_inactive.

condition 1:
virtio-spi controller only has transfer_one interface but no
transfer_one_message interface, and spi_transfer_one_message is the default transfer_one_message interface of virtio-spi controller.
Hardware SPI controller doesn't support CS timing configuration.

In this case, both transfer cs delay and device cs delay are
executed by software in spi_transfer_one_message function in guest Linux OS. So guest doesn't need pass these cs timing parameters to host.

condition 2:
virtio-spi controller only has transfer_one interface but no
transfer_one_message interface, and spi_transfer_one_message is the default transfer_one_message interface of virtio-spi controller.
Hardware SPI controller supports device CS timing configuration,
which means it has registers to hold these configuration values.

In this case, transfer cs delay is executed in
spi_transfer_one_message function, and guest need pass device cs delay to host. Because one SPI number may have more than one slave, these slaves also may have different device cs delay values, so for each transfer, guest should pass device cs delay along with the slave_id to host to overwrite the corresponding registers.

condition 3:
virtio-spi controller has transfer_one_message interface but no
transfer_one interface.
Hardware SPI controller doesn't support CS timing configuration.

In this case, host SPI driver should implement all cs delay
operations with the transfer cs delay and device cs delay received from the guest.

condition 4:
virtio-spi controller has transfer_one_message interface but no
transfer_one interface.
Hardware SPI controller supports CS timing configuration.

In this case, guest pass transfer cs delay and device cs delay to
guest, and guest SPI driver should implement logic to execute transfer cs delay then overwrite the corresponding register with the device cs delay values.

Except for condition 1, in other three conditions, guest should pass both transfer cs delay and device cs delay to host.

Best Regards
Haixu Cui

Quoted 260 lines

Linux spi_transfer defines @delay as:
delay to be introduced after this transfer before (optionally) changing the chipselect status, then starting the next transfer or completing this spi_message.

...and spi_device @cs_hold as: delay to be introduced by the
controller before CS is deasserted.

(This is the period labeled "D" in my diagram.)

I guess the only difference is if you have a series of transfers, where only the last transfer has @cs_change=true, and all preceding messages have @cs_change=false. In this case, @delay_ns would apply between each transfer, and after the last transfer @delay_ns + @cs_hold would apply before CS is deasserted:

      Transfer[0] (cs_change=false)
      delay(@delay_ns)
      Transfer[1] (cs_change=false)
      delay(@delay_ns)
      Transfer[3] (cs_change=true)
      delay(@delay_ns) + delay(@cs_hold_ns)
      CS deasserted

Quoted 4 lines

@cs_change_delay_ns: delay between cs deassert and assert when
cs_change is set, in ns unit
@cs_inactive_ns: delay to be introduced by the controller after CS
is deasserted, in ns unit

Similarly, aren't @cs_change_delay_ns and @cs_inactive_ns applied at the same point in the cycle?

(This is the period labeled "E" in my diagram.)

Linux spi_device defines @cs_inactive as: delay to be introduced by the
controller after CS is deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

...and spi_transfer defines @cs_change_delay as: delay between cs
deassert and assert when @cs_change is set and @spi_transfer is not the last in @spi_message.

It even says they will be added up.

The only difference seems to be that @cs_change_delay applies only when the transfer is not the last in a list (spi_message, which we don't have here.)

CS asserted
Transfer[0] (cs_change=true)
CS deasserted
delay(@cs_inactive) + delay(@cs_change_delay) CS asserted
Transfer[1] (cs_change=true)
CS deasserted
delay(@cs_inactive)

In both cases of redundancy above, the difference in semantics between the spi_device and spi_transfer only seem to apply when a series of transfers (spi_message or SPI_IOC_MESSAGE) are involved. Since (AFAICT) you aren't proposing that construct here, maybe the separate fields are not necessary? IOW, maybe @delay_ns and @cs_change_delay can be eliminated?

Please let me know if I've misunderstood anything.

Jonathon

Quoted 189 lines

Could you please help check if this new structure contains enough
information. Really appreciate for kind help.

Best Regards
Haixu Cui

Quoted 181 lines · Jonathon Reinhart

Quoted 29 lines

Hi @[email protected],
Thank you very much for your helpful comments.

I missed the delay and cs_change_delay parameters. I will add both
of them, although cs_change_delay can not be set from userspace, but can be set in kernel space.

For CS delays such as PRE_DELAY/POST_DELAY, it seems that they are
defined in structure spi_device:

@cs_setup: delay to be introduced by the controller after CS is
asserted.

@cs_hold: delay to be introduced by the controller before CS is
deasserted.

@cs_inactive: delay to be introduced by the controller after CS is
deasserted. If @cs_change_delay is used from @spi_transfer, then the two delays will be added up.

They show the SPI controller ability to control the CS
assertion/deassertion timing and should not be changed for each transfer (because thay can be updated by setting structure spi_transfer or structure spi_ioc_transfer). I think it better to define these parameter in host OS rather than in guest OS since it's the host OS to operate the hardware SPI controller directly. Besides, it can also avoid passing the same values from guest to host time after time.

Quoted 1 line

What's your opinion on this topic? Again, thank you very much.

Hi Haixu,

I took another look at the Linux structures and attempted to map up the delay parameters to the various points in a SPI sequence. Please correct me if I'm wrong about any of this.

Consider this diagram where CS# is an active-low chip select, and SCLK is the serial clock:

         ___.                                                   ._____.
     CS#    |___________________________________________________|     |___
            .                                                   .     .
            .                                                   .     .
    SCLK ________NNNN___NNNN___NNNN_______NNNN___NNNN___NNNN______________
            .   .    . .           .     .                  .   .     .
Delays:  +-A-+    +B+           +--C--+                  +-D-+--E--+
            .   .    . .           .     .                  .   .     .
            0   1    2 3           4     5                  6   7     8
            .   .    .             .     .                  .   .
Terms:   .   +Word+             .     .                  .   .
            .   .                  .     .                  .   .
            .   +-------Frame------+     +-------Frame------+   .
            .                                                   .
            +-----------------Transfer--------------------------+

Using NXP terminology:

From 1 to 2 is a "word" (with e.g. 8 bits).
From 1 to 4 is a "frame" (with 3 words).
From 1 to 6 is a "transfer" (with 3 frames).

Linux does not have the concept of a frame, only a series of transfers (a spi_message), each of which can specify whether CS changes or not.

I've identified these various timing points and attempted to match them to the Linux spi_device and spi_transfer parameters:

A - CS Setup: Delay after CS asserted before clock starts.
spi_device.cs_setup

B - Word Delay: Delay between words.
spi_transfer.word_delay

C - Frame Delay: Delay between frames.
(Linux does not have the concept of a SPI "frame".)

D - CS Hold: Delay after clock stops, and before CS deasserted.
spi_device.cs_hold (+ spi_transfer.delay ??)

E - CS Inactive: Delay after CS deasserted, before asserting again.
spi_device.cs_inactive + spi_transfer.cs_change_delay

While I agree with you that some of these timings are unlikely to change from transfer-to-transfer, the subset of which should be specified at the device level vs. per-transfer seems somewhat arbitrary. As you can see there is some overlap between them.

If I understand correctly, it appears that the CS hold time *can* be controlled on a per-transfer bases, using spi_transfer.delay ("after this transfer before changing the chipselect status").

That leaves CS setup as the only parameter that cannot be influenced on a per-transfer basis.

Theoretically, one might require different CS setup/hold times, depending on which slave_id they are talking to (on the same bus). In that case, one must set those spi_device parameters to the worst-case (maximum) values. However, this is already a Linux limitation, so I'm not sure it needs to be improved here.

I think you've achieved parity with what the Linux kernel API allows, and that's probably good enough. As you said, anything else can be adjusted in the host OS -- I'm not sure how the guest would go about achieving that, though. You're not proposing the use of configuration space for virtio-spi, are you?

Regards,
Jonathon Reinhart

Quoted 69 lines

Best Regards
Haixu Cui

Quoted 64 lines · On 6/28/2023 1:06 AM, [email protected] wrote:

Hi Haixu,

Quoted 3 lines

+The \field{word_delay} defines how long to wait between words within one SPI transfer,
+in ns unit.

I'm a little surprised to see a word_delay but no frame_delay or transfer_delay.

For example, many SPI peripherals require a delay after CS is asserted, but before the first SCLK edge, allowing them to prepare to send data. (E.g. an ADC might begin taking a sample.)

The linux struct spi_transfer has three delay fields:

* @cs_change_delay: delay between cs deassert and assert when
* @cs_change is set and @spi_transfer is not the last in
* @spi_message
* @delay: delay to be introduced after this transfer before
* (optionally) changing the chipselect status, then starting the
* next transfer or completing this @spi_message.
* @word_delay: inter word delay to be introduced after each word size
* (set by bits_per_word) transmission.

The userspace spidev.h API has only two:

* @delay_usecs: If nonzero, how long to delay after the last bit
* transfer before optionally deselecting the device before the
* next transfer.
* @word_delay_usecs: If nonzero, how long to wait between words within
* one transfer. This property needs explicit support in the SPI
* controller, otherwise it is silently ignored.

The NXP i.MX RT500 SPI (master) controller, for example, has four configurable delays:

- PRE_DELAY: After CS assertion, before first SCLK edge.
- POST_DELAY: After a transfer, before CS deassertion.
- FRAME_DELAY: Between frames (which are arbitrarily defined by software).
- TRANSFER_DELAY: Minimum CS deassertion time between transfers.

The NVIDIA Tegra114 SPI controller has:

- nvidia,cs-setup-clk-count: CS setup timing parameter.
- nvidia,cs-hold-clk-count: CS hold timing parameter.

I understand that accurately representing all possible delays might be difficult or futile, but I'm curious to understand why, of all the possible delays, inter-word delay was chosen for inclusion.

In a microcontroller, delays around CS (de)assertion can be customized by using a GPIO -- rather than the hardware SPI block -- to drive the CS signal. This way, delays can be inserted where needed. To do so with a virtualized SPI controller might prove difficult however, as the virtual channel carrying a CS GPIO signal might not be synchronized to the channel carrying the SPI data.

Curious to hear your thoughts.

Thanks,
Jonathon Reinhart