[U-Boot] [PATCH v2] TI DaVinci: Driver for the davinci SPI controller

From: Sekhar Nori nsekhar@ti.com
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by: Sekhar Nori nsekhar@ti.com Signed-off-by: Sudhakar Rajashekhara sudhakar.raj@ti.com ---
From the previous version following have been modified:
1. Sorted the entries in drivers/spi/Makefile alphabetically. 2. Implemented dummy functions for spi_cs_is_valid(), spi_cs_activate() and spi_cs_deactivate(). 3. Added GPL header for drivers/spi/davinci_spi.h file. 4. Added protection against multiple inclusion of SPI header file. 5. Replaced the macro based register offsets in SPI header file with structure. 6. Replaced the spi_readl and spi_writel functions with readl and writel respectively.
drivers/spi/Makefile | 1 + drivers/spi/davinci_spi.c | 221 ++++++++++++++++++++++++++++++++++++++++++++ drivers/spi/davinci_spi.h | 102 ++++++++++++++++++++ include/configs/da830evm.h | 2 +- 4 files changed, 325 insertions(+), 1 deletions(-) create mode 100644 drivers/spi/davinci_spi.c create mode 100644 drivers/spi/davinci_spi.h
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 824d8e7..f112ed0 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -29,6 +29,7 @@ COBJS-$(CONFIG_ATMEL_DATAFLASH_SPI) += atmel_dataflash_spi.o COBJS-$(CONFIG_ATMEL_SPI) += atmel_spi.o COBJS-$(CONFIG_BFIN_SPI) += bfin_spi.o COBJS-$(CONFIG_CF_SPI) += cf_spi.o +COBJS-$(CONFIG_DAVINCI_SPI) += davinci_spi.o COBJS-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c new file mode 100644 index 0000000..c3f1810 --- /dev/null +++ b/drivers/spi/davinci_spi.c @@ -0,0 +1,221 @@ +/* + * Copyright (C) 2009 Sekhar Nori, Texas Instruments, Inc <www.ti.com> + * + * Driver for SPI controller on DaVinci. Based on atmel_spi.c + * by Atmel Corporation + * + * Copyright (C) 2007 Atmel Corporation + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ +#include <common.h> +#include <spi.h> +#include <malloc.h> + +#include <asm/io.h> + +#include <asm/arch/hardware.h> + +#include "davinci_spi.h" + +static unsigned int data1_reg_val; + +void spi_init() +{ + /* do nothing */ +} + +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, + unsigned int max_hz, unsigned int mode) +{ + struct davinci_spi_slave *ds; + + ds = malloc(sizeof(struct davinci_spi_slave)); + if (!ds) + return NULL; + + ds->slave.bus = bus; + ds->slave.cs = cs; + ds->regs = (struct davinci_spi_regs *)CONFIG_SYS_SPI_BASE; + ds->freq = max_hz; + + return &ds->slave; +} + +void spi_free_slave(struct spi_slave *slave) +{ + struct davinci_spi_slave *ds = to_davinci_spi(slave); + + free(ds); +} + +int spi_claim_bus(struct spi_slave *slave) +{ + struct davinci_spi_slave *ds = to_davinci_spi(slave); + unsigned int scalar; + + /* Enable the SPI hardware */ + writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); + udelay(1000); + writel(SPIGCR0_SPIENA_MASK, &ds->regs->gcr0); + + /* Set master mode, powered up and not activated */ + writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1); + + /* CS, CLK, SIMO and SOMI are functional pins */ + writel((SPIPC0_EN0FUN_MASK) | (SPIPC0_CLKFUN_MASK) | + (SPIPC0_DOFUN_MASK) | (SPIPC0_DIFUN_MASK), &ds->regs->pc0); + + /* setup format */ + scalar = ((CONFIG_SYS_SPI_CLK / ds->freq) - 1) & 0xFF; + + writel(8 | /* character length */ + (scalar << SPIFMT_PRESCALE_SHIFT) | + /* clock signal delayed by half clk cycle */ + (1 << SPIFMT_PHASE_SHIFT) | + /* clock low in idle state - Mode 0 */ + (0 << SPIFMT_POLARITY_SHIFT) | + /* MSB shifted out first */ + (0 << SPIFMT_SHIFTDIR_SHIFT), &ds->regs->fmt0); + + /* hold cs active at end of transfer until explicitly de-asserted */ + data1_reg_val = (1 << SPIDAT1_CSHOLD_SHIFT) | + (slave->cs << SPIDAT1_CSNR_SHIFT); + writel(data1_reg_val, &ds->regs->dat1); + + /* + * Including a minor delay. No science here. Should be good even with + * no delay + */ + writel((50 << SPI_C2TDELAY_SHIFT) | + (50 << SPI_T2CDELAY_SHIFT), &ds->regs->delay); + + /* default chip select register */ + writel(SPIDEF_CSDEF0_MASK, &ds->regs->def); + + /* no interrupts */ + writel(0, &ds->regs->int0); + writel(0, &ds->regs->lvl); + + /* enable SPI */ + writel(readl(&ds->regs->gcr1) | (SPIGCR1_SPIENA_MASK), &ds->regs->gcr1); + + return 0; +} + +void spi_release_bus(struct spi_slave *slave) +{ + struct davinci_spi_slave *ds = to_davinci_spi(slave); + + /* Disable the SPI hardware */ + writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0); +} + +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, + const void *dout, void *din, unsigned long flags) +{ + struct davinci_spi_slave *ds = to_davinci_spi(slave); + unsigned int len; + int ret, i; + const u8 *txp = dout; + u8 *rxp = din; + + ret = 0; + + if (bitlen == 0) + /* Finish any previously submitted transfers */ + goto out; + + /* + * It's not clear how non-8-bit-aligned transfers are supposed to be + * represented as a stream of bytes...this is a limitation of + * the current SPI interface - here we terminate on receiving such a + * transfer request. + */ + if (bitlen % 8) { + /* Errors always terminate an ongoing transfer */ + flags |= SPI_XFER_END; + goto out; + } + + len = bitlen / 8; + + /* do an empty read to clear the current contents */ + readl(&ds->regs->buf); + + /* keep writing and reading 1 byte until done */ + for (i = 0; i < len; i++) { + /* wait till TXFULL is asserted */ + while (readl(&ds->regs->buf) & (SPIBUF_TXFULL_MASK)); + + /* write the data */ + data1_reg_val &= ~0xFFFF; + if (txp) { + data1_reg_val |= *txp & 0xFF; + txp++; + } + + /* + * Write to DAT1 is required to keep the serial transfer going. + * We just terminate when we reach the end. + */ + if ((i == (len - 1)) && (flags & SPI_XFER_END)) { + /* clear CS hold */ + writel(data1_reg_val & + ~(1 << SPIDAT1_CSHOLD_SHIFT), &ds->regs->dat1); + } else { + writel(data1_reg_val, &ds->regs->dat1); + } + + /* read the data - wait for data availability */ + while (readl(&ds->regs->buf) & (SPIBUF_RXEMPTY_MASK)); + + if (rxp) { + *rxp = readl(&ds->regs->buf) & 0xFF; + rxp++; + } else { + /* simply drop the read character */ + readl(&ds->regs->buf); + } + } + return 0; + +out: + if (flags & SPI_XFER_END) { + writel(data1_reg_val & + ~(1 << SPIDAT1_CSHOLD_SHIFT), &ds->regs->dat1); + } + return 0; +} + +int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{ + return 1; +} + +void spi_cs_activate(struct spi_slave *slave) +{ + /* do nothing */ +} + +void spi_cs_deactivate(struct spi_slave *slave) +{ + /* do nothing */ +} + diff --git a/drivers/spi/davinci_spi.h b/drivers/spi/davinci_spi.h new file mode 100644 index 0000000..5b9a832 --- /dev/null +++ b/drivers/spi/davinci_spi.h @@ -0,0 +1,102 @@ +/* + * Copyright (C) 2009 Sekhar Nori, Texas Instruments, Inc <www.ti.com> + * + * Register definitions for the DaVinci SPI Controller + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#ifndef _DAVINCI_SPI_H_ +#define _DAVINCI_SPI_H_ + +struct davinci_spi_regs { + dv_reg gcr0; /* 0x00 */ + dv_reg gcr1; /* 0x04 */ + dv_reg int0; /* 0x08 */ + dv_reg lvl; /* 0x0c */ + dv_reg flg; /* 0x10 */ + dv_reg pc0; /* 0x14 */ + dv_reg pc1; /* 0x18 */ + dv_reg pc2; /* 0x1c */ + dv_reg pc3; /* 0x20 */ + dv_reg pc4; /* 0x24 */ + dv_reg pc5; /* 0x28 */ + dv_reg rsvd[3]; + dv_reg dat0; /* 0x38 */ + dv_reg dat1; /* 0x3c */ + dv_reg buf; /* 0x40 */ + dv_reg emu; /* 0x44 */ + dv_reg delay; /* 0x48 */ + dv_reg def; /* 0x4c */ + dv_reg fmt0; /* 0x50 */ + dv_reg fmt1; /* 0x54 */ + dv_reg fmt2; /* 0x58 */ + dv_reg fmt3; /* 0x5c */ + dv_reg intvec0; /* 0x60 */ + dv_reg intvec1; /* 0x64 */ +}; + +#define BIT(x) (1 << (x)) + +/* SPIGCR0 */ +#define SPIGCR0_SPIENA_MASK 0x1 +#define SPIGCR0_SPIRST_MASK 0x0 + +/* SPIGCR0 */ +#define SPIGCR1_CLKMOD_MASK BIT(1) +#define SPIGCR1_MASTER_MASK BIT(0) +#define SPIGCR1_SPIENA_MASK BIT(24) + +/* SPIPC0 */ +#define SPIPC0_DIFUN_MASK BIT(11) /* SIMO */ +#define SPIPC0_DOFUN_MASK BIT(10) /* SOMI */ +#define SPIPC0_CLKFUN_MASK BIT(9) /* CLK */ +#define SPIPC0_EN0FUN_MASK BIT(0) + +/* SPIFMT0 */ +#define SPIFMT_SHIFTDIR_SHIFT 20 +#define SPIFMT_POLARITY_SHIFT 17 +#define SPIFMT_PHASE_SHIFT 16 +#define SPIFMT_PRESCALE_SHIFT 8 + +/* SPIDAT1 */ +#define SPIDAT1_CSHOLD_SHIFT 28 +#define SPIDAT1_CSNR_SHIFT 16 + +/* SPIDELAY */ +#define SPI_C2TDELAY_SHIFT 24 +#define SPI_T2CDELAY_SHIFT 16 + +/* SPIBUF */ +#define SPIBUF_RXEMPTY_MASK BIT(31) +#define SPIBUF_TXFULL_MASK BIT(29) + +/* SPIDEF */ +#define SPIDEF_CSDEF0_MASK BIT(0) + +struct davinci_spi_slave { + struct spi_slave slave; + struct davinci_spi_regs *regs; + u32 mr; + unsigned int freq; +}; + +static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) +{ + return container_of(slave, struct davinci_spi_slave, slave); +} + +#endif /* _DAVINCI_SPI_H_ */

