[PATCH 0/5] allow to define cs deassert times in us, ns and SCK-len

[PATCH 0/5] allow to define cs deassert times in us, ns and SCK-len

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Using spi_transfer.cs_change right now means that there is a hard-coded
10 us delay between deassert and re-assert of cs.

For some devices this is way too long and either wastes resources
unecessarily or the driver works arround this currently with multiple
spi messages.

So this patch set modifies the spie core and some drivers so that
in spi_transfer we now can now define the minimum delay in units of:
microseconds, nanoseconds or spi clock length.

But note that on lower specs machines those ns delays are not realistic.
Lower limits on the actual delay are put by the gpio framework latency
for setting gpio level.
On a Raspberry Pi 3 this overhead has been observed to be in the order
1.2us.

As an idea: the xfer->cs_change_delay_unit could become more generic
by renaming it to delay_unit and then also us it as a modifier when
using xfer->delay_usecs.

Martin Sperl (5):
  spi: core: allow defining time that cs is deasserted
  spi: core: allow reporting the effectivly used speed_hz for a transfer
  spi: core: allow defining time that cs is deasserted as a multiple of
    SCK
  spi: bcm2835: support effective_speed_hz
  spi: bcm2835aux: support effective_speed_hz

 drivers/spi/spi-bcm2835.c    |  5 ++--
 drivers/spi/spi-bcm2835aux.c |  5 ++--
 drivers/spi/spi.c            | 68 +++++++++++++++++++++++++++++++++++++-------
 include/linux/spi/spi.h      | 13 +++++++++
 4 files changed, 75 insertions(+), 16 deletions(-)

--
2.11.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] spi: core: allow defining time that cs is deasserted

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

For some SPI devices that support speed_hz > 1MHz the default 10 us delay
when cs_change = 1 is typically way to long and may result in poor spi bus
utilization.

This patch makes it possible to control the delay at micro or nano second
resolution on a per spi_transfer basis. It even allows an "as fast as
possible" mode with:
    xfer.cs_change_delay_unit = SPI_DELAY_UNIT_NSECS;
    xfer.cs_change_delay = 0;

The delay code is shared between delay_usecs and cs_change_delay for
consistency and reuse, so in the future this change_delay_unit could also
apply to delay_usec as well.

Note that on slower SOCs/CPU actually reaching ns deasserts on cs is not
realistic as the gpio overhead alone (without any delays added ) may
already leave cs deasserted for more than 1us - at least on a raspberry pi.
But at the very least this way we can keep it as short as possible.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/spi/spi.c       | 59 ++++++++++++++++++++++++++++++++++++++++---------
 include/linux/spi/spi.h |  7 ++++++
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9a7def7c3237..90659eb29660 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1071,6 +1071,52 @@ static int spi_transfer_wait(struct spi_controller *ctlr,
 	return 0;
 }

