Skip to contentSkip to conversation

TI DaVinci: Driver for the davinci SPI controller

PATCH Superseded Sudhakar Rajashekhara 3 participants 5 messages Jan 5, 2010 – Jan 6, 2010

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#1 lore raw

PATCH v2 1/1 TI DaVinci: Driver for the davinci SPI controller

Quoted 20 lines · Sudhakar Rajashekhara

From: Sekhar Nori <[email protected]>

This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.

---
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.

Quoted 4 lines
 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?

Quoted 3 lines
 4 files changed, 325 insertions(+), 1 deletions(-)
 create mode 100644 drivers/spi/davinci_spi.c
 create mode 100644 drivers/spi/davinci_spi.h

...

Quoted 6 lines
drivers/spi/davinci_spi.c new file +0 -0
new file mode 100644
index 0000000..c3f1810
@@ -0,0 +1,221 @@

...

drivers/spi/davinci_spi.c

Quoted 4 lines
drivers/spi/davinci_spi.c new file +4 -0
+
+	/* 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.

#2 lore raw

PATCH v2 1/1 TI DaVinci: Driver for the davinci SPI controller

Quoted 21 lines · Nick Thompson
Quoted 20 lines · Sudhakar Rajashekhara

From: Sekhar Nori <[email protected]>

This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.

---
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.

Quoted 7 lines
Quoted 4 lines
 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.

Quoted 25 lines
Quoted 3 lines
 4 files changed, 325 insertions(+), 1 deletions(-)
 create mode 100644 drivers/spi/davinci_spi.c
 create mode 100644 drivers/spi/davinci_spi.h

...

Quoted 6 lines
drivers/spi/davinci_spi.c new file +0 -0
new file mode 100644
index 0000000..c3f1810
@@ -0,0 +1,221 @@

...

drivers/spi/davinci_spi.c

Quoted 4 lines
drivers/spi/davinci_spi.c new file +4 -0
+
+	/* 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...

Quoted 6 lines
	/* 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

#3 lore raw

PATCH v2 1/1 TI DaVinci: Driver for the davinci SPI controller

In reply to Sudhakar Rajashekhara
Quoted 28 lines · Sudhakar Rajashekhara

From: Sekhar Nori <[email protected]>

This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.

---
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

<snip>

Quoted 32 lines

new file mode 100644
index 0000000..c3f1810

drivers/spi/davinci_spi.c new file +26 -0
@@ -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
+ */

drivers/spi/davinci_spi.c line 36

Quoted 10 lines
drivers/spi/davinci_spi.c new file +10 -0
+#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

drivers/spi/davinci_spi.c line 37

Quoted 1 line
drivers/spi/davinci_spi.c new file +1 -0
+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.

Quoted 17 lines
drivers/spi/davinci_spi.c new file +17 -0
+
+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;

drivers/spi/davinci_spi.c line 64

Quoted 10 lines
drivers/spi/davinci_spi.c new file +10 -0
+	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.

Quoted 21 lines
drivers/spi/davinci_spi.c new file +21 -0
+	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 */

drivers/spi/davinci_spi.c line 95

Quoted 10 lines
drivers/spi/davinci_spi.c new file +10 -0
+	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

Quoted 32 lines
drivers/spi/davinci_spi.c new file +32 -0
+
+	/* 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);

drivers/spi/davinci_spi.c line 137

Quoted 10 lines
drivers/spi/davinci_spi.c new file +10 -0
+}
+
+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.

Quoted 6 lines
drivers/spi/davinci_spi.c new file +6 -0
+
+	ret = 0;
+
+	if (bitlen == 0)
+		/* Finish any previously submitted transfers */
+		goto out;

drivers/spi/davinci_spi.c line 153

Quoted 10 lines
drivers/spi/davinci_spi.c new file +10 -0
+
+	/*
+	 * 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

Quoted 6 lines
drivers/spi/davinci_spi.c new file +6 -0
+		goto out;
+	}
+
+	len = bitlen / 8;
+
+	/* do an empty read to clear the current contents */

drivers/spi/davinci_spi.c line 169

Quoted 10 lines
drivers/spi/davinci_spi.c new file +10 -0
+	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

drivers/spi/davinci_spi.c line 170

Quoted 1 line
drivers/spi/davinci_spi.c new file +1 -0
+			data1_reg_val |= *txp & 0xFF;

Adding 0xff should not be necessary for a u8.

