* [PATCH v3 0/3] spi: introduce `struct spi_delay` data-type
@ 2019-09-16 7:10 Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 1/3] spi: move `cs_change_delay` backwards compat logic outside switch Alexandru Ardelean
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2019-09-16 7:10 UTC (permalink / raw)
To: linux-iio, linux-spi, linux-kernel; +Cc: jic23, broonie, Alexandru Ardelean
Discussion reference:
https://lore.kernel.org/lkml/20190913114550.956-1-alexandru.ardelean@analog.com/
This changeset introduces an `spi_delay` struct/data-type and makes the
IIO ADIS driver library the first user of this.
The patchset base is Jonathan's `iio/togreg` branch, but it also applies on
Mark's `spi/for-next` branch.
The reasons for choosing `cs_change_delay`, is:
1. this change introduces delay units
2. it is not yet widely used, meaning it can still be changed
Some delays like `delay` from `spi_transfer` would require a longer
update-time change & discussion.
Alexandru Ardelean (3):
spi: move `cs_change_delay` backwards compat logic outside switch
spi: introduce spi_delay struct as "value + unit" & spi_delay_exec()
spi: make `cs_change_delay` the first user of the `spi_delay` logic
drivers/iio/imu/adis.c | 24 +++++++--------
drivers/spi/spi.c | 68 +++++++++++++++++++++++++++++++----------
include/linux/spi/spi.h | 22 +++++++++----
3 files changed, 80 insertions(+), 34 deletions(-)
--
Changelog v2 -> v3:
* squash patches 3 & 4 into a single patch; otherwise the kernel is in
broken state between those 2 patches
Changelog v1 -> v2:
* split away from the RFC patchset, which aims to be a broader explanation
for this changeset; parts of v1 are not 100% defined yet, and may require
some discussion and refinement.
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3 1/3] spi: move `cs_change_delay` backwards compat logic outside switch
2019-09-16 7:10 [PATCH v3 0/3] spi: introduce `struct spi_delay` data-type Alexandru Ardelean
@ 2019-09-16 7:10 ` Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 2/3] spi: introduce spi_delay struct as "value + unit" & spi_delay_exec() Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 3/3] spi: make `cs_change_delay` the first user of the `spi_delay` logic Alexandru Ardelean
2 siblings, 0 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2019-09-16 7:10 UTC (permalink / raw)
To: linux-iio, linux-spi, linux-kernel; +Cc: jic23, broonie, Alexandru Ardelean
The `cs_change_delay` backwards compatibility value could be moved outside
of the switch statement.
The only reason to do it, is to make the next patches easier to diff.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/spi/spi.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 75ac046cae52..c90e02e6d62f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1114,16 +1114,15 @@ static void _spi_transfer_cs_change_delay(struct spi_message *msg,
u32 hz;
/* return early on "fast" mode - for everything but USECS */
- if (!delay && unit != SPI_DELAY_UNIT_USECS)
+ if (!delay) {
+ if (unit == SPI_DELAY_UNIT_USECS)
+ _spi_transfer_delay_ns(10000);
return;
+ }
switch (unit) {
case SPI_DELAY_UNIT_USECS:
- /* for compatibility use default of 10us */
- if (!delay)
- delay = 10000;
- else
- delay *= 1000;
+ delay *= 1000;
break;
case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v3 2/3] spi: introduce spi_delay struct as "value + unit" & spi_delay_exec()
2019-09-16 7:10 [PATCH v3 0/3] spi: introduce `struct spi_delay` data-type Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 1/3] spi: move `cs_change_delay` backwards compat logic outside switch Alexandru Ardelean
@ 2019-09-16 7:10 ` Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 3/3] spi: make `cs_change_delay` the first user of the `spi_delay` logic Alexandru Ardelean
2 siblings, 0 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2019-09-16 7:10 UTC (permalink / raw)
To: linux-iio, linux-spi, linux-kernel; +Cc: jic23, broonie, Alexandru Ardelean
There are plenty of delays that have been introduced in SPI core. Most of
them are in micro-seconds, some need to be in nano-seconds, and some in
clock-cycles.
For some of these delays (related to transfers & CS timing) it may make
sense to have a `spi_delay` struct that abstracts these a bit.
The important element of these delays [for unification] seems to be the
`unit` of the delay.
It looks like micro-seconds is good enough for most people, but every-once
in a while, some delays seem to require other units of measurement.
This change adds the `spi_delay` struct & a `spi_delay_exec()` function
that processes a `spi_delay` object/struct to execute the delay.
It's a copy of the `cs_change_delay` mechanism, but without the default
for 10 uS.
The clock-cycle delay unit is a bit special, as it needs to be bound to an
`spi_transfer` object to execute.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/spi/spi.c | 51 +++++++++++++++++++++++++++++++++++++++++
include/linux/spi/spi.h | 18 ++++++++++++---
2 files changed, 66 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c90e02e6d62f..1883de8ffa82 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1106,6 +1106,57 @@ static void _spi_transfer_delay_ns(u32 ns)
}
}
+static int _spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer *xfer)
+{
+ u32 delay = _delay->value;
+ u32 unit = _delay->unit;
+ u32 hz;
+
+ if (!delay)
+ return 0;
+
+ switch (unit) {
+ case SPI_DELAY_UNIT_USECS:
+ delay *= 1000;
+ break;
+ case SPI_DELAY_UNIT_NSECS: /* nothing to do here */
+ break;
+ case SPI_DELAY_UNIT_SCK:
+ /* clock cycles need to be obtained from spi_transfer */
+ if (!xfer)
+ return -EINVAL;
+ /* 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;
+ if (!hz)
+ return -EINVAL;
+ delay *= DIV_ROUND_UP(1000000000, hz);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return delay;
+}
+
+int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer)
+{
+ int delay;
+
+ if (!_delay)
+ return -EINVAL;
+
+ delay = _spi_delay_to_ns(_delay, xfer);
+ if (delay < 0)
+ return delay;
+
+ _spi_transfer_delay_ns(delay);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(spi_delay_exec);
+
static void _spi_transfer_cs_change_delay(struct spi_message *msg,
struct spi_transfer *xfer)
{
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index af4f265d0f67..c18cfa7cda35 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -89,6 +89,21 @@ void spi_statistics_add_transfer_stats(struct spi_statistics *stats,
#define SPI_STATISTICS_INCREMENT_FIELD(stats, field) \
SPI_STATISTICS_ADD_TO_FIELD(stats, field, 1)
+/**
+ * struct spi_delay - SPI delay information
+ * @value: Value for the delay
+ * @unit: Unit for the delay
+ */
+struct spi_delay {
+#define SPI_DELAY_UNIT_USECS 0
+#define SPI_DELAY_UNIT_NSECS 1
+#define SPI_DELAY_UNIT_SCK 2
+ u16 value;
+ u8 unit;
+};
+
+extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
+
/**
* struct spi_device - Controller side proxy for an SPI slave device
* @dev: Driver model representation of the device.
@@ -834,9 +849,6 @@ struct spi_transfer {
u16 delay_usecs;
u16 cs_change_delay;
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.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v3 3/3] spi: make `cs_change_delay` the first user of the `spi_delay` logic
2019-09-16 7:10 [PATCH v3 0/3] spi: introduce `struct spi_delay` data-type Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 1/3] spi: move `cs_change_delay` backwards compat logic outside switch Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 2/3] spi: introduce spi_delay struct as "value + unit" & spi_delay_exec() Alexandru Ardelean
@ 2019-09-16 7:10 ` Alexandru Ardelean
2 siblings, 0 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2019-09-16 7:10 UTC (permalink / raw)
To: linux-iio, linux-spi, linux-kernel; +Cc: jic23, broonie, Alexandru Ardelean
Since the logic for `spi_delay` struct + `spi_delay_exec()` has been copied
from the `cs_change_delay` logic, it's natural to make this delay, the
first user.
The `cs_change_delay` logic requires that the default remain 10 uS, in case
it is unspecified/unconfigured. So, there is some special handling needed
to do that.
The ADIS library is one of the few users of the new `cs_change_delay`
parameter for an spi_transfer.
The introduction of the `spi_delay` struct, requires that the users of of
`cs_change_delay` get an update. This change also updates the ADIS library.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/imu/adis.c | 24 ++++++++++++------------
drivers/spi/spi.c | 28 +++++++---------------------
include/linux/spi/spi.h | 4 +---
3 files changed, 20 insertions(+), 36 deletions(-)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 1631c255deab..2cd2cc2316c6 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -39,24 +39,24 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 4,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 6,
.bits_per_word = 8,
@@ -139,16 +139,16 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 4,
.rx_buf = adis->rx,
@@ -156,8 +156,8 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
- .cs_change_delay = adis->data->cs_change_delay,
- .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
+ .cs_change_delay.value = adis->data->cs_change_delay,
+ .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
}, {
.rx_buf = adis->rx + 2,
.bits_per_word = 8,
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 1883de8ffa82..d0bf0ffca042 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1160,9 +1160,9 @@ EXPORT_SYMBOL_GPL(spi_delay_exec);
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;
- u32 hz;
+ u32 delay = xfer->cs_change_delay.value;
+ u32 unit = xfer->cs_change_delay.unit;
+ int ret;
/* return early on "fast" mode - for everything but USECS */
if (!delay) {
@@ -1171,27 +1171,13 @@ static void _spi_transfer_cs_change_delay(struct spi_message *msg,
return;
}
- switch (unit) {
- case SPI_DELAY_UNIT_USECS:
- delay *= 1000;
- 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:
+ ret = spi_delay_exec(&xfer->cs_change_delay, xfer);
+ if (ret) {
dev_err_once(&msg->spi->dev,
"Use of unsupported delay unit %i, using default of 10us\n",
- xfer->cs_change_delay_unit);
- delay = 10000;
+ unit);
+ _spi_transfer_delay_ns(10000);
}
- /* now sleep for the requested amount of time */
- _spi_transfer_delay_ns(delay);
}
/*
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index c18cfa7cda35..9ded3f44d58e 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -754,7 +754,6 @@ extern void spi_res_release(struct spi_controller *ctlr,
* @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.
@@ -847,8 +846,7 @@ struct spi_transfer {
u8 bits_per_word;
u8 word_delay_usecs;
u16 delay_usecs;
- u16 cs_change_delay;
- u8 cs_change_delay_unit;
+ struct spi_delay cs_change_delay;
u32 speed_hz;
u16 word_delay;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-16 7:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-16 7:10 [PATCH v3 0/3] spi: introduce `struct spi_delay` data-type Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 1/3] spi: move `cs_change_delay` backwards compat logic outside switch Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 2/3] spi: introduce spi_delay struct as "value + unit" & spi_delay_exec() Alexandru Ardelean
2019-09-16 7:10 ` [PATCH v3 3/3] spi: make `cs_change_delay` the first user of the `spi_delay` logic Alexandru Ardelean
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 NNTP newsgroup(s).