On 05/01/10 04:47, Sudhakar Rajashekhara wrote:
From: Sekhar Nori nsekhar@ti.com
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by: Sekhar Nori nsekhar@ti.com Signed-off-by: Sudhakar Rajashekhara sudhakar.raj@ti.com
From the previous version following have been modified:
- Sorted the entries in drivers/spi/Makefile alphabetically.
- Implemented dummy functions for spi_cs_is_valid(), spi_cs_activate() and spi_cs_deactivate().
- Added GPL header for drivers/spi/davinci_spi.h file.
- Added protection against multiple inclusion of SPI header file.
- Replaced the macro based register offsets in SPI header file with structure.
- Replaced the spi_readl and spi_writel functions with readl and writel respectively.
drivers/spi/Makefile | 1 + drivers/spi/davinci_spi.c | 221 ++++++++++++++++++++++++++++++++++++++++++++ drivers/spi/davinci_spi.h | 102 ++++++++++++++++++++ include/configs/da830evm.h | 2 +-
No sign of this file in the patch set. Is this intentional?
4 files changed, 325 insertions(+), 1 deletions(-) create mode 100644 drivers/spi/davinci_spi.c create mode 100644 drivers/spi/davinci_spi.h
...
diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c new file mode 100644 index 0000000..c3f1810 --- /dev/null +++ b/drivers/spi/davinci_spi.c @@ -0,0 +1,221 @@
...
- /* CS, CLK, SIMO and SOMI are functional pins */
- writel((SPIPC0_EN0FUN_MASK) | (SPIPC0_CLKFUN_MASK) |
(SPIPC0_DOFUN_MASK) | (SPIPC0_DIFUN_MASK), &ds->regs->pc0);
There seem to be numerous cases, here and elsewhere in the file where bare defines are referenced within parenthesis for no obvious reason. If they are needed they should be in the #define statement. I think in all cases they are, so the above lines should be something like...
/* CS, CLK, SIMO and SOMI are functional pins */ writel((SPIPC0_EN0FUN_MASK | SPIPC0_CLKFUN_MASK | SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0);
...which also corrects the alignment of the last line.
Regards, Nick.

On Tue, Jan 05, 2010 at 15:22:39, Nick Thompson wrote:
On 05/01/10 04:47, Sudhakar Rajashekhara wrote:
From: Sekhar Nori nsekhar@ti.com
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by: Sekhar Nori nsekhar@ti.com Signed-off-by: Sudhakar Rajashekhara sudhakar.raj@ti.com
From the previous version following have been modified:
- Sorted the entries in drivers/spi/Makefile alphabetically.
- Implemented dummy functions for spi_cs_is_valid(), spi_cs_activate() and spi_cs_deactivate().
- Added GPL header for drivers/spi/davinci_spi.h file.
- Added protection against multiple inclusion of SPI header file.
- Replaced the macro based register offsets in SPI header file with structure.
- Replaced the spi_readl and spi_writel functions with readl and writel respectively.
drivers/spi/Makefile | 1 + drivers/spi/davinci_spi.c | 221 ++++++++++++++++++++++++++++++++++++++++++++ drivers/spi/davinci_spi.h | 102 ++++++++++++++++++++ include/configs/da830evm.h | 2 +-
No sign of this file in the patch set. Is this intentional?
My mistake. I'll correct it.
4 files changed, 325 insertions(+), 1 deletions(-) create mode 100644 drivers/spi/davinci_spi.c create mode 100644 drivers/spi/davinci_spi.h
...
diff --git a/drivers/spi/davinci_spi.c b/drivers/spi/davinci_spi.c new file mode 100644 index 0000000..c3f1810 --- /dev/null +++ b/drivers/spi/davinci_spi.c @@ -0,0 +1,221 @@
...
- /* CS, CLK, SIMO and SOMI are functional pins */
- writel((SPIPC0_EN0FUN_MASK) | (SPIPC0_CLKFUN_MASK) |
(SPIPC0_DOFUN_MASK) | (SPIPC0_DIFUN_MASK), &ds->regs->pc0);
There seem to be numerous cases, here and elsewhere in the file where bare defines are referenced within parenthesis for no obvious reason. If they are needed they should be in the #define statement. I think in all cases they are, so the above lines should be something like...
/* CS, CLK, SIMO and SOMI are functional pins */ writel((SPIPC0_EN0FUN_MASK | SPIPC0_CLKFUN_MASK | SPIPC0_DOFUN_MASK | SPIPC0_DIFUN_MASK), &ds->regs->pc0);
...which also corrects the alignment of the last line.
I'll remove such things and re-submit the patch.
Regards, Sudhakar

Sudhakar Rajashekhara wrote:
From: Sekhar Nori nsekhar@ti.com
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by: Sekhar Nori nsekhar@ti.com Signed-off-by: Sudhakar Rajashekhara sudhakar.raj@ti.com
From the previous version following have been modified:
- Sorted the entries in drivers/spi/Makefile alphabetically.
- Implemented dummy functions for spi_cs_is_valid(), spi_cs_activate() and spi_cs_deactivate().
- Added GPL header for drivers/spi/davinci_spi.h file.
- Added protection against multiple inclusion of SPI header file.
- Replaced the macro based register offsets in SPI header file with structure.
- Replaced the spi_readl and spi_writel functions with readl and writel respectively.
drivers/spi/Makefile | 1 + drivers/spi/davinci_spi.c | 221 ++++++++++++++++++++++++++++++++++++++++++++ drivers/spi/davinci_spi.h | 102 ++++++++++++++++++++ include/configs/da830evm.h | 2 +- 4 files changed, 325 insertions(+), 1 deletions(-) create mode 100644 drivers/spi/davinci_spi.c create mode 100644 drivers/spi/davinci_spi.h
<snip>
new file mode 100644 index 0000000..c3f1810 --- /dev/null +++ b/drivers/spi/davinci_spi.c @@ -0,0 +1,221 @@ +/*
- Copyright (C) 2009 Sekhar Nori, Texas Instruments, Inc <www.ti.com>
- Driver for SPI controller on DaVinci. Based on atmel_spi.c
- by Atmel Corporation
- Copyright (C) 2007 Atmel Corporation
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#include <common.h> +#include <spi.h> +#include <malloc.h>
+#include <asm/io.h>
+#include <asm/arch/hardware.h>
+#include "davinci_spi.h"
Please remove the extra spaces
+static unsigned int data1_reg_val;
Why is this static value used instead of reading ds->regs->dat1 ? Depending on the order of the function calling, this value may not mirror what is in the register.
+void spi_init() +{
- /* do nothing */
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int mode)
+{
- struct davinci_spi_slave *ds;
- ds = malloc(sizeof(struct davinci_spi_slave));
- if (!ds)
return NULL;
- ds->slave.bus = bus;
- ds->slave.cs = cs;
- ds->regs = (struct davinci_spi_regs *)CONFIG_SYS_SPI_BASE;
- ds->freq = max_hz;
- return &ds->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
- struct davinci_spi_slave *ds = to_davinci_spi(slave);
Check before you free. It would be nice if you could poison the pointer.
- free(ds);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
- struct davinci_spi_slave *ds = to_davinci_spi(slave);
- unsigned int scalar;
- /* Enable the SPI hardware */
- writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
- udelay(1000);
- writel(SPIGCR0_SPIENA_MASK, &ds->regs->gcr0);
- /* Set master mode, powered up and not activated */
- writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1);
- /* CS, CLK, SIMO and SOMI are functional pins */
- writel((SPIPC0_EN0FUN_MASK) | (SPIPC0_CLKFUN_MASK) |
(SPIPC0_DOFUN_MASK) | (SPIPC0_DIFUN_MASK), &ds->regs->pc0);
- /* setup format */
- scalar = ((CONFIG_SYS_SPI_CLK / ds->freq) - 1) & 0xFF;
- writel(8 | /* character length */
(scalar << SPIFMT_PRESCALE_SHIFT) |
/* clock signal delayed by half clk cycle */
(1 << SPIFMT_PHASE_SHIFT) |
/* clock low in idle state - Mode 0 */
(0 << SPIFMT_POLARITY_SHIFT) |
/* MSB shifted out first */
(0 << SPIFMT_SHIFTDIR_SHIFT), &ds->regs->fmt0);
Shifting 0's.. This could be improved
- /* hold cs active at end of transfer until explicitly de-asserted */
- data1_reg_val = (1 << SPIDAT1_CSHOLD_SHIFT) |
(slave->cs << SPIDAT1_CSNR_SHIFT);
- writel(data1_reg_val, &ds->regs->dat1);
- /*
* Including a minor delay. No science here. Should be good even with
* no delay
*/
- writel((50 << SPI_C2TDELAY_SHIFT) |
(50 << SPI_T2CDELAY_SHIFT), &ds->regs->delay);
- /* default chip select register */
- writel(SPIDEF_CSDEF0_MASK, &ds->regs->def);
- /* no interrupts */
- writel(0, &ds->regs->int0);
- writel(0, &ds->regs->lvl);
- /* enable SPI */
- writel(readl(&ds->regs->gcr1) | (SPIGCR1_SPIENA_MASK), &ds->regs->gcr1);
- return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{
- struct davinci_spi_slave *ds = to_davinci_spi(slave);
- /* Disable the SPI hardware */
- writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
+}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
const void *dout, void *din, unsigned long flags)
+{
- struct davinci_spi_slave *ds = to_davinci_spi(slave);
- unsigned int len;
- int ret, i;
- const u8 *txp = dout;
- u8 *rxp = din;
It is unclear if passing in NULL values for din and dout is normal behaviour. Please add a comment if it is. Also break transfer loop into a tx / rx parts.
- ret = 0;
- if (bitlen == 0)
/* Finish any previously submitted transfers */
goto out;
- /*
* It's not clear how non-8-bit-aligned transfers are supposed to be
* represented as a stream of bytes...this is a limitation of
* the current SPI interface - here we terminate on receiving such a
* transfer request.
*/
- if (bitlen % 8) {
/* Errors always terminate an ongoing transfer */
flags |= SPI_XFER_END;
It is unclear if you are forcing a flag state that may also be passed in. Please add a comment
goto out;
- }
- len = bitlen / 8;
- /* do an empty read to clear the current contents */
- readl(&ds->regs->buf);
- /* keep writing and reading 1 byte until done */
- for (i = 0; i < len; i++) {
/* wait till TXFULL is asserted */
while (readl(&ds->regs->buf) & (SPIBUF_TXFULL_MASK));
/* write the data */
data1_reg_val &= ~0xFFFF;
if (txp) {
Checking for a valid pointer should happen earlier. If an error happens here, a bogus value of the old data1_reg_val will be used. Move the check higher
data1_reg_val |= *txp & 0xFF;
Adding 0xff should not be necessary for a u8.
txp++;
}
<snip>
diff --git a/drivers/spi/davinci_spi.h b/drivers/spi/davinci_spi.h new file mode 100644 index 0000000..5b9a832 --- /dev/null +++ b/drivers/spi/davinci_spi.h @@ -0,0 +1,102 @@ +/*
<snip>
- Copyright (C) 2009 Sekhar Nori, Texas Instruments, Inc <www.ti.com>
+static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) +{
Check before calling
- return container_of(slave, struct davinci_spi_slave, slave);
+}
+#endif /* _DAVINCI_SPI_H_ */
Tom

Hi,
On Tue, Jan 05, 2010 at 19:49:19, Tom wrote:
Sudhakar Rajashekhara wrote:
From: Sekhar Nori nsekhar@ti.com
This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.
Signed-off-by: Sekhar Nori nsekhar@ti.com Signed-off-by: Sudhakar Rajashekhara sudhakar.raj@ti.com
<snip>
new file mode 100644 index 0000000..c3f1810 --- /dev/null +++ b/drivers/spi/davinci_spi.c @@ -0,0 +1,221 @@ +/*
- Copyright (C) 2009 Sekhar Nori, Texas Instruments, Inc <www.ti.com>
- Driver for SPI controller on DaVinci. Based on atmel_spi.c
- by Atmel Corporation
- Copyright (C) 2007 Atmel Corporation
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#include <common.h> +#include <spi.h> +#include <malloc.h>
+#include <asm/io.h>
+#include <asm/arch/hardware.h>
+#include "davinci_spi.h"
Please remove the extra spaces
I'll remove extra lines between the header file inclusions.
+static unsigned int data1_reg_val;
Why is this static value used instead of reading ds->regs->dat1 ? Depending on the order of the function calling, this value may not mirror what is in the register.
I'll remove the static variable.
+void spi_init() +{
- /* do nothing */
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int mode)
+{
- struct davinci_spi_slave *ds;
- ds = malloc(sizeof(struct davinci_spi_slave));
- if (!ds)
return NULL;
- ds->slave.bus = bus;
- ds->slave.cs = cs;
- ds->regs = (struct davinci_spi_regs *)CONFIG_SYS_SPI_BASE;
- ds->freq = max_hz;
- return &ds->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
- struct davinci_spi_slave *ds = to_davinci_spi(slave);
Check before you free. It would be nice if you could poison the pointer.
OK.
- free(ds);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
- struct davinci_spi_slave *ds = to_davinci_spi(slave);
- unsigned int scalar;
- /* Enable the SPI hardware */
- writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
- udelay(1000);
- writel(SPIGCR0_SPIENA_MASK, &ds->regs->gcr0);
- /* Set master mode, powered up and not activated */
- writel(SPIGCR1_MASTER_MASK | SPIGCR1_CLKMOD_MASK, &ds->regs->gcr1);
- /* CS, CLK, SIMO and SOMI are functional pins */
- writel((SPIPC0_EN0FUN_MASK) | (SPIPC0_CLKFUN_MASK) |
(SPIPC0_DOFUN_MASK) | (SPIPC0_DIFUN_MASK), &ds->regs->pc0);
- /* setup format */
- scalar = ((CONFIG_SYS_SPI_CLK / ds->freq) - 1) & 0xFF;
- writel(8 | /* character length */
(scalar << SPIFMT_PRESCALE_SHIFT) |
/* clock signal delayed by half clk cycle */
(1 << SPIFMT_PHASE_SHIFT) |
/* clock low in idle state - Mode 0 */
(0 << SPIFMT_POLARITY_SHIFT) |
/* MSB shifted out first */
(0 << SPIFMT_SHIFTDIR_SHIFT), &ds->regs->fmt0);
Shifting 0's.. This could be improved
OK.
- /* hold cs active at end of transfer until explicitly de-asserted */
- data1_reg_val = (1 << SPIDAT1_CSHOLD_SHIFT) |
(slave->cs << SPIDAT1_CSNR_SHIFT);
- writel(data1_reg_val, &ds->regs->dat1);
- /*
* Including a minor delay. No science here. Should be good even with
* no delay
*/
- writel((50 << SPI_C2TDELAY_SHIFT) |
(50 << SPI_T2CDELAY_SHIFT), &ds->regs->delay);
- /* default chip select register */
- writel(SPIDEF_CSDEF0_MASK, &ds->regs->def);
- /* no interrupts */
- writel(0, &ds->regs->int0);
- writel(0, &ds->regs->lvl);
- /* enable SPI */
- writel(readl(&ds->regs->gcr1) | (SPIGCR1_SPIENA_MASK), &ds->regs->gcr1);
- return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{
- struct davinci_spi_slave *ds = to_davinci_spi(slave);
- /* Disable the SPI hardware */
- writel(SPIGCR0_SPIRST_MASK, &ds->regs->gcr0);
+}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
const void *dout, void *din, unsigned long flags)
+{
- struct davinci_spi_slave *ds = to_davinci_spi(slave);
- unsigned int len;
- int ret, i;
- const u8 *txp = dout;
- u8 *rxp = din;
It is unclear if passing in NULL values for din and dout is normal behaviour. Please add a comment if it is. Also break transfer loop into a tx / rx parts.
NULL values for din and dout is normal and I'll document it. SPI peripheral is a transceiver kind of a peripheral on DaVinci. It requires TX for RX to work. Hence they are coupled like this.
- ret = 0;
- if (bitlen == 0)
/* Finish any previously submitted transfers */
goto out;
- /*
* It's not clear how non-8-bit-aligned transfers are supposed to be
* represented as a stream of bytes...this is a limitation of
* the current SPI interface - here we terminate on receiving such a
* transfer request.
*/
- if (bitlen % 8) {
/* Errors always terminate an ongoing transfer */
flags |= SPI_XFER_END;
It is unclear if you are forcing a flag state that may also be passed in. Please add a comment
SPI_XFER_END flag is being set if bitlen is not 8 bit aligned. This has been documented above.
goto out;
- }
- len = bitlen / 8;
- /* do an empty read to clear the current contents */
- readl(&ds->regs->buf);
- /* keep writing and reading 1 byte until done */
- for (i = 0; i < len; i++) {
/* wait till TXFULL is asserted */
while (readl(&ds->regs->buf) & (SPIBUF_TXFULL_MASK));
/* write the data */
data1_reg_val &= ~0xFFFF;
if (txp) {
Checking for a valid pointer should happen earlier. If an error happens here, a bogus value of the old data1_reg_val will be used. Move the check higher
data1_reg_val |= *txp & 0xFF;
Adding 0xff should not be necessary for a u8.
Agree, I'll remove it.
txp++;
}
<snip>
diff --git a/drivers/spi/davinci_spi.h b/drivers/spi/davinci_spi.h new file mode 100644 index 0000000..5b9a832 --- /dev/null +++ b/drivers/spi/davinci_spi.h @@ -0,0 +1,102 @@ +/*
<snip> > + * Copyright (C) 2009 Sekhar Nori, Texas Instruments, Inc <www.ti.com>
+static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave) +{
Check before calling
OK.
Thanks for your comments. I'll submit the updated patch soon.
Regards, Sudhakar
participants (3)
-
Nick Thompson
-
Sudhakar Rajashekhara
-
Tom