Unverified Commit 862dd2a9 authored by Vladimir Oltean's avatar Vladimir Oltean Committed by Mark Brown
Browse files

spi: Don't look at TX buffer for PTP system timestamping 



The API for PTP system timestamping (associating a SPI transaction with
the system time at which it was transferred) is flawed: it assumes that
the xfer->tx_buf pointer will always be present.

This is, of course, not always the case.

So introduce a "progress" variable that denotes how many word have been
transferred.

Fix the Freescale DSPI driver, the only user of the API so far, in the
same patch.

Fixes: b42faeee ("spi: Add a PTP system timestamp to the transfer structure")
Fixes: d6b71dfa ("spi: spi-fsl-dspi: Implement the PTP system timestamping for TCFQ mode")
Signed-off-by: default avatarVladimir Oltean <olteanv@gmail.com>
Link: https://lore.kernel.org/r/20191227012417.1057-1-olteanv@gmail.com


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent 9cd34efb

drivers/spi/spi-fsl-dspi.c

+6 −3
Original line number Diff line number Diff line
@@ -185,6 +185,7 @@ struct fsl_dspi { 
	struct spi_transfer			*cur_transfer;

	struct spi_message			*cur_msg;

	struct chip_data			*cur_chip;

	size_t					progress;

	size_t					len;

	const void				*tx;

	void					*rx;
@@ -658,7 +659,7 @@ static int dspi_rxtx(struct fsl_dspi *dspi) 
	u32 spi_tcr;



	spi_take_timestamp_post(dspi->ctlr, dspi->cur_transfer,

				dspi->tx - dspi->bytes_per_word, !dspi->irq);

				dspi->progress, !dspi->irq);



	/* Get transfer counter (in number of SPI transfers). It was

	 * reset to 0 when transfer(s) were started.
@@ -667,6 +668,7 @@ static int dspi_rxtx(struct fsl_dspi *dspi) 
	spi_tcnt = SPI_TCR_GET_TCNT(spi_tcr);

	/* Update total number of bytes that were transferred */

	msg->actual_length += spi_tcnt * dspi->bytes_per_word;

	dspi->progress += spi_tcnt;



	trans_mode = dspi->devtype_data->trans_mode;

	if (trans_mode == DSPI_EOQ_MODE)
@@ -679,7 +681,7 @@ static int dspi_rxtx(struct fsl_dspi *dspi) 
		return 0;



	spi_take_timestamp_pre(dspi->ctlr, dspi->cur_transfer,

			       dspi->tx, !dspi->irq);

			       dspi->progress, !dspi->irq);



	if (trans_mode == DSPI_EOQ_MODE)

		dspi_eoq_write(dspi);
@@ -768,6 +770,7 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr, 
		dspi->rx = transfer->rx_buf;

		dspi->rx_end = dspi->rx + transfer->len;

		dspi->len = transfer->len;

		dspi->progress = 0;

		/* Validated transfer specific frame size (defaults applied) */

		dspi->bits_per_word = transfer->bits_per_word;

		if (transfer->bits_per_word <= 8)
@@ -789,7 +792,7 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr, 
				     SPI_CTARE_DTCP(1));



		spi_take_timestamp_pre(dspi->ctlr, dspi->cur_transfer,

				       dspi->tx, !dspi->irq);

				       dspi->progress, !dspi->irq);



		trans_mode = dspi->devtype_data->trans_mode;

		switch (trans_mode) {

drivers/spi/spi.c

+8 −14
Original line number Diff line number Diff line
@@ -1499,8 +1499,7 @@ static void spi_pump_messages(struct kthread_work *work) 
 *			    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.

 * @progress: How many words (not bytes) have been transferred so far

 * @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
@@ -1510,21 +1509,19 @@ static void spi_pump_messages(struct kthread_work *work) 
 */

void spi_take_timestamp_pre(struct spi_controller *ctlr,

			    struct spi_transfer *xfer,

			    const void *tx, bool irqs_off)

			    size_t progress, 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))

	if (progress < xfer->ptp_sts_word_pre)

		return;



	/* Capture the resolution of the timestamp */

	xfer->ptp_sts_word_pre = (tx - xfer->tx_buf) / bytes_per_word;

	xfer->ptp_sts_word_pre = progress;



	xfer->timestamped_pre = true;
@@ -1546,23 +1543,20 @@ EXPORT_SYMBOL_GPL(spi_take_timestamp_pre); 
 *			     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.

 * @progress: How many words (not bytes) have been transferred so far

 * @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)

			     size_t progress, 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))

	if (progress < xfer->ptp_sts_word_post)

		return;



	ptp_read_system_postts(xfer->ptp_sts);
@@ -1573,7 +1567,7 @@ void spi_take_timestamp_post(struct spi_controller *ctlr, 
	}



	/* Capture the resolution of the timestamp */

	xfer->ptp_sts_word_post = (tx - xfer->tx_buf) / bytes_per_word;

	xfer->ptp_sts_word_post = progress;



	xfer->timestamped_post = true;

}

include/linux/spi/spi.h

+2 −2
Original line number Diff line number Diff line
@@ -689,10 +689,10 @@ extern void spi_finalize_current_transfer(struct spi_controller *ctlr); 
/* Helper calls for driver to timestamp transfer */

void spi_take_timestamp_pre(struct spi_controller *ctlr,

			    struct spi_transfer *xfer,

			    const void *tx, bool irqs_off);

			    size_t progress, bool irqs_off);

void spi_take_timestamp_post(struct spi_controller *ctlr,

			     struct spi_transfer *xfer,

			     const void *tx, bool irqs_off);

			     size_t progress, bool irqs_off);



/* the spi driver core manages memory for the spi_controller classdev */

extern struct spi_controller *__spi_alloc_controller(struct device *host,

CRA Git | Maintained and supported by SUSTech CRA and CCSE