From mboxrd@z Thu Jan 1 00:00:00 1970 From: Minkyu Kang Date: Tue, 21 May 2013 21:30:51 +0900 Subject: [U-Boot] [PATCH v2] EXYNOS: SPI: Support SPI_PREAMBLE mode In-Reply-To: <1368284655-10201-1-git-send-email-sjg@chromium.org> References: <1363933766-6555-3-git-send-email-rajeshwari.s@samsung.com> <1368284655-10201-1-git-send-email-sjg@chromium.org> Message-ID: <519B68FB.6050405@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Rajeshwari Shinde, On 12/05/13 00:04, Simon Glass wrote: > From: 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) > > 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, ®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,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(®s->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(®s->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 ( > + 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; please add blank line here. > + 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 SPI limits each transfer to 65535 bytes */ > bytelen = bitlen / 8; > - for (upto = 0; upto < bytelen; upto += todo) { > + for (upto = 0; !ret && upto < bytelen; upto += todo) { > todo = min(bytelen - upto, (1 << 16) - 1); > - spi_rx_tx(spi_slave, todo, &din, &dout); > + ret = spi_rx_tx(spi_slave, todo, &din, &dout, flags); It looks good. But I think, it's better to checking the error explicitly. if (!ret) break; > } > > /* Stop the transaction, if necessary. */ > - if ((flags & SPI_XFER_END)) > + if ((flags & SPI_XFER_END) && !(spi_slave->mode & SPI_SLAVE)) { > spi_cs_deactivate(slave); > + if (spi_slave->skip_preamble) { > + assert(!spi_slave->skip_preamble); > + debug("Failed to complete premable transaction\n"); > + ret = -1; > + } > + } > > - return 0; > + return ret; > } > > /** > @@ -325,6 +370,7 @@ void spi_cs_activate(struct spi_slave *slave) > > clrbits_le32(&spi_slave->regs->cs_reg, SPI_SLAVE_SIG_INACT); > debug("Activate CS, bus %d\n", spi_slave->slave.bus); > + spi_slave->skip_preamble = spi_slave->mode & SPI_PREAMBLE; > } > > /** > Thanks, Minkyu Kang.