Quoted 3 lines
drivers/spi/davinci_spi.c new file +3 -0
+			txp++;
+		}
+

<snip>

Quoted 8 lines
drivers/spi/davinci_spi.c new file +1 -0
+
drivers/spi/davinci_spi.h new file +1 -0
new file mode 100644
index 0000000..5b9a832
@@ -0,0 +1,102 @@
+/*

<snip>

drivers/spi/davinci_spi.h

Quoted 5 lines

+ * Copyright (C) 2009 Sekhar Nori, Texas Instruments, Inc <www.ti.com>

drivers/spi/davinci_spi.h new file +3 -0
+
+static inline struct davinci_spi_slave *to_davinci_spi(struct spi_slave *slave)
+{

Check before calling

drivers/spi/davinci_spi.h

Quoted 4 lines
drivers/spi/davinci_spi.h new file +4 -0
+	return container_of(slave, struct davinci_spi_slave, slave);
+}
+
+#endif /* _DAVINCI_SPI_H_ */

Tom

#4 lore raw

PATCH v2 1/1 TI DaVinci: Driver for the davinci SPI controller

Hi,

Quoted 9 lines · Tom
Quoted 8 lines · Sudhakar Rajashekhara

From: Sekhar Nori <[email protected]>

This adds a driver for the SPI controller found on davinci based SoCs from Texas Instruments.

---

<snip>

Quoted 36 lines
Quoted 36 lines

new file mode 100644
index 0000000..c3f1810

drivers/spi/davinci_spi.c new file +30 -0
@@ -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>
+
Quoted 9 lines

drivers/spi/davinci_spi.c line 36

Quoted 6 lines
drivers/spi/davinci_spi.c new file +6 -0
+#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.

Quoted 7 lines

drivers/spi/davinci_spi.c line 37

Quoted 1 line
drivers/spi/davinci_spi.c new file +1 -0
+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.

Quoted 21 lines
Quoted 21 lines
drivers/spi/davinci_spi.c new file +21 -0
+
+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;
Quoted 9 lines

drivers/spi/davinci_spi.c line 64

Quoted 6 lines
drivers/spi/davinci_spi.c new file +6 -0
+}
+
+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.

Quoted 25 lines
Quoted 25 lines
drivers/spi/davinci_spi.c new file +25 -0
+	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)	|
Quoted 8 lines

drivers/spi/davinci_spi.c line 95

Quoted 6 lines
drivers/spi/davinci_spi.c new file +6 -0
+		/* 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.

Quoted 37 lines
Quoted 37 lines
drivers/spi/davinci_spi.c new file +37 -0
+
+	/* 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)
+{
Quoted 9 lines

drivers/spi/davinci_spi.c line 137

Quoted 5 lines
drivers/spi/davinci_spi.c new file +5 -0
+	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.

Quoted 10 lines
Quoted 10 lines
drivers/spi/davinci_spi.c new file +10 -0
+
+	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
Quoted 9 lines

drivers/spi/davinci_spi.c line 153

Quoted 6 lines
drivers/spi/davinci_spi.c new file +6 -0
+	 * 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.

Quoted 22 lines
Quoted 6 lines
drivers/spi/davinci_spi.c new file +6 -0
+		goto out;
+	}
+
+	len = bitlen / 8;
+
+	/* do an empty read to clear the current contents */

drivers/spi/davinci_spi.c line 169

Quoted 10 lines
drivers/spi/davinci_spi.c new file +10 -0
+	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

Quoted 4 lines

drivers/spi/davinci_spi.c line 170

Quoted 1 line
drivers/spi/davinci_spi.c new file +1 -0
+			data1_reg_val |= *txp & 0xFF;

Adding 0xff should not be necessary for a u8.

Agree, I'll remove it.

Quoted 14 lines
Quoted 3 lines
drivers/spi/davinci_spi.c new file +3 -0
+			txp++;
+		}
+

<snip>

Quoted 8 lines
drivers/spi/davinci_spi.c new file +1 -0
+
drivers/spi/davinci_spi.h new file +1 -0
new file mode 100644
index 0000000..5b9a832
@@ -0,0 +1,102 @@
+/*

<snip>

Quoted 7 lines

drivers/spi/davinci_spi.h

Quoted 5 lines

+ * Copyright (C) 2009 Sekhar Nori, Texas Instruments, Inc <www.ti.com>

drivers/spi/davinci_spi.h new file +3 -0
+
+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