[U-Boot] [PATCH 1/3] spi: omap3: add support for more word lengths
Igor Grinberg
grinberg at compulab.co.il
Mon Oct 14 16:18:32 CEST 2013
Hi Nikita,
On 10/09/13 16:46, Nikita Kiryanov wrote:
> Current implementation only supports 8 bit word lengths, even though
> omap3 can handle anything between 4 and 32.
>
> Update the spi interface to support changing the SPI word length,
> and implement it in omap3_spi driver to support the full range of
> possible word lengths.
> This implementation is backwards compatible by defaulting to the old
> behavior of 8 bit word lengths.
> Also, it required a change to the omap3_spi non static I/O functions,
> but since they are not used anywhere else, no collateral changes are required.
>
> Cc: Tom Rini <trini at ti.com>
> Cc: Igor Grinberg <grinberg at compulab.co.il>
> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki at gmail.com>
> Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
> ---
> drivers/spi/omap3_spi.c | 78 ++++++++++++++++++++++++++++++++++++++-----------
> drivers/spi/omap3_spi.h | 11 ++++---
> include/spi.h | 13 +++++++++
> 3 files changed, 81 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/spi/omap3_spi.c b/drivers/spi/omap3_spi.c
> index 6bac245..add98e1 100644
> --- a/drivers/spi/omap3_spi.c
> +++ b/drivers/spi/omap3_spi.c
> @@ -20,7 +20,7 @@
> #include <asm/io.h>
> #include "omap3_spi.h"
>
> -#define WORD_LEN 8
> +#define SPI_DEFAULT_WORDLEN 8;
As already pointed out, the semicolon should not be there...
> #define SPI_WAIT_TIMEOUT 3000000;
While at this, can you please, send a separate patch
to fix the above semicolon?
>
> static void spi_reset(struct omap3_spi_slave *ds)
> @@ -128,10 +128,32 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> ds->regs = regs;
> ds->freq = max_hz;
> ds->mode = mode;
> + ds->wordlen = SPI_DEFAULT_WORDLEN;
While we know, OMAP SPI controller supports this,
I think many other controllers can also support this feature
and thus their drivers can benefit from this option.
Wouldn't it be better to add the wordlen field to the generic slave
structure instead of the OMAP specific structure?
>
> return &ds->slave;
> }
>
> +int spi_set_wordlen(struct spi_slave *slave, unsigned int wordlen)
> +{
> + struct omap3_spi_slave *ds;
> + int chconf;
> +
> + if (wordlen < 4 || wordlen > 32) {
> + printf("SPI error: unsupported wordlen %d\n", wordlen);
> + return -1;
> + }
> +
> + ds = to_omap3_spi(slave);
> + ds->wordlen = wordlen;
> +
> + chconf = readl(&ds->regs->channel[ds->slave.cs].chconf);
> + chconf &= ~OMAP3_MCSPI_CHCONF_WL_MASK;
> + chconf |= (wordlen - 1) << 7;
> + omap3_spi_write_chconf(ds, chconf);
I would recommend to update the controller just before the transaction
takes place, e.g. in spi_xfer() function, so the spi_set_wordlen() function
will only update the slave structure and will not touch the hardware.
Thus spi_set_wordlen() can be placed in the generic SPI code.
> +
> + return 0;
> +}
> +
> void spi_free_slave(struct spi_slave *slave)
> {
> struct omap3_spi_slave *ds = to_omap3_spi(slave);
> @@ -185,7 +207,7 @@ int spi_claim_bus(struct spi_slave *slave)
>
> /* wordlength */
> conf &= ~OMAP3_MCSPI_CHCONF_WL_MASK;
> - conf |= (WORD_LEN - 1) << 7;
> + conf |= (ds->wordlen - 1) << 7;
>
> /* set chipselect polarity; manage with FORCE */
> if (!(ds->mode & SPI_CS_HIGH))
> @@ -223,7 +245,7 @@ void spi_release_bus(struct spi_slave *slave)
> spi_reset(ds);
> }
>
> -int omap3_spi_write(struct spi_slave *slave, unsigned int len, const u8 *txp,
> +int omap3_spi_write(struct spi_slave *slave, unsigned int len, const void *txp,
> unsigned long flags)
> {
> struct omap3_spi_slave *ds = to_omap3_spi(slave);
> @@ -250,7 +272,13 @@ int omap3_spi_write(struct spi_slave *slave, unsigned int len, const u8 *txp,
> }
> }
> /* Write the data */
> - writel(txp[i], &ds->regs->channel[ds->slave.cs].tx);
> + unsigned int *tx = &ds->regs->channel[ds->slave.cs].tx;
> + if (ds->wordlen > 16)
> + writel(((u32 *)txp)[i], tx);
> + else if (ds->wordlen > 8)
> + writel(((u16 *)txp)[i], tx);
> + else
> + writel(((u8 *)txp)[i], tx);
Can we always have u32 for the txp and use masks ans shifts instead of
the ugly castings?
> }
>
> /* wait to finish of transfer */
> @@ -268,7 +296,7 @@ int omap3_spi_write(struct spi_slave *slave, unsigned int len, const u8 *txp,
> return 0;
> }
>
> -int omap3_spi_read(struct spi_slave *slave, unsigned int len, u8 *rxp,
> +int omap3_spi_read(struct spi_slave *slave, unsigned int len, void *rxp,
> unsigned long flags)
> {
> struct omap3_spi_slave *ds = to_omap3_spi(slave);
> @@ -302,7 +330,13 @@ int omap3_spi_read(struct spi_slave *slave, unsigned int len, u8 *rxp,
> omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_DIS);
>
> /* Read the data */
> - rxp[i] = readl(&ds->regs->channel[ds->slave.cs].rx);
> + unsigned int *rx = &ds->regs->channel[ds->slave.cs].rx;
> + if (ds->wordlen > 16)
> + ((u32 *)rxp)[i] = readl(rx);
> + else if (ds->wordlen > 8)
> + ((u16 *)rxp)[i] = (u16)readl(rx);
> + else
> + ((u8 *)rxp)[i] = (u8)readl(rx);
same here...
> }
>
> if (flags & SPI_XFER_END) {
> @@ -314,8 +348,8 @@ int omap3_spi_read(struct spi_slave *slave, unsigned int len, u8 *rxp,
> }
>
> /*McSPI Transmit Receive Mode*/
> -int omap3_spi_txrx(struct spi_slave *slave,
> - unsigned int len, const u8 *txp, u8 *rxp, unsigned long flags)
> +int omap3_spi_txrx(struct spi_slave *slave, unsigned int len,
> + const void *txp, void *rxp, unsigned long flags)
> {
> struct omap3_spi_slave *ds = to_omap3_spi(slave);
> int timeout = SPI_WAIT_TIMEOUT;
> @@ -344,7 +378,13 @@ int omap3_spi_txrx(struct spi_slave *slave,
> }
> }
> /* Write the data */
> - writel(txp[i], &ds->regs->channel[ds->slave.cs].tx);
> + unsigned int *tx = &ds->regs->channel[ds->slave.cs].tx;
> + if (ds->wordlen > 16)
> + writel(((u32 *)txp)[i], tx);
> + else if (ds->wordlen > 8)
> + writel(((u16 *)txp)[i], tx);
> + else
> + writel(((u8 *)txp)[i], tx);
and here...
>
> /*Read: wait for RX containing data (RXS == 1)*/
> while (!(readl(&ds->regs->channel[ds->slave.cs].chstat) &
> @@ -356,7 +396,13 @@ int omap3_spi_txrx(struct spi_slave *slave,
> }
> }
> /* Read the data */
> - rxp[i] = readl(&ds->regs->channel[ds->slave.cs].rx);
> + unsigned int *rx = &ds->regs->channel[ds->slave.cs].rx;
> + if (ds->wordlen > 16)
> + ((u32 *)rxp)[i] = readl(rx);
> + else if (ds->wordlen > 8)
> + ((u16 *)rxp)[i] = (u16)readl(rx);
> + else
> + ((u8 *)rxp)[i] = (u8)readl(rx);
and here...
> }
> /* Disable the channel */
> omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_DIS);
> @@ -375,14 +421,12 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
> {
> struct omap3_spi_slave *ds = to_omap3_spi(slave);
> unsigned int len;
> - const u8 *txp = dout;
> - u8 *rxp = din;
> int ret = -1;
>
> - if (bitlen % 8)
> + if (bitlen % ds->wordlen)
> return -1;
>
> - len = bitlen / 8;
> + len = bitlen / ds->wordlen;
>
> if (bitlen == 0) { /* only change CS */
> int chconf = readl(&ds->regs->channel[ds->slave.cs].chconf);
> @@ -400,11 +444,11 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
> ret = 0;
> } else {
> if (dout != NULL && din != NULL)
> - ret = omap3_spi_txrx(slave, len, txp, rxp, flags);
> + ret = omap3_spi_txrx(slave, len, dout, din, flags);
> else if (dout != NULL)
> - ret = omap3_spi_write(slave, len, txp, flags);
> + ret = omap3_spi_write(slave, len, dout, flags);
> else if (din != NULL)
> - ret = omap3_spi_read(slave, len, rxp, flags);
> + ret = omap3_spi_read(slave, len, din, flags);
> }
> return ret;
> }
> diff --git a/drivers/spi/omap3_spi.h b/drivers/spi/omap3_spi.h
> index 01537b6..aeddb53 100644
> --- a/drivers/spi/omap3_spi.h
> +++ b/drivers/spi/omap3_spi.h
> @@ -92,6 +92,7 @@ struct omap3_spi_slave {
> struct mcspi *regs;
> unsigned int freq;
> unsigned int mode;
> + unsigned int wordlen;
See above.
> };
>
> static inline struct omap3_spi_slave *to_omap3_spi(struct spi_slave *slave)
> @@ -99,11 +100,13 @@ static inline struct omap3_spi_slave *to_omap3_spi(struct spi_slave *slave)
> return container_of(slave, struct omap3_spi_slave, slave);
> }
>
> -int omap3_spi_txrx(struct spi_slave *slave, unsigned int len, const u8 *txp,
> - u8 *rxp, unsigned long flags);
> -int omap3_spi_write(struct spi_slave *slave, unsigned int len, const u8 *txp,
> +int omap3_spi_set_wordlen(struct spi_slave *slave, unsigned int wordlen);
> +
> +int omap3_spi_txrx(struct spi_slave *slave, unsigned int len, const void *txp,
> + void *rxp, unsigned long flags);
> +int omap3_spi_write(struct spi_slave *slave, unsigned int len, const void *txp,
> unsigned long flags);
> -int omap3_spi_read(struct spi_slave *slave, unsigned int len, u8 *rxp,
> +int omap3_spi_read(struct spi_slave *slave, unsigned int len, void *rxp,
> unsigned long flags);
u32 *?
>
> #endif /* _OMAP3_SPI_H_ */
[...]
> diff --git a/include/spi.h b/include/spi.h
> index c0dab57..ae318ff 100644
> --- a/include/spi.h
> +++ b/include/spi.h
> @@ -149,6 +149,19 @@ int spi_claim_bus(struct spi_slave *slave);
> void spi_release_bus(struct spi_slave *slave);
>
> /*-----------------------------------------------------------------------
> + * Set the word length for SPI transactions
> + *
> + * Set the word length (number of bits per word) for SPI transactions.
> + * This setting is persistent until changed.
> + *
> + * slave: The SPI slave
> + * wordlen: The number of bits in a word
> + *
> + * Returns: 0 on success, -1 on failure.
> + */
> +int spi_set_wordlen(struct spi_slave *slave, unsigned int wordlen);
> +
> +/*-----------------------------------------------------------------------
> * SPI transfer
> *
> * This writes "bitlen" bits out the SPI MOSI port and simultaneously clocks
>
--
Regards,
Igor.
More information about the U-Boot
mailing list