virtio-comment.lists.oasis-open.org archive mirror
 help / color / mirror / Atom feed
From: Haixu Cui <quic_haixcui@quicinc.com>
To: <jrreinhart@google.com>
Cc: Cornelia Huck <cohuck@redhat.com>, <quic_ztu@quicinc.com>,
	<virtio-dev@lists.oasis-open.org>,
	<virtio-comment@lists.oasis-open.org>
Subject: [virtio-comment] Re: [virtio-dev][PATCH 2/2] virtio-spi: add the device specification
Date: Wed, 28 Jun 2023 17:44:34 +0800	[thread overview]
Message-ID: <fd2e0d5d-a3e9-027a-bec1-d0e821ec6421@quicinc.com> (raw)
In-Reply-To: <000000000000925c1305ff1f7c76@google.com>

Hi @jrreinhart@google.com,
     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

On 6/28/2023 1:06 AM, jrreinhart@google.com wrote:
> Hi Haixu,
> 
>> +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

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


       reply	other threads:[~2023-06-28  9:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <000000000000925c1305ff1f7c76@google.com>
2023-06-28  9:44 ` Haixu Cui [this message]
     [not found]   ` <CAJJa5HxA6T+xv--80aWEtGWNx=a-yfNXwNsnyz3s8praHqg6Ew@mail.gmail.com>
2023-07-07  7:21     ` [virtio-comment] Re: [virtio-dev][PATCH 2/2] virtio-spi: add the device specification Haixu Cui
     [not found]       ` <CAJJa5HwV76co8n4WeeL-mcMwwL9sKVWz_Ejiv-EMWgemsGGS3w@mail.gmail.com>
2023-07-17 14:53         ` Haixu Cui
     [not found]           ` <CAJJa5HxvVFtcjX=PVw5UnAStG=GiDRUkEr_LRbyAmwvfAMTFwA@mail.gmail.com>
2023-07-20  5:45             ` Haixu Cui
     [not found] <20230417140353.870-1-quic_haixcui@quicinc.com>
     [not found] ` <20230417140353.870-3-quic_haixcui@quicinc.com>
     [not found]   ` <98b247c1-b970-72ba-6888-f83144f08116@opensynergy.com>
2023-07-28  6:53     ` Haixu Cui

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=fd2e0d5d-a3e9-027a-bec1-d0e821ec6421@quicinc.com \
    --to=quic_haixcui@quicinc.com \
    --cc=cohuck@redhat.com \
    --cc=jrreinhart@google.com \
    --cc=quic_ztu@quicinc.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio-dev@lists.oasis-open.org \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).