+static void _spi_transfer_delay_ns(u32 ns)
+{
+	if (!ns)
+		return;
+	if (ns <= 1000) {
+		ndelay(ns);
+	} else {
+		u32 us = DIV_ROUND_UP(ns, 1000);
+
+		if (us <= 10)
+			udelay(us);
+		else
+			usleep_range(us, us + DIV_ROUND_UP(us, 10));
+	}
+}
+
+static void _spi_transfer_cs_change_delay(struct spi_message *msg,
+					  struct spi_transfer *xfer)
+{
+	u32 delay = xfer->cs_change_delay;
+	u32 unit = xfer->cs_change_delay_unit;
+
+	/* return early on "fast" mode - for everything but USECS */
+	if (!delay && unit != SPI_DELAY_UNIT_USECS)
+		return;
+
+	switch (unit) {
+	case SPI_DELAY_UNIT_USECS:
+		/* for compatibility use default of 10us */
+		if (!delay)
+			delay = 10000;
+		else
+			delay *= 1000;
+		break;
+	case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
+		break;
+	default:
+		dev_err_once(&msg->spi->dev,
+			     "Use of unsupported delay unit %i, using default of 10us\n",
+			     xfer->cs_change_delay_unit);
+		delay = 10000;
+	}
+	/* now sleep for the requested amount of time */
+	_spi_transfer_delay_ns(delay);
+}
+
 /*
  * spi_transfer_one_message - Default implementation of transfer_one_message()
  *
@@ -1129,14 +1175,8 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 		if (msg->status != -EINPROGRESS)
 			goto out;

-		if (xfer->delay_usecs) {
-			u16 us = xfer->delay_usecs;
-
-			if (us <= 10)
-				udelay(us);
-			else
-				usleep_range(us, us + DIV_ROUND_UP(us, 10));
-		}
+		if (xfer->delay_usecs)
+			_spi_transfer_delay_ns(xfer->delay_usecs * 1000);

 		if (xfer->cs_change) {
 			if (list_is_last(&xfer->transfer_list,
@@ -1144,7 +1184,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 				keep_cs = true;
 			} else {
 				spi_set_cs(msg->spi, false);
-				udelay(10);
+				_spi_transfer_cs_change_delay(msg, xfer);
 				spi_set_cs(msg->spi, true);
 			}
 		}
@@ -3642,4 +3682,3 @@ static int __init spi_init(void)
  * include needing to have boardinfo data structures be much more public.
  */
 postcore_initcall(spi_init);
