[U-Boot] [PATCH 2/2 V5] spi: exynos: Support SPI_PREAMBLE mode
Simon Glass
sjg at chromium.org
Sun Jun 2 20:00:47 CEST 2013
Hi,
On Sun, Jun 2, 2013 at 10:41 AM, Jagan Teki <jagannadh.teki at gmail.com>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
> <rajeshwari.s at samsung.com> 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 <sjg at chromium.org>
> > Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>
> > ---
> > 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;
> > +
> > + return 0;
> > }
> >
> > /**
> > @@ -276,6 +315,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 +329,24 @@ 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);
> > + 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 +373,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;
>
> So for each cs active, we can assign the state of prempble to
> skip_preamble and then
> we check if the preamble is there on below logic
> + if (spi_slave->skip_preamble) {
> + if (temp == SPI_PREAMBLE_END_BYTE) {
> + spi_slave->skip_preamble = 0;
> + stopping = 0;
> + }
>
> If the preamble is there, read the rx_data for preamble data.
> If ie the preamble data then add the skip to 0. and continue to read
> the next data as read rxed data by
> skipping preamble with toread--.
>
The problem here is is that the U-Boot SPI interface doesn't quite allow
this. For example, 'sf probe' does a short write, then in a separate
spi_xfer() call it does a read. Should the preamble be waited for in the
first write? Perhaps not, if more bytes need to be sent before the other
end can respond. It is a little complicated and we are working on another
approach. But this code does work and is heavily tested. I would be
reluctant to change it in mainline without further testing. We are working
on generalising this.
> Was my understanding is correct, please correct me if am wrong.
>
> --
> Thanks,
> Jagan.
>
Regards,
Simon
More information about the U-Boot
mailing list