diff mbox

[7/7] spi: spi-fsl-dspi: Add support for TCFQ transfer mode

Message ID 1422356608-15961-2-git-send-email-bhuvanchandra.dv@toradex.com
State Needs Review / ACK, archived
Headers show

Checks

Context Check Description
robh/checkpatch warning total: 1 errors, 0 warnings, 0 lines checked
robh/patch-applied success

Commit Message

Bhuvanchandra DV Jan. 27, 2015, 11:03 a.m. UTC 
From: Chao Fu <B44548@freescale.com>

TCFQ is interrupt of Transfer Complete Flag in DSPI module.
EOQ is interrupt of End of Queue Flag in DSPI module.
For adopting of different platform, either of them is a way of DSPI
transfer data. This patch add TCF support for DSPI module  in other platform.

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
---
 .../devicetree/bindings/spi/spi-fsl-dspi.txt       |  2 +
 drivers/spi/spi-fsl-dspi.c                         | 74 +++++++++++++++++++---
 2 files changed, 68 insertions(+), 8 deletions(-)

Comments

Mark Brown Jan. 28, 2015, 7:03 p.m. UTC | #1 
On Tue, Jan 27, 2015 at 04:33:28PM +0530, Bhuvanchandra DV wrote:

> TCFQ is interrupt of Transfer Complete Flag in DSPI module.
> EOQ is interrupt of End of Queue Flag in DSPI module.
> For adopting of different platform, either of them is a way of DSPI
> transfer data. This patch add TCF support for DSPI module  in other platform.

I can't really understand what the purpose of this is, sorry.  What are
these modes and why would one choose one over the other?  Your changelog
suggests this is being done per board which implies that it's tuning for
performance but that's usually something that should be done in device
drivers, not the device tree - perhaps the tuning will change over time
as the driver is optimized or perhaps it varies depending on the
transfer.

The code mentions a DMA option as well, generally where it's available
DMA is something that's chosen at runtime based on the length of the
transfer.
Mark Brown Jan. 29, 2015, 12:13 p.m. UTC | #2 
On Thu, Jan 29, 2015 at 11:58:25AM +0000, BhuvanChandra.DV wrote:

> As far as i understood the major difference between the two modes are when
> the interrupt to trigger, as EOQ mode will trigger the interrupt at end of
> queue and TCF will trigger the interrupt at every frame transfer. Probably
> mode selection shouldn't be a device tree property, but i don't see any
> automatic way to choose between the modes.
> Maybe a config would be more appropriate?

Or if there's either no particular reason to choose one over the other
or one is always better then just pick one in the driver and don't worry
about implementing both.
Stefan Agner Jan. 29, 2015, 6:10 p.m. UTC | #3 
On 2015-01-29 13:53, BhuvanChandra.DV wrote:
> On 01/29/2015 05:43 PM, Mark Brown wrote:
>> On Thu, Jan 29, 2015 at 11:58:25AM +0000, BhuvanChandra.DV wrote:
>>
>>> As far as i understood the major difference between the two modes are when
>>> the interrupt to trigger, as EOQ mode will trigger the interrupt at end of
>>> queue and TCF will trigger the interrupt at every frame transfer. Probably
>>> mode selection shouldn't be a device tree property, but i don't see any
>>> automatic way to choose between the modes.
>>> Maybe a config would be more appropriate?
>> Or if there's either no particular reason to choose one over the other
>> or one is always better then just pick one in the driver and don't worry
>> about implementing both.
> 
> Among the two, EOQ would be better since with TCF, interrupts are generated at
> every frame transfer, which could be a problem at higher frequencies.
> I think we can omit this patch then.

It would be interesting to know what the authors original motivation
implementing this feature was. Reading the email of the original
patchset indicates that there are platforms where only TCF is supported:

<quote>
For adopting of different platform, either of them is a way of DSPI
transfer data.
</quote>

However, I don't know which platform that would be. Also, it seems that
Chao Fu's email is not valid anymore. 

