All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Kundrát" <jan.kundrat@cesnet.cz>
To: Baolin Wang <baolin.wang@linaro.org>, <broonie@kernel.org>,
	<linux-spi@vger.kernel.org>
Cc: <robh+dt@kernel.org>, <mark.rutland@arm.com>,
	<orsonzhai@gmail.com>, <zhang.lyra@gmail.com>,
	<lanqing.liu@spreadtrum.com>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] spi: Introduce one new field to set word delay
Date: Wed, 29 Aug 2018 13:33:24 +0200	[thread overview]
Message-ID: <01c9b188-5b67-40c0-b08d-99a0543ddf54@cesnet.cz> (raw)
In-Reply-To: <a77a6bb8b3abf1c127014c78f71533c24afacd93.1534423916.git.baolin.wang@linaro.org>

On čtvrtek 16. srpna 2018 14:54:49 CEST, Baolin Wang wrote:
> + * @word_delay: clock cycles to inter word delay after each word size
> + *	(set by bits_per_word) transmission.

I need a similar functionality for talking to a SPI device from userspace  
-- see my attempt for implementing this in spi-orion.c at [1]. The device's 
datasheet says that I should wait, e.g., 3µs between each two words. I 
therefore like this patch :).

The description can be improved because it left me wondering what "clock 
this is about. I suppose it's about the SPI clock cycles and not CPU clock 
cycles, right? I'll be hapy to patch this once Baolin confirms that that is 
the intended meaning.

It seems that this is only implemented in one newly added driver. I'm 
interested in supporting this in spi-orion.c, but that sounds like 
driver-specific work for something which is pretty generic. How should this 
be implemented? Given that drivers for SPI masters can implement a function 
which transfers several words at once, there are not that many better 
possibilities than adding udelay()s, though. Thoughts?

What is your plan to do with drivers which do not implement this (yet)? If 
a spi_transfer gets queued which asks for a word_delay delay, it is 
silently ignored now, AFAIU.

What about userspace support, spidev and spi_ioc_transfer (that's my 
target, actually)? Is it OK to s/pad/word_delay/ in the spidev code and 
pass that to the generated struct spi_transfer? In my opinion, once we 
support specifying this from userspace, one has to definitely check that 
the SPI controller is ready to honor this request. Do we want a new bit in 
spi_controller.flags for this?

With kind regards,
Jan

[1] https://patchwork.kernel.org/patch/10221397/

WARNING: multiple messages have this Message-ID (diff)
From: "Jan Kundrát" <jan.kundrat@cesnet.cz>
To: Baolin Wang <baolin.wang@linaro.org>,
	broonie@kernel.org, linux-spi@vger.kernel.org
Cc: robh+dt@kernel.org, mark.rutland@arm.com, orsonzhai@gmail.com,
	zhang.lyra@gmail.com, lanqing.liu@spreadtrum.com,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] spi: Introduce one new field to set word delay
Date: Wed, 29 Aug 2018 13:33:24 +0200	[thread overview]
Message-ID: <01c9b188-5b67-40c0-b08d-99a0543ddf54@cesnet.cz> (raw)
In-Reply-To: <a77a6bb8b3abf1c127014c78f71533c24afacd93.1534423916.git.baolin.wang@linaro.org>

On čtvrtek 16. srpna 2018 14:54:49 CEST, Baolin Wang wrote:
> + * @word_delay: clock cycles to inter word delay after each word size
> + *	(set by bits_per_word) transmission.

I need a similar functionality for talking to a SPI device from userspace  
-- see my attempt for implementing this in spi-orion.c at [1]. The device's 
datasheet says that I should wait, e.g., 3µs between each two words. I 
therefore like this patch :).

The description can be improved because it left me wondering what "clock 
this is about. I suppose it's about the SPI clock cycles and not CPU clock 
cycles, right? I'll be hapy to patch this once Baolin confirms that that is 
the intended meaning.

It seems that this is only implemented in one newly added driver. I'm 
interested in supporting this in spi-orion.c, but that sounds like 
driver-specific work for something which is pretty generic. How should this 
be implemented? Given that drivers for SPI masters can implement a function 
which transfers several words at once, there are not that many better 
possibilities than adding udelay()s, though. Thoughts?

What is your plan to do with drivers which do not implement this (yet)? If 
a spi_transfer gets queued which asks for a word_delay delay, it is 
silently ignored now, AFAIU.

What about userspace support, spidev and spi_ioc_transfer (that's my 
target, actually)? Is it OK to s/pad/word_delay/ in the spidev code and 
pass that to the generated struct spi_transfer? In my opinion, once we 
support specifying this from userspace, one has to definitely check that 
the SPI controller is ready to honor this request. Do we want a new bit in 
spi_controller.flags for this?

With kind regards,
Jan

[1] https://patchwork.kernel.org/patch/10221397/

  parent reply	other threads:[~2018-08-29 11:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-16 12:54 [PATCH v2 1/3] spi: Introduce one new field to set word delay Baolin Wang
2018-08-16 12:54 ` [PATCH v2 2/3] dt-bindings: spi: Add Spreadtrum SPI controller documentation Baolin Wang
2018-08-20 19:38   ` Rob Herring
2018-08-20 19:38     ` Rob Herring
2018-08-16 12:54 ` [PATCH v2 3/3] spi: sprd: Add SPI driver for Spreadtrum SC9860 Baolin Wang
2018-08-28 20:57   ` Applied "spi: sprd: Add SPI driver for Spreadtrum SC9860" to the spi tree Mark Brown
2018-08-28 20:57     ` Mark Brown
2018-08-28 20:57     ` Mark Brown
2018-08-29 11:02 ` Applied "spi: Introduce one new field to set word delay" " Mark Brown
2018-08-29 11:02   ` Mark Brown
2018-08-29 11:02   ` Mark Brown
2018-08-29 11:33 ` Jan Kundrát [this message]
2018-08-29 11:33   ` [PATCH v2 1/3] spi: Introduce one new field to set word delay Jan Kundrát
2018-08-29 11:47   ` Mark Brown
2018-08-31  8:38   ` Baolin Wang

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=01c9b188-5b67-40c0-b08d-99a0543ddf54@cesnet.cz \
    --to=jan.kundrat@cesnet.cz \
    --cc=baolin.wang@linaro.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lanqing.liu@spreadtrum.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=orsonzhai@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=zhang.lyra@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.