From b42faeee718ce13ef6eb99c24880b58deb54c8fa Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Thu, 5 Sep 2019 04:01:12 +0300 Subject: spi: Add a PTP system timestamp to the transfer structure SPI is one of the interfaces used to access devices which have a POSIX clock driver (real time clocks, 1588 timers etc). The fact that the SPI bus is slow is not what the main problem is, but rather the fact that drivers don't take a constant amount of time in transferring data over SPI. When there is a high delay in the readout of time, there will be uncertainty in the value that has been read out of the peripheral. When that delay is constant, the uncertainty can at least be approximated with a certain accuracy which is fine more often than not. Timing jitter occurs all over in the kernel code, and is mainly caused by having to let go of the CPU for various reasons such as preemption, servicing interrupts, going to sleep, etc. Another major reason is CPU dynamic frequency scaling. It turns out that the problem of retrieving time from a SPI peripheral with high accuracy can be solved by the use of "PTP system timestamping" - a mechanism to correlate the time when the device has snapshotted its internal time counter with the Linux system time at that same moment. This is sufficient for having a precise time measurement - it is not necessary for the whole SPI transfer to be transmitted "as fast as possible", or "as low-jitter as possible". The system has to be low-jitter for a very short amount of time to be effective. This patch introduces a PTP system timestamping mechanism in struct spi_transfer. This is to be used by SPI device drivers when they need to know the exact time at which the underlying device's time was snapshotted. More often than not, SPI peripherals have a very exact timing for when their SPI-to-interconnect bridge issues a transaction for snapshotting and reading the time register, and that will be dependent on when the SPI-to-interconnect bridge figures out that this is what it should do, aka as soon as it sees byte N of the SPI transfer. Since spi_device drivers are the ones who'd know best how the peripheral behaves in this regard, expose a mechanism in spi_transfer which allows them to specify which word (or word range) from the transfer should be timestamped. Add a default implementation of the PTP system timestamping in the SPI core. This is not going to be satisfactory performance-wise, but should at least increase the likelihood that SPI device drivers will use PTP system timestamping in the future. There are 3 entry points from the core towards the SPI controller drivers: - transfer_one: The driver is passed individual spi_transfers to execute. This is the easiest to timestamp. - transfer_one_message: The core passes the driver an entire spi_message (a potential batch of spi_transfers). The core puts the same pre and post timestamp to all transfers within a message. This is not ideal, but nothing better can be done by default anyway, since the core has no insight into how the driver batches the transfers. - transfer: Like transfer_one_message, but for unqueued drivers (i.e. the driver implements its own queue scheduling). Signed-off-by: Vladimir Oltean Link: https://lore.kernel.org/r/20190905010114.26718-3-olteanv@gmail.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index f9502dbbb5c1..9bb36c32cbf9 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1171,6 +1171,11 @@ static int spi_transfer_one_message(struct spi_controller *ctlr, spi_statistics_add_transfer_stats(statm, xfer, ctlr); spi_statistics_add_transfer_stats(stats, xfer, ctlr); + if (!ctlr->ptp_sts_supported) { + xfer->ptp_sts_word_pre = 0; + ptp_read_system_prets(xfer->ptp_sts); + } + if (xfer->tx_buf || xfer->rx_buf) { reinit_completion(&ctlr->xfer_completion); @@ -1197,6 +1202,11 @@ static int spi_transfer_one_message(struct spi_controller *ctlr, xfer->len); } + if (!ctlr->ptp_sts_supported) { + ptp_read_system_postts(xfer->ptp_sts); + xfer->ptp_sts_word_post = xfer->len; + } + trace_spi_transfer_stop(msg, xfer); if (msg->status != -EINPROGRESS) @@ -1265,6 +1275,7 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer); */ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread) { + struct spi_transfer *xfer; struct spi_message *msg; bool was_busy = false; unsigned long flags; @@ -1391,6 +1402,13 @@ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread) goto out; } + if (!ctlr->ptp_sts_supported && !ctlr->transfer_one) { + list_for_each_entry(xfer, &msg->transfers, transfer_list) { + xfer->ptp_sts_word_pre = 0; + ptp_read_system_prets(xfer->ptp_sts); + } + } + ret = ctlr->transfer_one_message(ctlr, msg); if (ret) { dev_err(&ctlr->dev, @@ -1418,6 +1436,99 @@ static void spi_pump_messages(struct kthread_work *work) __spi_pump_messages(ctlr, true); } +/** + * spi_take_timestamp_pre - helper for drivers to collect the beginning of the + * TX timestamp for the requested byte from the SPI + * transfer. The frequency with which this function + * must be called (once per word, once for the whole + * transfer, once per batch of words etc) is arbitrary + * as long as the @tx buffer offset is greater than or + * equal to the requested byte at the time of the + * call. The timestamp is only taken once, at the + * first such call. It is assumed that the driver + * advances its @tx buffer pointer monotonically. + * @ctlr: Pointer to the spi_controller structure of the driver + * @xfer: Pointer to the transfer being timestamped + * @tx: Pointer to the current word within the xfer->tx_buf that the driver is + * preparing to transmit right now. + * @irqs_off: If true, will disable IRQs and preemption for the duration of the + * transfer, for less jitter in time measurement. Only compatible + * with PIO drivers. If true, must follow up with + * spi_take_timestamp_post or otherwise system will crash. + * WARNING: for fully predictable results, the CPU frequency must + * also be under control (governor). + */ +void spi_take_timestamp_pre(struct spi_controller *ctlr, + struct spi_transfer *xfer, + const void *tx, bool irqs_off) +{ + u8 bytes_per_word = DIV_ROUND_UP(xfer->bits_per_word, 8); + + if (!xfer->ptp_sts) + return; + + if (xfer->timestamped_pre) + return; + + if (tx < (xfer->tx_buf + xfer->ptp_sts_word_pre * bytes_per_word)) + return; + + /* Capture the resolution of the timestamp */ + xfer->ptp_sts_word_pre = (tx - xfer->tx_buf) / bytes_per_word; + + xfer->timestamped_pre = true; + + if (irqs_off) { + local_irq_save(ctlr->irq_flags); + preempt_disable(); + } + + ptp_read_system_prets(xfer->ptp_sts); +} +EXPORT_SYMBOL_GPL(spi_take_timestamp_pre); + +/** + * spi_take_timestamp_post - helper for drivers to collect the end of the + * TX timestamp for the requested byte from the SPI + * transfer. Can be called with an arbitrary + * frequency: only the first call where @tx exceeds + * or is equal to the requested word will be + * timestamped. + * @ctlr: Pointer to the spi_controller structure of the driver + * @xfer: Pointer to the transfer being timestamped + * @tx: Pointer to the current word within the xfer->tx_buf that the driver has + * just transmitted. + * @irqs_off: If true, will re-enable IRQs and preemption for the local CPU. + */ +void spi_take_timestamp_post(struct spi_controller *ctlr, + struct spi_transfer *xfer, + const void *tx, bool irqs_off) +{ + u8 bytes_per_word = DIV_ROUND_UP(xfer->bits_per_word, 8); + + if (!xfer->ptp_sts) + return; + + if (xfer->timestamped_post) + return; + + if (tx < (xfer->tx_buf + xfer->ptp_sts_word_post * bytes_per_word)) + return; + + ptp_read_system_postts(xfer->ptp_sts); + + if (irqs_off) { + local_irq_restore(ctlr->irq_flags); + preempt_enable(); + } + + /* Capture the resolution of the timestamp */ + xfer->ptp_sts_word_post = (tx - xfer->tx_buf) / bytes_per_word; + + xfer->timestamped_post = true; +} +EXPORT_SYMBOL_GPL(spi_take_timestamp_post); + /** * spi_set_thread_rt - set the controller to pump at realtime priority * @ctlr: controller to boost priority of @@ -1503,6 +1614,7 @@ EXPORT_SYMBOL_GPL(spi_get_next_queued_message); */ void spi_finalize_current_message(struct spi_controller *ctlr) { + struct spi_transfer *xfer; struct spi_message *mesg; unsigned long flags; int ret; @@ -1511,6 +1623,13 @@ void spi_finalize_current_message(struct spi_controller *ctlr) mesg = ctlr->cur_msg; spin_unlock_irqrestore(&ctlr->queue_lock, flags); + if (!ctlr->ptp_sts_supported && !ctlr->transfer_one) { + list_for_each_entry(xfer, &mesg->transfers, transfer_list) { + ptp_read_system_postts(xfer->ptp_sts); + xfer->ptp_sts_word_post = xfer->len; + } + } + spi_unmap_msg(ctlr, mesg); if (ctlr->cur_msg_prepared && ctlr->unprepare_message) { @@ -3273,6 +3392,7 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) static int __spi_async(struct spi_device *spi, struct spi_message *message) { struct spi_controller *ctlr = spi->controller; + struct spi_transfer *xfer; /* * Some controllers do not support doing regular SPI transfers. Return @@ -3288,6 +3408,13 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) trace_spi_message_submit(message); + if (!ctlr->ptp_sts_supported) { + list_for_each_entry(xfer, &message->transfers, transfer_list) { + xfer->ptp_sts_word_pre = 0; + ptp_read_system_prets(xfer->ptp_sts); + } + } + return ctlr->transfer(spi, message); } -- cgit v1.2.3 From 6b3f236a998550dba91a46a22feb1cc02f39fb06 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Thu, 26 Sep 2019 13:51:29 +0300 Subject: spi: move `cs_change_delay` backwards compat logic outside switch 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 Link: https://lore.kernel.org/r/20190926105147.7839-2-alexandru.ardelean@analog.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 9bb36c32cbf9..7ba981cdb86b 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; -- cgit v1.2.3 From b2c98153f45fc17b9fcb241000f2d131ddea6030 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Thu, 26 Sep 2019 13:51:30 +0300 Subject: spi: introduce spi_delay struct as "value + unit" & spi_delay_exec() 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 Link: https://lore.kernel.org/r/20190926105147.7839-3-alexandru.ardelean@analog.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/spi/spi.h | 18 ++++++++++++++--- 2 files changed, 66 insertions(+), 3 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 7ba981cdb86b..7499a4efbaba 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 27f6b046cf92..8f643de3197b 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -90,6 +90,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. @@ -887,9 +902,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; -- cgit v1.2.3 From 329f0dac4cad9fa4b1439a88180d91bcb5c4eaf8 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Thu, 26 Sep 2019 13:51:31 +0300 Subject: spi: make `cs_change_delay` the first user of the `spi_delay` logic 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 Link: https://lore.kernel.org/r/20190926105147.7839-4-alexandru.ardelean@analog.com Signed-off-by: Mark Brown --- drivers/iio/imu/adis.c | 24 ++++++++++++------------ drivers/spi/spi.c | 28 +++++++--------------------- include/linux/spi/spi.h | 4 +--- 3 files changed, 20 insertions(+), 36 deletions(-) (limited to 'drivers/spi/spi.c') 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 7499a4efbaba..b69c14082c52 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 8f643de3197b..7670be934643 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -778,7 +778,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. @@ -900,8 +899,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; -- cgit v1.2.3 From 6c613f68aabf33385c01e949204ac5ed30887161 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Thu, 26 Sep 2019 13:51:35 +0300 Subject: spi: core,atmel: convert `word_delay_usecs` -> `word_delay` for spi_device This change does a conversion from the `word_delay_usecs` -> `word_delay` for the `spi_device` struct. This allows users to specify inter-word delays in other unit types (nano-seconds or clock cycles), depending on how users want. The Atmel SPI driver is the only current user of the `word_delay_usecs` field (from the `spi_device` struct). So, it needed a slight conversion to use the `word_delay` as an `spi_delay` struct. In SPI core, the only required mechanism is to update the `word_delay` information per `spi_transfer`. This requires a bit more logic than before, because it needs that both delays be converted to a common unit (nano-seconds) for comparison. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20190926105147.7839-8-alexandru.ardelean@analog.com Signed-off-by: Mark Brown --- drivers/spi/spi-atmel.c | 26 ++++++++++++++++++++++++-- drivers/spi/spi.c | 24 ++++++++++++++++++++++-- include/linux/spi/spi.h | 7 ++----- 3 files changed, 48 insertions(+), 9 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c index 3ed5e663da6f..1471b049f99a 100644 --- a/drivers/spi/spi-atmel.c +++ b/drivers/spi/spi-atmel.c @@ -1149,12 +1149,31 @@ atmel_spi_pdc_interrupt(int irq, void *dev_id) return ret; } +static int atmel_word_delay_csr(struct spi_device *spi, struct atmel_spi *as) +{ + struct spi_delay *delay = &spi->word_delay; + u32 value = delay->value; + + switch (delay->unit) { + case SPI_DELAY_UNIT_NSECS: + value /= 1000; + break; + case SPI_DELAY_UNIT_USECS: + break; + default: + return -EINVAL; + } + + return (as->spi_clk / 1000000 * value) >> 5; +} + static int atmel_spi_setup(struct spi_device *spi) { struct atmel_spi *as; struct atmel_spi_device *asd; u32 csr; unsigned int bits = spi->bits_per_word; + int word_delay_csr; as = spi_master_get_devdata(spi->master); @@ -1178,11 +1197,14 @@ static int atmel_spi_setup(struct spi_device *spi) */ csr |= SPI_BF(DLYBS, 0); + word_delay_csr = atmel_word_delay_csr(spi, as); + if (word_delay_csr < 0) + return word_delay_csr; + /* DLYBCT adds delays between words. This is useful for slow devices * that need a bit of time to setup the next transfer. */ - csr |= SPI_BF(DLYBCT, - (as->spi_clk / 1000000 * spi->word_delay_usecs) >> 5); + csr |= SPI_BF(DLYBCT, word_delay_csr); asd = spi->controller_state; if (!asd) { diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index b69c14082c52..307e440dd92d 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -3281,6 +3281,26 @@ void spi_set_cs_timing(struct spi_device *spi, u8 setup, u8 hold, } EXPORT_SYMBOL_GPL(spi_set_cs_timing); +static int _spi_xfer_word_delay_update(struct spi_transfer *xfer, + struct spi_device *spi) +{ + int delay1, delay2; + + delay1 = _spi_delay_to_ns(&xfer->word_delay, xfer); + if (delay1 < 0) + return delay1; + + delay2 = _spi_delay_to_ns(&spi->word_delay, xfer); + if (delay2 < 0) + return delay2; + + if (delay1 < delay2) + memcpy(&xfer->word_delay, &spi->word_delay, + sizeof(xfer->word_delay)); + + return 0; +} + static int __spi_validate(struct spi_device *spi, struct spi_message *message) { struct spi_controller *ctlr = spi->controller; @@ -3416,8 +3436,8 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) return -EINVAL; } - if (xfer->word_delay_usecs < spi->word_delay_usecs) - xfer->word_delay_usecs = spi->word_delay_usecs; + if (_spi_xfer_word_delay_update(xfer, spi)) + return -EINVAL; } message->status = -EINPROGRESS; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 6cb67ad53ffa..ebeb272aeb0f 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -139,7 +139,7 @@ extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer); * the spi_master. * @cs_gpiod: gpio descriptor of the chipselect line (optional, NULL when * not using a GPIO line) - * @word_delay_usecs: microsecond delay to be inserted between consecutive + * @word_delay: delay to be inserted between consecutive * words of a transfer * * @statistics: statistics for the spi_device @@ -189,7 +189,7 @@ struct spi_device { const char *driver_override; int cs_gpio; /* LEGACY: chip select gpio */ struct gpio_desc *cs_gpiod; /* chip select gpio desc */ - uint8_t word_delay_usecs; /* inter-word delay */ + struct spi_delay word_delay; /* inter-word delay */ /* the statistics */ struct spi_statistics statistics; @@ -781,8 +781,6 @@ extern void spi_res_release(struct spi_controller *ctlr, * @delay_usecs: microseconds to delay after this transfer before * (optionally) changing the chipselect status, then starting * the next transfer or completing this @spi_message. - * @word_delay_usecs: microseconds to inter word delay after each word size - * (set by bits_per_word) transmission. * @word_delay: inter word delay to be introduced after each word size * (set by bits_per_word) transmission. * @effective_speed_hz: the effective SCK-speed that was used to @@ -897,7 +895,6 @@ struct spi_transfer { #define SPI_NBITS_DUAL 0x02 /* 2bits transfer */ #define SPI_NBITS_QUAD 0x04 /* 4bits transfer */ u8 bits_per_word; - u8 word_delay_usecs; u16 delay_usecs; struct spi_delay cs_change_delay; struct spi_delay word_delay; -- cgit v1.2.3 From bebcfd272df648542c458d28fbd6a8f9428b5310 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Thu, 26 Sep 2019 13:51:36 +0300 Subject: spi: introduce `delay` field for `spi_transfer` + spi_transfer_delay_exec() The change introduces the `delay` field to the `spi_transfer` struct as an `struct spi_delay` type. This intends to eventually replace `delay_usecs`. But, since there are many users of `delay_usecs`, this needs some intermediate work. A helper called `spi_transfer_delay_exec()` is also added, which maintains backwards compatibility with `delay_usecs`, by assigning the value to `delay` if non-zero. This should maintain backwards compatibility with current users of `udelay_usecs`. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20190926105147.7839-9-alexandru.ardelean@analog.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 6 +++--- include/linux/spi/spi.h | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 307e440dd92d..2e448078a117 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1248,8 +1248,7 @@ static int spi_transfer_one_message(struct spi_controller *ctlr, if (msg->status != -EINPROGRESS) goto out; - if (xfer->delay_usecs) - _spi_transfer_delay_ns(xfer->delay_usecs * 1000); + spi_transfer_delay_exec(xfer); if (xfer->cs_change) { if (list_is_last(&xfer->transfer_list, @@ -3026,10 +3025,11 @@ struct spi_replaced_transfers *spi_replace_transfers( /* add to list */ list_add(&xfer->transfer_list, rxfer->replaced_after); - /* clear cs_change and delay_usecs for all but the last */ + /* clear cs_change and delay for all but the last */ if (i) { xfer->cs_change = false; xfer->delay_usecs = 0; + xfer->delay.value = 0; } } diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index ebeb272aeb0f..fe5b85df2c79 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -778,6 +778,9 @@ 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 + * @delay: delay to be introduced after this transfer before + * (optionally) changing the chipselect status, then starting + * the next transfer or completing this @spi_message. * @delay_usecs: microseconds to delay after this transfer before * (optionally) changing the chipselect status, then starting * the next transfer or completing this @spi_message. @@ -896,6 +899,7 @@ struct spi_transfer { #define SPI_NBITS_QUAD 0x04 /* 4bits transfer */ u8 bits_per_word; u16 delay_usecs; + struct spi_delay delay; struct spi_delay cs_change_delay; struct spi_delay word_delay; u32 speed_hz; @@ -1003,6 +1007,20 @@ spi_transfer_del(struct spi_transfer *t) list_del(&t->transfer_list); } +static inline int +spi_transfer_delay_exec(struct spi_transfer *t) +{ + struct spi_delay d; + + if (t->delay_usecs) { + d.value = t->delay_usecs; + d.unit = SPI_DELAY_UNIT_USECS; + return spi_delay_exec(&d, NULL); + } + + return spi_delay_exec(&t->delay, t); +} + /** * spi_message_init_with_transfers - Initialize spi_message and append transfers * @m: spi_message to be initialized -- cgit v1.2.3 From 8105936684681195d9073880b06a123b2e316811 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Thu, 26 Sep 2019 13:51:42 +0300 Subject: spi: tegra114: change format for `spi_set_cs_timing()` function The initial version of `spi_set_cs_timing()` was implemented with consideration only for clock-cycles as delay. For cases like `CS setup` time, it's sometimes needed that micro-seconds (or nano-seconds) are required, or sometimes even longer delays, for cases where the device needs a little longer to start transferring that after CS is asserted. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20190926105147.7839-15-alexandru.ardelean@analog.com Signed-off-by: Mark Brown --- drivers/spi/spi-tegra114.c | 22 ++++++++++++++++++++-- drivers/spi/spi.c | 16 ++++++++++------ include/linux/spi/spi.h | 9 ++++++--- 3 files changed, 36 insertions(+), 11 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c index 8133dc49d34f..e6a450d9b4f0 100644 --- a/drivers/spi/spi-tegra114.c +++ b/drivers/spi/spi-tegra114.c @@ -723,15 +723,31 @@ static void tegra_spi_deinit_dma_param(struct tegra_spi_data *tspi, dma_release_channel(dma_chan); } -static void tegra_spi_set_hw_cs_timing(struct spi_device *spi, u8 setup_dly, - u8 hold_dly, u8 inactive_dly) +static int tegra_spi_set_hw_cs_timing(struct spi_device *spi, + struct spi_delay *setup, + struct spi_delay *hold, + struct spi_delay *inactive) { struct tegra_spi_data *tspi = spi_master_get_devdata(spi->master); + u8 setup_dly, hold_dly, inactive_dly; u32 setup_hold; u32 spi_cs_timing; u32 inactive_cycles; u8 cs_state; + if ((setup && setup->unit != SPI_DELAY_UNIT_SCK) || + (hold && hold->unit != SPI_DELAY_UNIT_SCK) || + (inactive && inactive->unit != SPI_DELAY_UNIT_SCK)) { + dev_err(&spi->dev, + "Invalid delay unit %d, should be SPI_DELAY_UNIT_SCK\n", + SPI_DELAY_UNIT_SCK); + return -EINVAL; + } + + setup_dly = setup ? setup->value : 0; + hold_dly = hold ? hold->value : 0; + inactive_dly = inactive ? inactive->value : 0; + setup_dly = min_t(u8, setup_dly, MAX_SETUP_HOLD_CYCLES); hold_dly = min_t(u8, hold_dly, MAX_SETUP_HOLD_CYCLES); if (setup_dly && hold_dly) { @@ -758,6 +774,8 @@ static void tegra_spi_set_hw_cs_timing(struct spi_device *spi, u8 setup_dly, tspi->spi_cs_timing2 = spi_cs_timing; tegra_spi_writel(tspi, spi_cs_timing, SPI_CS_TIMING2); } + + return 0; } static u32 tegra_spi_setup_transfer_one(struct spi_device *spi, diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 2e448078a117..6beeb363515c 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -3269,15 +3269,19 @@ EXPORT_SYMBOL_GPL(spi_setup); /** * spi_set_cs_timing - configure CS setup, hold, and inactive delays * @spi: the device that requires specific CS timing configuration - * @setup: CS setup time in terms of clock count - * @hold: CS hold time in terms of clock count - * @inactive_dly: CS inactive delay between transfers in terms of clock count + * @setup: CS setup time specified via @spi_delay + * @hold: CS hold time specified via @spi_delay + * @inactive: CS inactive delay between transfers specified via @spi_delay + * + * Return: zero on success, else a negative error code. */ -void spi_set_cs_timing(struct spi_device *spi, u8 setup, u8 hold, - u8 inactive_dly) +int spi_set_cs_timing(struct spi_device *spi, struct spi_delay *setup, + struct spi_delay *hold, struct spi_delay *inactive) { if (spi->controller->set_cs_timing) - spi->controller->set_cs_timing(spi, setup, hold, inactive_dly); + return spi->controller->set_cs_timing(spi, setup, hold, + inactive); + return -ENOTSUPP; } EXPORT_SYMBOL_GPL(spi_set_cs_timing); diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index fe5b85df2c79..f9b4ba2db08d 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -524,8 +524,8 @@ struct spi_controller { * to configure specific CS timing through spi_set_cs_timing() after * spi_setup(). */ - void (*set_cs_timing)(struct spi_device *spi, u8 setup_clk_cycles, - u8 hold_clk_cycles, u8 inactive_clk_cycles); + int (*set_cs_timing)(struct spi_device *spi, struct spi_delay *setup, + struct spi_delay *hold, struct spi_delay *inactive); /* bidirectional bulk transfers * @@ -1068,7 +1068,10 @@ static inline void spi_message_free(struct spi_message *m) kfree(m); } -extern void spi_set_cs_timing(struct spi_device *spi, u8 setup, u8 hold, u8 inactive_dly); +extern int spi_set_cs_timing(struct spi_device *spi, + struct spi_delay *setup, + struct spi_delay *hold, + struct spi_delay *inactive); extern int spi_setup(struct spi_device *spi); extern int spi_async(struct spi_device *spi, struct spi_message *message); -- cgit v1.2.3 From 25093bdeb6bcae728e12e3795261dbd3677060a9 Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Thu, 26 Sep 2019 13:51:43 +0300 Subject: spi: implement SW control for CS times This change implements CS control for setup, hold & inactive delays. The `cs_setup` delay is completely new, and can help with cases where asserting the CS, also brings the device out of power-sleep, where there needs to be a longer (than usual), before transferring data. The `cs_hold` time can overlap with the `delay` (or `delay_usecs`) from an SPI transfer. The main difference is that `cs_hold` implies that CS will be de-asserted. The `cs_inactive` delay does not have a clear use-case yet. It has been implemented mostly because the `spi_set_cs_timing()` function implements it. To some degree, this could overlap or replace `cs_change_delay`, but this will require more consideration/investigation in the future. All these delays have been added to the `spi_controller` struct, as they would typically be configured by calling `spi_set_cs_timing()` after an `spi_setup()` call. Software-mode for CS control, implies that the `set_cs_timing()` hook has not been provided for the `spi_controller` object. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20190926105147.7839-16-alexandru.ardelean@analog.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- include/linux/spi/spi.h | 5 +++++ 2 files changed, 49 insertions(+), 1 deletion(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 6beeb363515c..21628b0728f1 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -775,6 +775,15 @@ int spi_register_board_info(struct spi_board_info const *info, unsigned n) static void spi_set_cs(struct spi_device *spi, bool enable) { + bool enable1 = enable; + + if (!spi->controller->set_cs_timing) { + if (enable1) + spi_delay_exec(&spi->controller->cs_setup, NULL); + else + spi_delay_exec(&spi->controller->cs_hold, NULL); + } + if (spi->mode & SPI_CS_HIGH) enable = !enable; @@ -800,6 +809,11 @@ static void spi_set_cs(struct spi_device *spi, bool enable) } else if (spi->controller->set_cs) { spi->controller->set_cs(spi, !enable); } + + if (!spi->controller->set_cs_timing) { + if (!enable1) + spi_delay_exec(&spi->controller->cs_inactive, NULL); + } } #ifdef CONFIG_HAS_DMA @@ -3278,10 +3292,39 @@ EXPORT_SYMBOL_GPL(spi_setup); int spi_set_cs_timing(struct spi_device *spi, struct spi_delay *setup, struct spi_delay *hold, struct spi_delay *inactive) { + size_t len; + if (spi->controller->set_cs_timing) return spi->controller->set_cs_timing(spi, setup, hold, inactive); - return -ENOTSUPP; + + if ((setup && setup->unit == SPI_DELAY_UNIT_SCK) || + (hold && hold->unit == SPI_DELAY_UNIT_SCK) || + (inactive && inactive->unit == SPI_DELAY_UNIT_SCK)) { + dev_err(&spi->dev, + "Clock-cycle delays for CS not supported in SW mode\n"); + return -ENOTSUPP; + } + + len = sizeof(struct spi_delay); + + /* copy delays to controller */ + if (setup) + memcpy(&spi->controller->cs_setup, setup, len); + else + memset(&spi->controller->cs_setup, 0, len); + + if (hold) + memcpy(&spi->controller->cs_hold, hold, len); + else + memset(&spi->controller->cs_hold, 0, len); + + if (inactive) + memcpy(&spi->controller->cs_inactive, inactive, len); + else + memset(&spi->controller->cs_inactive, 0, len); + + return 0; } EXPORT_SYMBOL_GPL(spi_set_cs_timing); diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index f9b4ba2db08d..cfd87b18f077 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -609,6 +609,11 @@ struct spi_controller { /* Optimized handlers for SPI memory-like operations. */ const struct spi_controller_mem_ops *mem_ops; + /* CS delays */ + struct spi_delay cs_setup; + struct spi_delay cs_hold; + struct spi_delay cs_inactive; + /* gpio chip select */ int *cs_gpios; struct gpio_desc **cs_gpiods; -- cgit v1.2.3 From 3984d39b0e41ac4de8b4530ae3911ccf52ed4bbf Mon Sep 17 00:00:00 2001 From: Alexandru Ardelean Date: Thu, 26 Sep 2019 13:51:44 +0300 Subject: spi: spi-fsl-espi: convert transfer delay to `spi_delay` format The way the max delay is computed for this controller, it looks like it is searching for the max delay from an SPI message a using that. No idea if this is valid. But this change should support both `delay_usecs` and the new `delay` data which is of `spi_delay` type. Signed-off-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20190926105147.7839-17-alexandru.ardelean@analog.com Signed-off-by: Mark Brown --- drivers/spi/spi-fsl-espi.c | 16 +++++++++++++--- drivers/spi/spi.c | 9 +++++---- include/linux/spi/spi.h | 1 + 3 files changed, 19 insertions(+), 7 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c index f20326714b9d..f72daf66f3f3 100644 --- a/drivers/spi/spi-fsl-espi.c +++ b/drivers/spi/spi-fsl-espi.c @@ -437,6 +437,7 @@ static int fsl_espi_do_one_msg(struct spi_master *master, struct spi_message *m) { unsigned int delay_usecs = 0, rx_nbits = 0; + unsigned int delay_nsecs = 0, delay_nsecs1 = 0; struct spi_transfer *t, trans = {}; int ret; @@ -445,8 +446,16 @@ static int fsl_espi_do_one_msg(struct spi_master *master, goto out; list_for_each_entry(t, &m->transfers, transfer_list) { - if (t->delay_usecs > delay_usecs) - delay_usecs = t->delay_usecs; + if (t->delay_usecs) { + if (t->delay_usecs > delay_usecs) { + delay_usecs = t->delay_usecs; + delay_nsecs = delay_usecs * 1000; + } + } else { + delay_nsecs1 = spi_delay_to_ns(&t->delay, t); + if (delay_nsecs1 > delay_nsecs) + delay_nsecs = delay_nsecs1; + } if (t->rx_nbits > rx_nbits) rx_nbits = t->rx_nbits; } @@ -457,7 +466,8 @@ static int fsl_espi_do_one_msg(struct spi_master *master, trans.len = m->frame_length; trans.speed_hz = t->speed_hz; trans.bits_per_word = t->bits_per_word; - trans.delay_usecs = delay_usecs; + trans.delay.value = delay_nsecs; + trans.delay.unit = SPI_DELAY_UNIT_NSECS; trans.rx_nbits = rx_nbits; if (trans.len) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 21628b0728f1..5414a10afd65 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1120,7 +1120,7 @@ static void _spi_transfer_delay_ns(u32 ns) } } -static int _spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer *xfer) +int spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer *xfer) { u32 delay = _delay->value; u32 unit = _delay->unit; @@ -1153,6 +1153,7 @@ static int _spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer *xfer) return delay; } +EXPORT_SYMBOL_GPL(spi_delay_to_ns); int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer) { @@ -1161,7 +1162,7 @@ int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer) if (!_delay) return -EINVAL; - delay = _spi_delay_to_ns(_delay, xfer); + delay = spi_delay_to_ns(_delay, xfer); if (delay < 0) return delay; @@ -3333,11 +3334,11 @@ static int _spi_xfer_word_delay_update(struct spi_transfer *xfer, { int delay1, delay2; - delay1 = _spi_delay_to_ns(&xfer->word_delay, xfer); + delay1 = spi_delay_to_ns(&xfer->word_delay, xfer); if (delay1 < 0) return delay1; - delay2 = _spi_delay_to_ns(&spi->word_delay, xfer); + delay2 = spi_delay_to_ns(&spi->word_delay, xfer); if (delay2 < 0) return delay2; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index cfd87b18f077..c40d6af2bf07 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -103,6 +103,7 @@ struct spi_delay { u8 unit; }; +extern int spi_delay_to_ns(struct spi_delay *_delay, struct spi_transfer *xfer); extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer); /** -- cgit v1.2.3 From be73e323aee4e10eccef2b0f862c0196f392f012 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 23 Oct 2019 15:16:43 +0300 Subject: spi: Fix spelling in the comments Two spelling mistakes are being fixed. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20191023121643.25237-1-andriy.shevchenko@linux.intel.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 5414a10afd65..5ba19ef809c2 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -92,7 +92,7 @@ static ssize_t driver_override_store(struct device *dev, if (len) { spi->driver_override = driver_override; } else { - /* Emptry string, disable driver override */ + /* Empty string, disable driver override */ spi->driver_override = NULL; kfree(driver_override); } @@ -469,7 +469,7 @@ static LIST_HEAD(board_list); static LIST_HEAD(spi_controller_list); /* - * Used to protect add/del opertion for board_info list and + * Used to protect add/del operation for board_info list and * spi_controller list, and their matching process * also used to protect object of type struct idr */ -- cgit v1.2.3 From d948e6ca189985495a21cd622c31e30e72b6b688 Mon Sep 17 00:00:00 2001 From: Luhua Xu Date: Wed, 30 Oct 2019 17:03:54 +0800 Subject: spi: add power control when set_cs As to set_cs takes effect immediately, power spi is needed when setup spi. Cc: Mark Brown Signed-off-by: Luhua Xu Link: https://lore.kernel.org/r/1572426234-30019-1-git-send-email-luhua.xu@mediatek.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 5ba19ef809c2..294d0038eea6 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -3261,7 +3261,20 @@ int spi_setup(struct spi_device *spi) if (spi->controller->setup) status = spi->controller->setup(spi); - spi_set_cs(spi, false); + if (spi->controller->auto_runtime_pm && spi->controller->set_cs) { + status = pm_runtime_get_sync(spi->controller->dev.parent); + if (status < 0) { + pm_runtime_put_noidle(spi->controller->dev.parent); + dev_err(&spi->controller->dev, "Failed to power device: %d\n", + status); + return status; + } + spi_set_cs(spi, false); + pm_runtime_mark_last_busy(spi->controller->dev.parent); + pm_runtime_put_autosuspend(spi->controller->dev.parent); + } else { + spi_set_cs(spi, false); + } if (spi->rt && !spi->controller->rt) { spi->controller->rt = true; -- cgit v1.2.3 From 57a9460705f105e1d79d1410c5cfe285beda8986 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Mon, 11 Nov 2019 11:53:34 -0800 Subject: spi: Fix regression to return zero on success instead of positive value Commit d948e6ca1899 ("spi: add power control when set_cs") added generic runtime PM handling, but also changed the return value to be 1 instead of 0 that we had earlier as pm_runtime_get functions return a positve value on success. This causes SPI devices to return errors for cases where they do: ret = spi_setup(spi); if (ret) return ret; As in many cases the SPI devices do not check for if (ret < 0). Let's fix this by setting the status to 0 on succeess after the runtime PM calls. Let's not return 0 at the end of the function as this might break again later on if the function changes and starts returning status again. Fixes: d948e6ca1899 ("spi: add power control when set_cs") Cc: Luhua Xu Cc: wsd_upstream@mediatek.com Signed-off-by: Tony Lindgren Link: https://lore.kernel.org/r/20191111195334.44833-1-tony@atomide.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 294d0038eea6..0d40953b463c 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -3269,6 +3269,15 @@ int spi_setup(struct spi_device *spi) status); return status; } + + /* + * We do not want to return positive value from pm_runtime_get, + * there are many instances of devices calling spi_setup() and + * checking for a non-zero return value instead of a negative + * return value. + */ + status = 0; + spi_set_cs(spi, false); pm_runtime_mark_last_busy(spi->controller->dev.parent); pm_runtime_put_autosuspend(spi->controller->dev.parent); -- cgit v1.2.3