@Xiubo Li, maybe you can shed some light on this?

>From the platform I am concerned about, Vybrid, it seems not very
useful, so I'm fine with omitting that patch.

--
Stefan 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Jan. 30, 2015, 12:30 p.m. UTC | #4 
On Thu, Jan 29, 2015 at 07:10:31PM +0100, Stefan Agner wrote:

> It would be interesting to know what the authors original motivation
> implementing this feature was. Reading the email of the original
> patchset indicates that there are platforms where only TCF is supported:

> <quote>
> For adopting of different platform, either of them is a way of DSPI
> transfer data.
> </quote>

> However, I don't know which platform that would be. Also, it seems that
> Chao Fu's email is not valid anymore. 

It's not clear to me if the above means that this is due to hardware
differences or due to tuning for the platform.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
index cbbe16e..b50439f 100644
--- a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
+++ b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
@@ -15,6 +15,8 @@  Optional property:
 - big-endian: If present the dspi device's registers are implemented
   in big endian mode, otherwise in native mode(same with CPU), for more
   detail please see: Documentation/devicetree/bindings/regmap/regmap.txt.
+- tcfq-mode: If present, the data transfer will be done at TCFQ interrupt.
+  By default, driver chooses EOQ interrupt if 'tcfq-mode' property was not set.
 
 Example:
 
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index e0ce906..feca2fd 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -63,9 +63,11 @@ 
 #define SPI_CTAR0_SLAVE	0x0c
 
 #define SPI_SR			0x2c
+#define SPI_SR_TCFQF		0x80000000
 #define SPI_SR_EOQF		0x10000000
 
 #define SPI_RSER		0x30
+#define SPI_RSER_TCFQE		0x80000000
 #define SPI_RSER_EOQFE		0x10000000
 
 #define SPI_PUSHR		0x34
@@ -105,6 +107,12 @@  struct chip_data {
 	u16 void_write_data;
 };
 
+enum dspi_trans_mode {
+	DSPI_EOQ_MODE = 0,
+	DSPI_TCFQ_MODE,
+	DSPI_DMA_MODE, /*TODO*/
+};
+
 struct fsl_dspi {
 	struct spi_master	*master;
 	struct platform_device	*pdev;
@@ -125,6 +133,7 @@  struct fsl_dspi {
 	u8			cs;
 	u16			void_write_data;
 	u32			cs_change;
+	enum dspi_trans_mode	trans_mode;
 
 	wait_queue_head_t	waitq;
 	u32			waitflags;
@@ -225,7 +234,7 @@  static void dspi_data_from_popr(struct fsl_dspi *dspi, int rx_word)
 	}
 }
 
-static int dspi_transfer_write(struct fsl_dspi *dspi)
+static int dspi_eoq_write(struct fsl_dspi *dspi)
 {
 	int tx_count = 0;
 	int tx_word;
@@ -269,7 +278,7 @@  static int dspi_transfer_write(struct fsl_dspi *dspi)
 	return tx_count * (tx_word + 1);
 }
 
-static int dspi_transfer_read(struct fsl_dspi *dspi)
+static int dspi_eoq_read(struct fsl_dspi *dspi)
 {
 	int rx_count = 0;
 	int rx_word = is_double_byte_mode(dspi);
@@ -283,6 +292,37 @@  static int dspi_transfer_read(struct fsl_dspi *dspi)
 	return rx_count;
 }
 
+static int dspi_tcfq_write(struct fsl_dspi *dspi)
+{
+	int tx_word;
+	u32 dspi_pushr = 0;
+
+	tx_word = is_double_byte_mode(dspi);
+
+	if (tx_word && (dspi->len == 1)) {
+		dspi->dataflags |= TRAN_STATE_WORD_ODD_NUM;
+		regmap_update_bits(dspi->regmap, SPI_CTAR(dspi->cs),
+				SPI_FRAME_BITS_MASK, SPI_FRAME_BITS(8));
+		tx_word = 0;
+	}
+
+	dspi_pushr = dspi_data_to_pushr(dspi, tx_word);
+
+	if ((dspi->cs_change) && (!dspi->len))
+		dspi_pushr &= ~SPI_PUSHR_CONT;
+
+	regmap_write(dspi->regmap, SPI_PUSHR, dspi_pushr);
+
+	return tx_word + 1;
+}
+
+static void dspi_tcfq_read(struct fsl_dspi *dspi)
+{
+	int rx_word = is_double_byte_mode(dspi);
+
+	dspi_data_from_popr(dspi, rx_word);
+}
+
 static int dspi_transfer_one_message(struct spi_master *master,
 		struct spi_message *message)
 {
@@ -326,8 +366,13 @@  static int dspi_transfer_one_message(struct spi_master *master,
 			regmap_write(dspi->regmap, SPI_CTAR(dspi->cs),
 					dspi->cur_chip->ctar_val);
 
-		regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE);
-		message->actual_length += dspi_transfer_write(dspi);
+		if (dspi->trans_mode == DSPI_EOQ_MODE) {
+			regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE);
+			message->actual_length += dspi_eoq_write(dspi);
+		} else if (dspi->trans_mode == DSPI_TCFQ_MODE) {
+			regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_TCFQE);
+			message->actual_length += dspi_tcfq_write(dspi);
+		}
 
 		if (wait_event_interruptible(dspi->waitq, dspi->waitflags))
 			dev_err(&dspi->pdev->dev, "wait transfer complete fail!\n");
@@ -399,8 +444,13 @@  static irqreturn_t dspi_interrupt(int irq, void *dev_id)
 
 	struct spi_message *msg = dspi->cur_msg;
 
-	regmap_write(dspi->regmap, SPI_SR, SPI_SR_EOQF);
-	dspi_transfer_read(dspi);
+	if (dspi->trans_mode == DSPI_EOQ_MODE) {
+		regmap_write(dspi->regmap, SPI_SR, SPI_SR_EOQF);
+		dspi_eoq_read(dspi);
+	} else if (dspi->trans_mode == DSPI_TCFQ_MODE) {
+		regmap_write(dspi->regmap, SPI_SR, SPI_SR_TCFQF);
+		dspi_tcfq_read(dspi);
+	}
 
 	if (!dspi->len) {
 		if (dspi->dataflags & TRAN_STATE_WORD_ODD_NUM)
@@ -409,8 +459,12 @@  static irqreturn_t dspi_interrupt(int irq, void *dev_id)
 
 		dspi->waitflags = 1;
 		wake_up_interruptible(&dspi->waitq);
-	} else
-		msg->actual_length += dspi_transfer_write(dspi);
+	} else {
+		if (dspi->trans_mode == DSPI_EOQ_MODE)
+			msg->actual_length += dspi_eoq_write(dspi);
+		else if (dspi->trans_mode == DSPI_TCFQ_MODE)
+			msg->actual_length += dspi_tcfq_write(dspi);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -470,6 +524,7 @@  static int dspi_probe(struct platform_device *pdev)
 	dspi = spi_master_get_devdata(master);
 	dspi->pdev = pdev;
 	dspi->master = master;
+	dspi->trans_mode = DSPI_EOQ_MODE;
 
 	master->transfer = NULL;
 	master->setup = dspi_setup;
@@ -481,6 +536,9 @@  static int dspi_probe(struct platform_device *pdev)
 	master->bits_per_word_mask = SPI_BPW_MASK(4) | SPI_BPW_MASK(8) |
 					SPI_BPW_MASK(16);
 
+	if (of_property_read_bool(np, "tcfq-mode"))
+		dspi->trans_mode = DSPI_TCFQ_MODE;
+
 	ret = of_property_read_u32(np, "spi-num-chipselects", &cs_num);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "can't get spi-num-chipselects\n");