Re: [U-Boot] [PATCH v2] EXYNOS: SPI: Support SPI_PREAMBLE mode

2013-05-21 Thread Minkyu Kang 
Dear Rajeshwari Shinde,

On 12/05/13 00:04, Simon Glass wrote:
 From: Rajeshwari Shinde rajeshwar...@samsung.com
 
 Support interfaces with a preamble before each received message.
 
 We handle this when the client has requested a SPI_XFER_END, meaning
 that we must close of the transaction. In this case we read until we
 see the preamble (or a timeout occurs), skipping all data before and
 including the preamble. The client will receive only data bytes after
 the preamble.
 
 Signed-off-by: Simon Glass s...@chromium.org
 Signed-off-by: Rajeshwari Shinde rajeshwar...@samsung.com
 ---
 Changes in v2:
 - Remove preamable_count variable which is not really needed
 - Fix checkpatch warning (multiple assignments)
 
  drivers/spi/exynos_spi.c | 62 
 +---
  1 file changed, 54 insertions(+), 8 deletions(-)
 
 diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c
 index 607e1cd..b7b2ea5 100644
 --- a/drivers/spi/exynos_spi.c
 +++ b/drivers/spi/exynos_spi.c
 @@ -51,6 +51,7 @@ struct exynos_spi_slave {
   unsigned int mode;
   enum periph_id periph_id;   /* Peripheral ID for this device */
   unsigned int fifo_size;
 + int skip_preamble;
  };
  
  static struct spi_bus *spi_get_bus(unsigned dev_index)
 @@ -105,6 +106,8 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, 
 unsigned int cs,
   else
   spi_slave-fifo_size = 256;
  
 + spi_slave-skip_preamble = 0;
 +
   spi_slave-freq = bus-frequency;
   if (max_hz)
   spi_slave-freq = min(max_hz, spi_slave-freq);
 @@ -217,17 +220,23 @@ static void spi_request_bytes(struct exynos_spi *regs, 
 int count)
   writel(count | SPI_PACKET_CNT_EN, regs-pkt_cnt);
  }
  
 -static void spi_rx_tx(struct exynos_spi_slave *spi_slave, int todo,
 - void **dinp, void const **doutp)
 +static int spi_rx_tx(struct exynos_spi_slave *spi_slave, int todo,
 + void **dinp, void const **doutp, unsigned long flags)
  {
   struct exynos_spi *regs = spi_slave-regs;
   uchar *rxp = *dinp;
   const uchar *txp = *doutp;
   int rx_lvl, tx_lvl;
   uint out_bytes, in_bytes;
 + int toread;
 + unsigned start = get_timer(0);
 + int stopping;
  
   out_bytes = in_bytes = todo;
  
 + stopping = spi_slave-skip_preamble  (flags  SPI_XFER_END) 
 + !(spi_slave-mode  SPI_SLAVE);
 +
   /*
* If there's something to send, do a software reset and set a
* transaction size.
 @@ -238,6 +247,7 @@ static void spi_rx_tx(struct exynos_spi_slave *spi_slave, 
 int todo,
* Bytes are transmitted/received in pairs. Wait to receive all the
* data because then transmission will be done as well.
*/
 + toread = in_bytes;

please add blank line here.

   while (in_bytes) {
   int temp;
  
 @@ -250,13 +260,41 @@ static void spi_rx_tx(struct exynos_spi_slave 
 *spi_slave, int todo,
   }
   if (rx_lvl  0  in_bytes) {

I think, in_bytes is always true.
It's a redundant checking.

   temp = readl(regs-rx_data);
 - if (rxp)
 + if (!rxp  !stopping) {
 + in_bytes--;
 + } else if (spi_slave-skip_preamble) {
 + if (temp == SPI_PREAMBLE_END_BYTE) {
 + spi_slave-skip_preamble = 0;
 + stopping = 0;
 + }
 + } else {
   *rxp++ = temp;
 - in_bytes--;
 + in_bytes--;
 + }
 + toread--;
 + }


I think, this if statement is not logical.
The checking condition is differ, so it looks confused.

How about this?

+   temp = readl(regs-rx_data);
+   if (spi_slave-skip_preamble) {
+   if (temp == SPI_PREAMBLE_END_BYTE) {
+   spi_slave-skip_preamble = 0;
+   stopping = 0;
+   }
+   } else {
+   if (rxp || stopping)
+   *rxp++ = temp;
+   in_bytes--;
+   }
+
+   toread--;


 + /*
 +  * We have run out of input data, but haven't read enough
 +  * bytes after the preamble yet. Read some more, and make
 +  * sure that we transmit dummy bytes too, to keep things
 +  * going.
 +  */

This comment is what for?
please move this comment into the brace.

 + else if (in_bytes  !toread) {

in_bytes.. ditto.

And please fix brace.
} else if (

 +

[U-Boot] [PATCH v2] EXYNOS: SPI: Support SPI_PREAMBLE mode

2013-05-11 Thread Simon Glass 
From: Rajeshwari Shinde rajeshwar...@samsung.com

Support interfaces with a preamble before each received message.

We handle this when the client has requested a SPI_XFER_END, meaning
that we must close of the transaction. In this case we read until we
see the preamble (or a timeout occurs), skipping all data before and
including the preamble. The client will receive only data bytes after
the preamble.

Signed-off-by: Simon Glass s...@chromium.org
Signed-off-by: Rajeshwari Shinde rajeshwar...@samsung.com
---
Changes in v2:
- Remove preamable_count variable which is not really needed
- Fix checkpatch warning (multiple assignments)

 drivers/spi/exynos_spi.c | 62 +---
 1 file changed, 54 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c
index 607e1cd..b7b2ea5 100644
--- a/drivers/spi/exynos_spi.c
+++ b/drivers/spi/exynos_spi.c
@@ -51,6 +51,7 @@ struct exynos_spi_slave {
unsigned int mode;
enum periph_id periph_id;   /* Peripheral ID for this device */
unsigned int fifo_size;
+   int skip_preamble;
 };
 
 static struct spi_bus *spi_get_bus(unsigned dev_index)
@@ -105,6 +106,8 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, 
unsigned int cs,
else
spi_slave-fifo_size = 256;
 
+   spi_slave-skip_preamble = 0;
+
spi_slave-freq = bus-frequency;
if (max_hz)
spi_slave-freq = min(max_hz, spi_slave-freq);
@@ -217,17 +220,23 @@ static void spi_request_bytes(struct exynos_spi *regs, 
int count)
writel(count | SPI_PACKET_CNT_EN, regs-pkt_cnt);
 }
 
-static void spi_rx_tx(struct exynos_spi_slave *spi_slave, int todo,
-   void **dinp, void const **doutp)
+static int spi_rx_tx(struct exynos_spi_slave *spi_slave, int todo,
+   void **dinp, void const **doutp, unsigned long flags)
 {
struct exynos_spi *regs = spi_slave-regs;
uchar *rxp = *dinp;
const uchar *txp = *doutp;
int rx_lvl, tx_lvl;
uint out_bytes, in_bytes;
+   int toread;
+   unsigned start = get_timer(0);
+   int stopping;
 
out_bytes = in_bytes = todo;
 
+   stopping = spi_slave-skip_preamble  (flags  SPI_XFER_END) 
+   !(spi_slave-mode  SPI_SLAVE);
+
/*
 * If there's something to send, do a software reset and set a
 * transaction size.
@@ -238,6 +247,7 @@ static void spi_rx_tx(struct exynos_spi_slave *spi_slave, 
int todo,
 * Bytes are transmitted/received in pairs. Wait to receive all the
 * data because then transmission will be done as well.
 */
+   toread = in_bytes;
while (in_bytes) {
int temp;
 
@@ -250,13 +260,41 @@ static void spi_rx_tx(struct exynos_spi_slave *spi_slave, 
int todo,
}
if (rx_lvl  0  in_bytes) {
temp = readl(regs-rx_data);
-   if (rxp)
+   if (!rxp  !stopping) {
+   in_bytes--;
+   } else if (spi_slave-skip_preamble) {
+   if (temp == SPI_PREAMBLE_END_BYTE) {
+   spi_slave-skip_preamble = 0;
+   stopping = 0;
+   }
+   } else {
*rxp++ = temp;
-   in_bytes--;
+   in_bytes--;
+   }
+   toread--;
+   }
+   /*
+* We have run out of input data, but haven't read enough
+* bytes after the preamble yet. Read some more, and make
+* sure that we transmit dummy bytes too, to keep things
+* going.
+*/
+   else if (in_bytes  !toread) {
+   assert(!out_bytes);
+   out_bytes = in_bytes;
+   toread = in_bytes;
+   txp = NULL;
+   spi_request_bytes(regs, toread);
+   }
+   if (spi_slave-skip_preamble  get_timer(start)  100) {
+   printf(SPI timeout: in_bytes=%d, out_bytes=%d, ,
+  in_bytes, out_bytes);
+   return -1;
}
}
*dinp = rxp;
*doutp = txp;
+   return 0;
 }
 
 /**
@@ -276,6 +314,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, 
const void *dout,
struct exynos_spi_slave *spi_slave = to_exynos_spi(slave);
int upto, todo;
int bytelen;
+   int ret = 0;
 
/* spi core configured to do 8 bit transfers */
if (bitlen % 8) {
@@ -289,16 +328,22 @@ int spi_xfer(struct spi_slave *slave, unsigned int 
bitlen, const void *dout,
 
/* Exynos

Re: [U-Boot] [PATCH v2] EXYNOS: SPI: Support SPI_PREAMBLE mode

2013-05-11 Thread Simon Glass 
On Sat, May 11, 2013 at 9:04 AM, Simon Glass s...@chromium.org wrote:
 From: Rajeshwari Shinde rajeshwar...@samsung.com

 Support interfaces with a preamble before each received message.

 We handle this when the client has requested a SPI_XFER_END, meaning
 that we must close of the transaction. In this case we read until we
 see the preamble (or a timeout occurs), skipping all data before and
 including the preamble. The client will receive only data bytes after
 the preamble.

 Signed-off-by: Simon Glass s...@chromium.org
 Signed-off-by: Rajeshwari Shinde rajeshwar...@samsung.com
 ---
 Changes in v2:
 - Remove preamable_count variable which is not really needed
 - Fix checkpatch warning (multiple assignments)

This v2 patch fixes the nits reported.

Acked-by: Simon Glass s...@chromium.org
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot