[PATCH 6/7] spi: spi-fsl-dspi: split the resuable code

[PATCH 6/7] spi: spi-fsl-dspi: split the resuable code

[Bhuvanchandra DV]

From: Chao Fu <B44548-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Add two functions:
- dspi_data_from_popr
- dspi_data_to_pushr

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv-2KBjVHiyJgBBDgjK7y7TUQ@public.gmane.org>
---
 drivers/spi/spi-fsl-dspi.c | 118 +++++++++++++++++++++++----------------------
 1 file changed, 60 insertions(+), 58 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index b20dbb5..e0ce906 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -167,12 +167,68 @@ static void hz_to_spi_baud(char *pbr, char *br, int speed_hz,
 	*br =  ARRAY_SIZE(brs) - 1;
 }
 
+static u32 dspi_data_to_pushr(struct fsl_dspi *dspi, int tx_word)
+{
+	u16 d16;
+	u8 d8;
+
+	if (tx_word) {
+		if (!(dspi->dataflags & TRAN_STATE_TX_VOID)) {
+			d16 = *(u16 *)dspi->tx;
+			dspi->tx += 2;
+		} else {
+			d16 = dspi->void_write_data;
+		}
+
+		dspi->len -= 2;
+
+		return	SPI_PUSHR_TXDATA(d16) |
+			SPI_PUSHR_PCS(dspi->cs) |
+			SPI_PUSHR_CTAS(dspi->cs) |
+			SPI_PUSHR_CONT;
+	} else {
+		if (!(dspi->dataflags & TRAN_STATE_TX_VOID)) {
+
+			d8 = *(u8 *)dspi->tx;
+			dspi->tx++;
+		} else {
+			d8 = (u8)dspi->void_write_data;
+		}
+
+		dspi->len--;
+
+		return SPI_PUSHR_TXDATA(d8) |
+			SPI_PUSHR_PCS(dspi->cs) |
+			SPI_PUSHR_CTAS(dspi->cs) |
+			SPI_PUSHR_CONT;
+	}
+}
+
+static void dspi_data_from_popr(struct fsl_dspi *dspi, int rx_word)
+{
+	u16 d;
+	unsigned int val;
+
+	if (rx_word) {
+		regmap_read(dspi->regmap, SPI_POPR, &val);
+		d = SPI_POPR_RXDATA(val);
+
+		if (!(dspi->dataflags & TRAN_STATE_RX_VOID))
+			*(u16 *)dspi->rx = d;
+		dspi->rx += 2;
+	} else {
+		regmap_read(dspi->regmap, SPI_POPR, &val);
+		d = SPI_POPR_RXDATA(val);
+		if (!(dspi->dataflags & TRAN_STATE_RX_VOID))
+			*(u8 *)dspi->rx = d;
+		dspi->rx++;
+	}
+}
+
 static int dspi_transfer_write(struct fsl_dspi *dspi)
 {
 	int tx_count = 0;
 	int tx_word;
-	u16 d16;
-	u8  d8;
 	u32 dspi_pushr = 0;
 	int first = 1;
 
@@ -190,39 +246,7 @@ static int dspi_transfer_write(struct fsl_dspi *dspi)
 	}
 
 	while (dspi->len && (tx_count < DSPI_FIFO_SIZE)) {
-		if (tx_word) {
-			if (dspi->len == 1)
-				break;
-
-			if (!(dspi->dataflags & TRAN_STATE_TX_VOID)) {
-				d16 = *(u16 *)dspi->tx;
-				dspi->tx += 2;
-			} else {
-				d16 = dspi->void_write_data;
-			}
-
-			dspi_pushr = SPI_PUSHR_TXDATA(d16) |
-				SPI_PUSHR_PCS(dspi->cs) |
-				SPI_PUSHR_CTAS(dspi->cs) |
-				SPI_PUSHR_CONT;
-
-			dspi->len -= 2;
-		} else {
-			if (!(dspi->dataflags & TRAN_STATE_TX_VOID)) {
-
-				d8 = *(u8 *)dspi->tx;
-				dspi->tx++;
-			} else {
-				d8 = (u8)dspi->void_write_data;
-			}
-
-			dspi_pushr = SPI_PUSHR_TXDATA(d8) |
-				SPI_PUSHR_PCS(dspi->cs) |
-				SPI_PUSHR_CTAS(dspi->cs) |
-				SPI_PUSHR_CONT;
-
-			dspi->len--;
-		}
+		dspi_pushr = dspi_data_to_pushr(dspi, tx_word);
 
 		if (dspi->len == 0 || tx_count == DSPI_FIFO_SIZE - 1) {
 			/* last transfer in the transfer */
@@ -249,32 +273,10 @@ static int dspi_transfer_read(struct fsl_dspi *dspi)
 {
 	int rx_count = 0;
 	int rx_word = is_double_byte_mode(dspi);
-	u16 d;
 
 	while ((dspi->rx < dspi->rx_end)
 			&& (rx_count < DSPI_FIFO_SIZE)) {
-		if (rx_word) {
-			unsigned int val;
-
-			if ((dspi->rx_end - dspi->rx) == 1)
-				break;
-
-			regmap_read(dspi->regmap, SPI_POPR, &val);
-			d = SPI_POPR_RXDATA(val);
-
-			if (!(dspi->dataflags & TRAN_STATE_RX_VOID))
-				*(u16 *)dspi->rx = d;
-			dspi->rx += 2;
-
-		} else {
-			unsigned int val;
-
-			regmap_read(dspi->regmap, SPI_POPR, &val);
-			d = SPI_POPR_RXDATA(val);
-			if (!(dspi->dataflags & TRAN_STATE_RX_VOID))
-				*(u8 *)dspi->rx = d;
-			dspi->rx++;
-		}
+		dspi_data_from_popr(dspi, rx_word);
 		rx_count++;
 	}
 
-- 
2.2.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Bhuvanchandra DV]

From: Chao Fu <B44548-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

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-2KBjVHiyJgBBDgjK7y7TUQ@public.gmane.org>
---
 .../devicetree/bindings/spi/spi-fsl-dspi.txt       |  2 +
 drivers/spi/spi-fsl-dspi.c                         | 74 +++++++++++++++++++---
 2 files changed, 68 insertions(+), 8 deletions(-)

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");
-- 
2.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Mark Brown]

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

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 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 7/7] spi: spi-fsl-dspi: Add support for TCFQ transfer mode

[BhuvanChandra.DV]

On 01/29/2015 12:33 AM, Mark Brown wrote:
> 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.

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?

@Chao, is there any specific reason to implement this mode?

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[BhuvanChandra.DV]

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.

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

[Stefan Agner]

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 linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 634 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]