-
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 314d922ca607..f2ce1fb403ef 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -703,6 +703,9 @@ extern void spi_res_release(struct spi_controller *ctlr,
  * @bits_per_word: select a bits_per_word other than the device default
  *      for this transfer. If 0 the default (from @spi_device) is used.
  * @cs_change: affects chipselect after this transfer completes
+ * @cs_change_delay: delay between cs deassert and assert when
+ *      @cs_change is set and @spi_transfer is not the last in @spi_message
+ * @cs_change_delay_unit: unit of cs_change_delay
  * @delay_usecs: microseconds to delay after this transfer before
  *	(optionally) changing the chipselect status, then starting
  *	the next transfer or completing this @spi_message.
@@ -789,6 +792,10 @@ struct spi_transfer {
 #define	SPI_NBITS_QUAD		0x04 /* 4bits transfer */
 	u8		bits_per_word;
 	u16		delay_usecs;
+	u16		cs_change_delay;
+	u8		cs_change_delay_unit;
+#define SPI_DELAY_UNIT_USECS	0
+#define SPI_DELAY_UNIT_NSECS	1
 	u32		speed_hz;
 	u16		word_delay;

--
2.11.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Applied "spi: core: allow defining time that cs is deasserted" to the spi tree

[Mark Brown]

The patch

   spi: core: allow defining time that cs is deasserted

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.3

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 0ff2de8bb163551ec4230a5a6f3c40c1f6adec4f Mon Sep 17 00:00:00 2001
From: Martin Sperl <kernel@martin.sperl.org>
Date: Sat, 23 Feb 2019 08:49:48 +0000
Subject: [PATCH] spi: core: allow defining time that cs is deasserted

For some SPI devices that support speed_hz > 1MHz the default 10 us delay
when cs_change = 1 is typically way to long and may result in poor spi bus
utilization.

This patch makes it possible to control the delay at micro or nano second
resolution on a per spi_transfer basis. It even allows an "as fast as
possible" mode with:
    xfer.cs_change_delay_unit = SPI_DELAY_UNIT_NSECS;
    xfer.cs_change_delay = 0;

The delay code is shared between delay_usecs and cs_change_delay for
consistency and reuse, so in the future this change_delay_unit could also
apply to delay_usec as well.

Note that on slower SOCs/CPU actually reaching ns deasserts on cs is not
realistic as the gpio overhead alone (without any delays added ) may
already leave cs deasserted for more than 1us - at least on a raspberry pi.
But at the very least this way we can keep it as short as possible.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi.c       | 59 ++++++++++++++++++++++++++++++++++-------
 include/linux/spi/spi.h |  7 +++++
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 5e75944ad5d1..7e8ffe3fdc00 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1090,6 +1090,52 @@ static int spi_transfer_wait(struct spi_controller *ctlr,
 	return 0;
 }
 
+static void _spi_transfer_delay_ns(u32 ns)
+{
+	if (!ns)
+		return;
+	if (ns <= 1000) {
+		ndelay(ns);
+	} else {
+		u32 us = DIV_ROUND_UP(ns, 1000);
+
+		if (us <= 10)
+			udelay(us);
+		else
+			usleep_range(us, us + DIV_ROUND_UP(us, 10));
+	}
+}
+
+static void _spi_transfer_cs_change_delay(struct spi_message *msg,
+					  struct spi_transfer *xfer)
+{
+	u32 delay = xfer->cs_change_delay;
+	u32 unit = xfer->cs_change_delay_unit;
+
+	/* return early on "fast" mode - for everything but USECS */
+	if (!delay && unit != SPI_DELAY_UNIT_USECS)
+		return;
+
+	switch (unit) {
+	case SPI_DELAY_UNIT_USECS:
+		/* for compatibility use default of 10us */
+		if (!delay)
+			delay = 10000;
+		else
+			delay *= 1000;
+		break;
+	case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
+		break;
+	default:
+		dev_err_once(&msg->spi->dev,
+			     "Use of unsupported delay unit %i, using default of 10us\n",
+			     xfer->cs_change_delay_unit);
+		delay = 10000;
+	}
+	/* now sleep for the requested amount of time */
+	_spi_transfer_delay_ns(delay);
+}
+
 /*
  * spi_transfer_one_message - Default implementation of transfer_one_message()
  *
@@ -1148,14 +1194,8 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 		if (msg->status != -EINPROGRESS)
 			goto out;
 
-		if (xfer->delay_usecs) {
-			u16 us = xfer->delay_usecs;
-
-			if (us <= 10)
-				udelay(us);
-			else
-				usleep_range(us, us + DIV_ROUND_UP(us, 10));
-		}
+		if (xfer->delay_usecs)
+			_spi_transfer_delay_ns(xfer->delay_usecs * 1000);
 
 		if (xfer->cs_change) {
 			if (list_is_last(&xfer->transfer_list,
@@ -1163,7 +1203,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 				keep_cs = true;
 			} else {
 				spi_set_cs(msg->spi, false);
-				udelay(10);
+				_spi_transfer_cs_change_delay(msg, xfer);
 				spi_set_cs(msg->spi, true);
 			}
 		}
@@ -3757,4 +3797,3 @@ static int __init spi_init(void)
  * include needing to have boardinfo data structures be much more public.
  */
 postcore_initcall(spi_init);
-
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 053abd22ad31..023beb9e9e4b 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -735,6 +735,9 @@ extern void spi_res_release(struct spi_controller *ctlr,
  * @bits_per_word: select a bits_per_word other than the device default
  *      for this transfer. If 0 the default (from @spi_device) is used.
  * @cs_change: affects chipselect after this transfer completes
+ * @cs_change_delay: delay between cs deassert and assert when
+ *      @cs_change is set and @spi_transfer is not the last in @spi_message
+ * @cs_change_delay_unit: unit of cs_change_delay
  * @delay_usecs: microseconds to delay after this transfer before
  *	(optionally) changing the chipselect status, then starting
  *	the next transfer or completing this @spi_message.
@@ -824,6 +827,10 @@ struct spi_transfer {
 	u8		bits_per_word;
 	u8		word_delay_usecs;
 	u16		delay_usecs;
+	u16		cs_change_delay;
+	u8		cs_change_delay_unit;
+#define SPI_DELAY_UNIT_USECS	0
+#define SPI_DELAY_UNIT_NSECS	1
 	u32		speed_hz;
 	u16		word_delay;
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] spi: core: allow reporting the effectivly used speed_hz for a transfer

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Provide a means for the spi bus driver to report the effectively used
spi clock frequency used for each spi_transfer.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/spi/spi.c       | 1 +
 include/linux/spi/spi.h | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 90659eb29660..5a4616894d57 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3004,6 +3004,7 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 	 */
 	message->frame_length = 0;
 	list_for_each_entry(xfer, &message->transfers, transfer_list) {
+		xfer->effective_speed_hz = 0;
 		message->frame_length += xfer->len;
 		if (!xfer->bits_per_word)
 			xfer->bits_per_word = spi->bits_per_word;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f2ce1fb403ef..f503a712423d 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -711,6 +711,9 @@ extern void spi_res_release(struct spi_controller *ctlr,
  *	the next transfer or completing this @spi_message.
  * @word_delay: clock cycles to inter word delay after each word size
  *	(set by bits_per_word) transmission.
+ * @effective_speed_hz: the effective SCK-speed that was used to
+ *      transfer this transfer. Set to 0 if the spi bus driver does
+ *      not support it.
  * @transfer_list: transfers are sequenced through @spi_message.transfers
  * @tx_sg: Scatterlist for transmit, currently not for client use
  * @rx_sg: Scatterlist for receive, currently not for client use
@@ -799,6 +802,8 @@ struct spi_transfer {
 	u32		speed_hz;
 	u16		word_delay;

+	u32		effective_speed_hz;
+
 	struct list_head transfer_list;
 };

--
2.11.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] spi: core: allow defining time that cs is deasserted as a multiple of SCK

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Support setting a delay between cs assert and deassert as
a multiple of spi clock length.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/spi/spi.c       | 8 ++++++++
 include/linux/spi/spi.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 5a4616894d57..d5259bdd4b88 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1092,6 +1092,7 @@ static void _spi_transfer_cs_change_delay(struct spi_message *msg,
 {
 	u32 delay = xfer->cs_change_delay;
 	u32 unit = xfer->cs_change_delay_unit;
+	u32 hz;

 	/* return early on "fast" mode - for everything but USECS */
 	if (!delay && unit != SPI_DELAY_UNIT_USECS)
@@ -1107,6 +1108,13 @@ static void _spi_transfer_cs_change_delay(struct spi_message *msg,
 		break;
 	case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
 		break;
+	case SPI_DELAY_UNIT_SCK:
+		/* if there is no effective speed know, then approximate
+		 * by underestimating with half the requested hz
+		 */
+		hz = xfer->effective_speed_hz ?: xfer->speed_hz / 2;
+		delay *= DIV_ROUND_UP(1000000000, hz);
+		break;
 	default:
 		dev_err_once(&msg->spi->dev,
 			     "Use of unsupported delay unit %i, using default of 10us\n",
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f503a712423d..0b1d5e2b8b8b 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -799,6 +799,7 @@ struct spi_transfer {
 	u8		cs_change_delay_unit;
 #define SPI_DELAY_UNIT_USECS	0
 #define SPI_DELAY_UNIT_NSECS	1
+#define SPI_DELAY_UNIT_SCK	2
 	u32		speed_hz;
 	u16		word_delay;

--
2.11.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] spi: core: allow defining time that cs is deasserted as a multiple of SCK

[kernel]

Hi Lukas!

> On 23.02.2019, at 13:40, Lukas Wunner <lukas@wunner.de> wrote:
> 
> On Sat, Feb 23, 2019 at 08:49:50AM +0000, kernel@martin.sperl.org wrote:
>> Support setting a delay between cs assert and deassert as
>> a multiple of spi clock length.
> 
> To what end?
...
> 
> The ability to define a unit seems somewhat over-engineered to me.
> You're introducing lots of branching and integer divisions (both
> comparatively expensive operations) which are executed for every
> single transfer (i.e. in a hot path).  I'd recommend standardizing
> on a single unit.  If 1 usec is too coarse-grained for your use case,
> convert everything to nsec.

Well - a spi_device driver does not actually know what is the
actual spi clock configured/used by the spi-bus-driver.

All it knows is that the device requires X clock cycles of cs high.
So how would you define that so that is valid for all cases without
replicating code in every driver that needs it (everywhere slightly
differently)?

As for hot-path: we are not in the hot path - we need to waste time
when any of those parameters are set!

cs_change is not that commonly used - or cs_delay_usec for that matter.

The corresponding ifs int the hot path are in the code right now - 
worsted case we are jumping a few more cpu instructions further than we
would right now.

Also the code has been moved into a separate function and some of it
is used from multiple locations.

Thus it is up to the compiler to know best if it is worth inlining the 
function or just calling it.

From a CPU utilization perspective: for most situations we will
(most likely) be busy waiting to pass the required time anyway.
So those divides are just taking time from those busy cycles…

Finally if the computation yields a shorter value than a (conservative)
time-value that would have been given instead, then those computations
(divides)  have helped save cpu cycles and SPI bus bandwidth, so they
are beneficial.

As for the rpi-mailing-list: if this is an issue, then this is a
separate issue that needs to get addressed separately as it is not
really code-related.

Martin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] spi: core: allow defining time that cs is deasserted as a multiple of SCK

[kernel]

> On 24.02.2019, at 11:39, Lukas Wunner <lukas@wunner.de> wrote:
> 
> On Sat, Feb 23, 2019 at 02:15:13PM +0100, kernel@martin.sperl.org wrote:
>> On 23.02.2019, at 13:40, Lukas Wunner <lukas@wunner.de> wrote:
>>> On Sat, Feb 23, 2019 at 08:49:50AM +0000, kernel@martin.sperl.org wrote:
>>>> Support setting a delay between cs assert and deassert as
>>>> a multiple of spi clock length.
>>> 
>>> The ability to define a unit seems somewhat over-engineered to me.
>>> You're introducing lots of branching and integer divisions (both
>>> comparatively expensive operations) which are executed for every
>>> single transfer (i.e. in a hot path).  I'd recommend standardizing
>>> on a single unit.  If 1 usec is too coarse-grained for your use case,
>>> convert everything to nsec.
>> 
>> Well - a spi_device driver does not actually know what is the
>> actual spi clock configured/used by the spi-bus-driver.
> 
> Which slave device and driver are we talking about anyway?
> Is the driver actually making use of the ability to change the
> speed per-transfer?  Most slaves I've dealt with so far use a
> constant speed.  In that case the master driver could set the
> slave's ->speed_hz member to the effective speed on ->setup()
> and the slave's driver could then calculate the delay in nsec
> or usec based on that speed.

Unfortunately this is not always the case...

Some devices - like the mcp2517fd -  have for example an internal PLL
based on an external clock. So during setup you have to use speed_hz 
of <clock_hz> / 2 (or 4MHz at most) and only when PLL is in sync we 
may be using speed_hz from the dt (or less if a module parameter is
used to limit ourselves further)

So the initial setup would not be able to help here - and every
bus controller would now be required to implement setup.

It also means open coding the calculations in each driver that 
needs something like this.

Thus it is - imo - in the right location to support it in spi core.

This is also the reason why I was asking if the same argument
for a multiple of SCKs may apply also to delay_us.

An ADC spi-slave would be an example where this could be needed
to keep the spi bus quiet during sampling - I remember having
seen one datasheet where cs was required to remain asserted
during sampling and after X SCK cycles the conversion could get read.

> 
>> All it knows is that the device requires X clock cycles of cs high.
> 
> That's what I was driving at with my "to what end?" question.  You've
> apparently got a slave device that requires a delay of a specific number
> of clock cycles.  You should make that explicit in the commit messages,
> ideally by naming the slave that requires it.  Right now your commit
> messages only explain the "what", not the "why".  For an uninitiated
> reader it's helpful to understand the specific motivation of your code
> changes.

I will make that commit message clearer in case a new version of 
the patchset is needed for other reasons.

Thanks,
	Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] spi: core: allow defining time that cs is deasserted as a multiple of SCK

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 746 bytes --]

On Sun, Feb 24, 2019 at 12:03:33PM +0100, kernel@martin.sperl.org wrote:

> Some devices - like the mcp2517fd -  have for example an internal PLL
> based on an external clock. So during setup you have to use speed_hz 
> of <clock_hz> / 2 (or 4MHz at most) and only when PLL is in sync we 
> may be using speed_hz from the dt (or less if a module parameter is
> used to limit ourselves further)

> So the initial setup would not be able to help here - and every
> bus controller would now be required to implement setup.

> It also means open coding the calculations in each driver that 
> needs something like this.

> Thus it is - imo - in the right location to support it in spi core.

I agree, this feature makes sense to me.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] spi: core: allow defining time that cs is deasserted as a multiple of SCK

[kernel]

> On 26.02.2019, at 12:37, Mark Brown <broonie@kernel.org> wrote:
> 
> On Sun, Feb 24, 2019 at 12:03:33PM +0100, kernel@martin.sperl.org wrote:
> 
>> Some devices - like the mcp2517fd -  have for example an internal PLL
>> based on an external clock. So during setup you have to use speed_hz 
>> of <clock_hz> / 2 (or 4MHz at most) and only when PLL is in sync we 
>> may be using speed_hz from the dt (or less if a module parameter is
>> used to limit ourselves further)
> 
>> So the initial setup would not be able to help here - and every
>> bus controller would now be required to implement setup.
> 
>> It also means open coding the calculations in each driver that 
>> needs something like this.
> 
>> Thus it is - imo - in the right location to support it in spi core.
> 
> I agree, this feature makes sense to me.

Is there anything that really block this patch?

Do you want a rebase?
Anything else?

Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/5] spi: bcm2835: support effective_speed_hz

[kernel]

From: Martin Sperl <kernel@martin.sperl.org>

Setting spi_transfer->effective_speed_hz in transfer_one so that
it can get used in cs_change_delay configured with delay as a muliple
of SPI clock cycles.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
---
 drivers/spi/spi-bcm2835.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 35aebdfd3b4e..c2912c1e09c5 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -777,7 +777,6 @@ static int bcm2835_spi_transfer_one(struct spi_master *master,
 {
 	struct bcm2835_spi *bs = spi_master_get_devdata(master);
 	unsigned long spi_hz, clk_hz, cdiv;
-	unsigned long spi_used_hz;
 	unsigned long long xfer_time_us;
 	u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);

@@ -797,7 +796,7 @@ static int bcm2835_spi_transfer_one(struct spi_master *master,
 	} else {
 		cdiv = 0; /* 0 is the slowest we can go */
 	}
-	spi_used_hz = cdiv ? (clk_hz / cdiv) : (clk_hz / 65536);
+	tfr->effective_speed_hz = cdiv ? (clk_hz / cdiv) : (clk_hz / 65536);
 	bcm2835_wr(bs, BCM2835_SPI_CLK, cdiv);

 	/* handle all the 3-wire mode */
@@ -823,7 +822,7 @@ static int bcm2835_spi_transfer_one(struct spi_master *master,
 	xfer_time_us = (unsigned long long)tfr->len
 		* 9 /* clocks/byte - SPI-HW waits 1 clock after each byte */
 		* 1000000;
-	do_div(xfer_time_us, spi_used_hz);
+	do_div(xfer_time_us, tfr->effective_speed_hz);

 	/* for short requests run polling*/
 	if (xfer_time_us <= BCM2835_SPI_POLLING_LIMIT_US)
--
2.11.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] spi: bcm2835: support effective_speed_hz

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 304 bytes --]

On Sat, Feb 23, 2019 at 08:49:51AM +0000, kernel@martin.sperl.org wrote:
> From: Martin Sperl <kernel@martin.sperl.org>
> 
> Setting spi_transfer->effective_speed_hz in transfer_one so that
> it can get used in cs_change_delay configured with delay as a muliple
> of SPI clock cycles.

This too.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel