Re: [U-Boot] [PATCH 2/2 V5] spi: exynos: Support SPI_PREAMBLE mode

2013-06-02 Thread Simon Glass
Hi,

On Sun, Jun 2, 2013 at 10:41 AM, Jagan Teki wrote:

> Hi,
>
> I know this has been reviewed multiples time, but I have few questions on
> it.
>
> I think this preamble is one of spi mode characteristic, if so does it
> specific to a hw?
>
> On Wed, May 29, 2013 at 11:40 AM, Rajeshwari Shinde
>  wrote:
> > 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 
> > Signed-off-by: Rajeshwari Shinde 
> > ---
> > Changes in V2:
> > - Remove preamable_count variable which is not really needed
> > - Fix checkpatch warning (multiple assignments)
> > Changes in V3:
> > - Modified the if logic in spi_rx_tx function
> > - Added blank lines as suggested by Minkyu Kang.
> > - Removed in_bytes check in while loop.
> > - Added a error check.
> > Changes in V4:
> > - Corrected a if condition.
> > Changes in V5:
> > - In commit message header changed EXYNOS: SPI: to spi:exynos.
> >  drivers/spi/exynos_spi.c |   69
> +++--
> >  1 files changed, 59 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c
> > index 607e1cd..01378d0 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, ®s->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,8 @@ 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;
> >
> > @@ -248,15 +259,43 @@ static void spi_rx_tx(struct exynos_spi_slave
> *spi_slave, int todo,
> > writel(temp, ®s->tx_data);
> > out_bytes--;
> > }
> > -   if (rx_lvl > 0 && in_bytes) {
> > +   if (rx_lvl > 0) {
> > temp = readl(®s->rx_data);
> > -   if (rxp)
> > -   *rxp++ = temp;
> > -   in_bytes--;
> > +   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--;
> > +   } else if (!toread) {
> > +   /*
> > +* We have run out of input data, but haven't
> read
> > +* enough bytes after the preamble yet. R

Re: [U-Boot] [PATCH 2/2 V5] spi: exynos: Support SPI_PREAMBLE mode

2013-06-02 Thread Jagan Teki
Hi,

I know this has been reviewed multiples time, but I have few questions on it.

I think this preamble is one of spi mode characteristic, if so does it
specific to a hw?

On Wed, May 29, 2013 at 11:40 AM, Rajeshwari Shinde
 wrote:
> 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 
> Signed-off-by: Rajeshwari Shinde 
> ---
> Changes in V2:
> - Remove preamable_count variable which is not really needed
> - Fix checkpatch warning (multiple assignments)
> Changes in V3:
> - Modified the if logic in spi_rx_tx function
> - Added blank lines as suggested by Minkyu Kang.
> - Removed in_bytes check in while loop.
> - Added a error check.
> Changes in V4:
> - Corrected a if condition.
> Changes in V5:
> - In commit message header changed EXYNOS: SPI: to spi:exynos.
>  drivers/spi/exynos_spi.c |   69 +++--
>  1 files changed, 59 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c
> index 607e1cd..01378d0 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, ®s->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,8 @@ 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;
>
> @@ -248,15 +259,43 @@ static void spi_rx_tx(struct exynos_spi_slave 
> *spi_slave, int todo,
> writel(temp, ®s->tx_data);
> out_bytes--;
> }
> -   if (rx_lvl > 0 && in_bytes) {
> +   if (rx_lvl > 0) {
> temp = readl(®s->rx_data);
> -   if (rxp)
> -   *rxp++ = temp;
> -   in_bytes--;
> +   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--;
> +   } else if (!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.
> +*/
> +   assert(!out_bytes);
> +   out_bytes = in_bytes;
> + 

[U-Boot] [PATCH 2/2 V5] spi: exynos: Support SPI_PREAMBLE mode

2013-05-28 Thread Rajeshwari Shinde
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 
Signed-off-by: Rajeshwari Shinde 
---
Changes in V2:
- Remove preamable_count variable which is not really needed
- Fix checkpatch warning (multiple assignments)
Changes in V3:
- Modified the if logic in spi_rx_tx function
- Added blank lines as suggested by Minkyu Kang.
- Removed in_bytes check in while loop.
- Added a error check.
Changes in V4:
- Corrected a if condition.
Changes in V5:
- In commit message header changed EXYNOS: SPI: to spi:exynos.
 drivers/spi/exynos_spi.c |   69 +++--
 1 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c
index 607e1cd..01378d0 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, ®s->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,8 @@ 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;
 
@@ -248,15 +259,43 @@ static void spi_rx_tx(struct exynos_spi_slave *spi_slave, 
int todo,
writel(temp, ®s->tx_data);
out_bytes--;
}
-   if (rx_lvl > 0 && in_bytes) {
+   if (rx_lvl > 0) {
temp = readl(®s->rx_data);
-   if (rxp)
-   *rxp++ = temp;
-   in_bytes--;
+   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--;
+   } else if (!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.
+*/
+   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;
+
+   retu