[PATCH v3 0/1] spi: Using Trigger number to transmit/receive data

[PATCH v3 0/1] spi: Using Trigger number to transmit/receive data

[Cao Minh Hiep]

From: Hiep Cao Minh <cm-hiep@jinso.co.jp>

Hi Geert-san,

I have updated a new version of the QSPI patch (verion 3), that I modified as 
your comments. This patch supports for r8a7790 SOC and developed base on the 
upstream-stable-v3.17 and have tested on the Lager board.
Please consider the following patches for the r8a7790 Soc.

Version 2:
Added the DMA support on it.In order to use DMA When the DMA is available. 
This patch supports for r8a7790 SOC and developed base on the upstream-v3.17-rc7
and have tested on the Lager board.

Version 1:

This is the patch of QSPI that support for r8a7790 SOC. The pupose of 
this patch is to transmit and receive the 32 bytes of data when the data 
already has prepared on Transmit/Receive Buffer. Using Transmit/Receive 
Buffer Data Trigger Number to do that instead transmits/receives a byte 
of data. With this patch will improve the speed of transfer data.
This patch was developed base on the upstream-v3.17-rc5 and have 
tested on the Lager board.

Best Regards,
Cao Minh Hiep 

Hiep Cao Minh (1):
  spi: Using Trigger number to transmit/receive data

 drivers/spi/spi-rspi.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 117 insertions(+), 7 deletions(-)

-- 
1.9.1

[PATCH v3 1/1] spi: Using Trigger number to transmit/receive data

[Cao Minh Hiep]

From: Hiep Cao Minh <cm-hiep-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>

In order to transmit and receive data when have 32 bytes of data that
ready has prepared on Transmit/Receive Buffer to transmit or receive.
Instead transmits/receives a byte data using Transmit/Receive Buffer
Data Triggering Number will improve the speed of transfer data.

Signed-off-by: Hiep Cao Minh <cm-hiep-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>
---
 drivers/spi/spi-rspi.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 117 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index ad87a98..32a2515 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -182,6 +182,13 @@
 #define SPBFCR_RXRST		0x40	/* Receive Buffer Data Reset */
 #define SPBFCR_TXTRG_MASK	0x30	/* Transmit Buffer Data Triggering Number */
 #define SPBFCR_RXTRG_MASK	0x07	/* Receive Buffer Data Triggering Number */
+/* QSPI on R-Car Gen2 */
+#define SPBFCR_TXTRG_1B		0x00	/* 31 bytes (1 byte available) */
+#define SPBFCR_TXTRG_32B	0x30	/* 0 byte (32 bytes available) */
+#define SPBFCR_RXTRG_1B		0x00	/* 1 byte (31 bytes available) */
+#define SPBFCR_RXTRG_32B	0x07	/* 32 bytes (0 byte available) */
+
+#define QSPI_BUFFER_SIZE        32u
 
 struct rspi_data {
 	void __iomem *addr;
@@ -371,6 +378,52 @@ static int qspi_set_config_register(struct rspi_data *rspi, int access_size)
 	return 0;
 }
 
+static void qspi_update(const struct rspi_data *rspi, u8 mask, u8 val, u8 reg)
+{
+	u8 data;
+
+	data = rspi_read8(rspi, reg);
+	data &= ~mask;
+	data |= (val & mask);
+	rspi_write8(rspi, data, reg);
+}
+
+static int qspi_set_send_trigger(struct rspi_data *rspi, unsigned int len)
+{
+	unsigned int n;
+
+	n = min(len, QSPI_BUFFER_SIZE);
+
+	if (len >= QSPI_BUFFER_SIZE) {
+		/* sets triggering number to 32 bytes */
+		qspi_update(rspi, SPBFCR_TXTRG_MASK,
+			     SPBFCR_TXTRG_32B, QSPI_SPBFCR);
+	} else {
+		/* sets triggering number to 1 byte */
+		qspi_update(rspi, SPBFCR_TXTRG_MASK,
+			     SPBFCR_TXTRG_1B, QSPI_SPBFCR);
+	}
+
+	return n;
+}
+
+static void qspi_set_receive_trigger(struct rspi_data *rspi, unsigned int len)
+{
+	unsigned int n;
+
+	n = min(len, QSPI_BUFFER_SIZE);
+
+	if (len >= QSPI_BUFFER_SIZE) {
+		/* sets triggering number to 32 bytes */
+		qspi_update(rspi, SPBFCR_RXTRG_MASK,
+			     SPBFCR_RXTRG_32B, QSPI_SPBFCR);
+	} else {
+		/* sets triggering number to 1 byte */
+		qspi_update(rspi, SPBFCR_RXTRG_MASK,
+			     SPBFCR_RXTRG_1B, QSPI_SPBFCR);
+	}
+}
+
 #define set_config_register(spi, n) spi->ops->set_config_register(spi, n)
 
 static void rspi_enable_irq(const struct rspi_data *rspi, u8 enable)
@@ -614,19 +667,29 @@ static bool rspi_can_dma(struct spi_master *master, struct spi_device *spi,
 	return __rspi_can_dma(rspi, xfer);
 }
 
-static int rspi_common_transfer(struct rspi_data *rspi,
-				struct spi_transfer *xfer)
+static int rspi_dma_check_then_transfer(struct rspi_data *rspi,
+					 struct spi_transfer *xfer)
 {
-	int ret;
-
 	if (rspi->master->can_dma && __rspi_can_dma(rspi, xfer)) {
 		/* rx_buf can be NULL on RSPI on SH in TX-only Mode */
-		ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
+		int ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
 					xfer->rx_buf ? &xfer->rx_sg : NULL);
 		if (ret != -EAGAIN)
-			return ret;
+			return 0;
 	}
 
+	return -EAGAIN;
+}
+
+static int rspi_common_transfer(struct rspi_data *rspi,
+				struct spi_transfer *xfer)
+{
+	int ret;
+
+	ret = rspi_dma_check_then_transfer(rspi, xfer);
+	if (ret != -EAGAIN)
+		return ret;
+
 	ret = rspi_pio_transfer(rspi, xfer->tx_buf, xfer->rx_buf, xfer->len);
 	if (ret < 0)
 		return ret;
@@ -666,12 +729,59 @@ static int rspi_rz_transfer_one(struct spi_master *master,
 	return rspi_common_transfer(rspi, xfer);
 }
 
+static int qspi_trigger_transfer_out_int(struct rspi_data *rspi, const u8 *tx,
+					u8 *rx, unsigned int len)
+{
+	unsigned int i, n, ret;
+	int error;
+
+	while (len > 0) {
+		n = qspi_set_send_trigger(rspi, len);
+		qspi_set_receive_trigger(rspi, len);
+		if (n == QSPI_BUFFER_SIZE) {
+			error = rspi_wait_for_tx_empty(rspi);
+			if (error < 0) {
+				dev_err(&rspi->master->dev, "transmit timeout\n");
+				return error;
+			}
+			for (i = 0; i < n; i++)
+				rspi_write_data(rspi, *tx++);
+
+			error = rspi_wait_for_rx_full(rspi);
+			if (error < 0) {
+				dev_err(&rspi->master->dev, "receive timeout\n");
+				return error;
+			}
+			for (i = 0; i < n; i++)
+				*rx++ = rspi_read_data(rspi);
+		} else {
+			ret = rspi_pio_transfer(rspi, tx, rx, n);
+			if (ret < 0)
+				return ret;
+		}
+		len -= n;
+	}
+
+	return 0;
+}
+
 static int qspi_transfer_out_in(struct rspi_data *rspi,
 				struct spi_transfer *xfer)
 {
+	int ret;
+
 	qspi_receive_init(rspi);
 
-	return rspi_common_transfer(rspi, xfer);
+	ret = rspi_dma_check_then_transfer(rspi, xfer);
+	if (ret != -EAGAIN)
+		return ret;
+
+	ret = qspi_trigger_transfer_out_int(rspi, xfer->tx_buf,
+					    xfer->rx_buf, xfer->len);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static int qspi_transfer_out(struct rspi_data *rspi, struct spi_transfer *xfer)
-- 
1.9.1

--
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 v3 1/1] spi: Using Trigger number to transmit/receive data

[Mark Brown]

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

On Thu, Oct 23, 2014 at 12:14:13PM +0900, Cao Minh Hiep wrote:
> From: Hiep Cao Minh <cm-hiep-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>
> 
> In order to transmit and receive data when have 32 bytes of data that
> ready has prepared on Transmit/Receive Buffer to transmit or receive.
> Instead transmits/receives a byte data using Transmit/Receive Buffer
> Data Triggering Number will improve the speed of transfer data.

Applied, thanks.  It'd have helped to be more explicit about what a
"trigger number" is here - it's not a commonly used term.  "Trigger
level" is more normal.

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

Re: [PATCH v3 1/1] spi: Using Trigger number to transmit/receive data

[Cao Minh Hiep]

Hi Mark-san

On 2015年03月30日 13:30, Mark Brown wrote:
> On Thu, Oct 23, 2014 at 12:14:13PM +0900, Cao Minh Hiep wrote:
>> From: Hiep Cao Minh <cm-hiep@jinso.co.jp>
>>
>> In order to transmit and receive data when have 32 bytes of data that
>> ready has prepared on Transmit/Receive Buffer to transmit or receive.
>> Instead transmits/receives a byte data using Transmit/Receive Buffer
>> Data Triggering Number will improve the speed of transfer data.
> Applied, thanks.  It'd have helped to be more explicit about what a
> "trigger number" is here - it's not a commonly used term.  "Trigger
> level" is more normal.
Thank you very much!
Hiep.

Re: [PATCH v3 1/1] spi: Using Trigger number to transmit/receive data

[Geert Uytterhoeven]

Hi Hiep-san,

On Thu, Oct 23, 2014 at 5:14 AM, Cao Minh Hiep <cm-hiep-HEF513clHfp3+QwDJ9on6Q@public.gmane.org> wrote:
> From: Hiep Cao Minh <cm-hiep-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>
>
> In order to transmit and receive data when have 32 bytes of data that
> ready has prepared on Transmit/Receive Buffer to transmit or receive.
> Instead transmits/receives a byte data using Transmit/Receive Buffer
> Data Triggering Number will improve the speed of transfer data.

Please accept my apologies for these late review comments.
I had completely forgotten about your patch, until I noticed Mark had
applied it.

> --- a/drivers/spi/spi-rspi.c
> +++ b/drivers/spi/spi-rspi.c
> @@ -614,19 +667,29 @@ static bool rspi_can_dma(struct spi_master *master, struct spi_device *spi,
>         return __rspi_can_dma(rspi, xfer);
>  }
>
> -static int rspi_common_transfer(struct rspi_data *rspi,
> -                               struct spi_transfer *xfer)
> +static int rspi_dma_check_then_transfer(struct rspi_data *rspi,
> +                                        struct spi_transfer *xfer)
>  {
> -       int ret;
> -
>         if (rspi->master->can_dma && __rspi_can_dma(rspi, xfer)) {
>                 /* rx_buf can be NULL on RSPI on SH in TX-only Mode */
> -               ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
> +               int ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
>                                         xfer->rx_buf ? &xfer->rx_sg : NULL);
>                 if (ret != -EAGAIN)
> -                       return ret;
> +                       return 0;

Shouldn't you propagate "ret" here, instead of returning zero?
Else you will turn a failure into a success below (see "here").

>         }
>
> +       return -EAGAIN;
> +}
> +
> +static int rspi_common_transfer(struct rspi_data *rspi,
> +                               struct spi_transfer *xfer)
> +{
> +       int ret;
> +
> +       ret = rspi_dma_check_then_transfer(rspi, xfer);
> +       if (ret != -EAGAIN)
> +               return ret;

... here.

> +
>         ret = rspi_pio_transfer(rspi, xfer->tx_buf, xfer->rx_buf, xfer->len);
>         if (ret < 0)
>                 return ret;
> @@ -666,12 +729,59 @@ static int rspi_rz_transfer_one(struct spi_master *master,
>         return rspi_common_transfer(rspi, xfer);
>  }
>
> +static int qspi_trigger_transfer_out_int(struct rspi_data *rspi, const u8 *tx,

I guess this should be called "qspi_trigger_transfer_out_in" (without "t")?

> +                                       u8 *rx, unsigned int len)

>  static int qspi_transfer_out_in(struct rspi_data *rspi,
>                                 struct spi_transfer *xfer)
>  {
> +       int ret;
> +
>         qspi_receive_init(rspi);
>
> -       return rspi_common_transfer(rspi, xfer);
> +       ret = rspi_dma_check_then_transfer(rspi, xfer);
> +       if (ret != -EAGAIN)
> +               return ret;
> +
> +       ret = qspi_trigger_transfer_out_int(rspi, xfer->tx_buf,
> +                                           xfer->rx_buf, xfer->len);
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;

You could just write

      return qspi_trigger_transfer_out_int(rspi, xfer->tx_buf,
                                           xfer->rx_buf, xfer->len);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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 v3 1/1] spi: Using Trigger number to transmit/receive data

[Cao Minh Hiep]

Hi Geert-san
I am sorry for late replying.

On 2015年04月08日 03:51, Geert Uytterhoeven wrote:
> Hi Hiep-san,
>
> On Thu, Oct 23, 2014 at 5:14 AM, Cao Minh Hiep <cm-hiep@jinso.co.jp> wrote:
>> From: Hiep Cao Minh <cm-hiep@jinso.co.jp>
>>
>> In order to transmit and receive data when have 32 bytes of data that
>> ready has prepared on Transmit/Receive Buffer to transmit or receive.
>> Instead transmits/receives a byte data using Transmit/Receive Buffer
>> Data Triggering Number will improve the speed of transfer data.
> Please accept my apologies for these late review comments.
> I had completely forgotten about your patch, until I noticed Mark had
> applied it.
  No problems. Thanks,
>> --- a/drivers/spi/spi-rspi.c
>> +++ b/drivers/spi/spi-rspi.c
>> @@ -614,19 +667,29 @@ static bool rspi_can_dma(struct spi_master *master, struct spi_device *spi,
>>          return __rspi_can_dma(rspi, xfer);
>>   }
>>
>> -static int rspi_common_transfer(struct rspi_data *rspi,
>> -                               struct spi_transfer *xfer)
>> +static int rspi_dma_check_then_transfer(struct rspi_data *rspi,
>> +                                        struct spi_transfer *xfer)
>>   {
>> -       int ret;
>> -
>>          if (rspi->master->can_dma && __rspi_can_dma(rspi, xfer)) {
>>                  /* rx_buf can be NULL on RSPI on SH in TX-only Mode */
>> -               ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
>> +               int ret = rspi_dma_transfer(rspi, &xfer->tx_sg,
>>                                          xfer->rx_buf ? &xfer->rx_sg : NULL);
>>                  if (ret != -EAGAIN)
>> -                       return ret;
>> +                       return 0;
> Shouldn't you propagate "ret" here, instead of returning zero?
> Else you will turn a failure into a success below (see "here").
Thanks, I will keep the "ret" here.
>>          }
>>
>> +       return -EAGAIN;
>> +}
>> +
>> +static int rspi_common_transfer(struct rspi_data *rspi,
>> +                               struct spi_transfer *xfer)
>> +{
>> +       int ret;
>> +
>> +       ret = rspi_dma_check_then_transfer(rspi, xfer);
>> +       if (ret != -EAGAIN)
>> +               return ret;
> ... here.
>
>> +
>>          ret = rspi_pio_transfer(rspi, xfer->tx_buf, xfer->rx_buf, xfer->len);
>>          if (ret < 0)
>>                  return ret;
>> @@ -666,12 +729,59 @@ static int rspi_rz_transfer_one(struct spi_master *master,
>>          return rspi_common_transfer(rspi, xfer);
>>   }
>>
>> +static int qspi_trigger_transfer_out_int(struct rspi_data *rspi, const u8 *tx,
> I guess this should be called "qspi_trigger_transfer_out_in" (without "t")?
>
thanks, I will re-do that.

>> +                                       u8 *rx, unsigned int len)
>>   static int qspi_transfer_out_in(struct rspi_data *rspi,
>>                                  struct spi_transfer *xfer)
>>   {
>> +       int ret;
>> +
>>          qspi_receive_init(rspi);
>>
>> -       return rspi_common_transfer(rspi, xfer);
>> +       ret = rspi_dma_check_then_transfer(rspi, xfer);
>> +       if (ret != -EAGAIN)
>> +               return ret;
>> +
>> +       ret = qspi_trigger_transfer_out_int(rspi, xfer->tx_buf,
>> +                                           xfer->rx_buf, xfer->len);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       return 0;
> You could just write
>
>        return qspi_trigger_transfer_out_int(rspi, xfer->tx_buf,
>                                             xfer->rx_buf, xfer->len);
ok. Thanks,

I will update your comments in other patch, then sending them to you and 
Mark-san.

Best Regards,
Hiep.