Skip to contentSkip to conversation

Add an SPI driver for PPC440EPx

PATCH RESEND New Steven A. Falco 4 participants 4 messages Dec 8, 2008

lore
Share & tools
lore

Steven A. Falco

author
#0 lore raw

PATCH RESEND 1/2 Add an SPI driver for PPC440EPx

This patch adds a SPI driver for the PPC440EPx processor.

Signed-off-by Steven A. Falco <[email protected]>

---
Sorry - forgot the subject line, the first time.

 cpu/ppc4xx/Makefile |    1 +
 cpu/ppc4xx/spi.c    |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/4xx_spi.h   |   50 ++++++++++++++++++++
 3 files changed, 179 insertions(+), 0 deletions(-)
 create mode 100644 cpu/ppc4xx/spi.c
 create mode 100644 include/4xx_spi.h
cpu/ppc4xx/Makefile +1 -0
index 96ab5c6..bdebfb6 100644
@@ -54,6 +54,7 @@ COBJS	+= iop480_uart.o
 COBJS	+= ndfc.o
 COBJS	+= sdram.o
 COBJS	+= speed.o
+COBJS	+= spi.o
 COBJS	+= tlb.o
 COBJS	+= traps.o
 COBJS	+= usb.o
cpu/ppc4xx/spi.c new file +128 -0

128 lines not on this page · raw message

include/4xx_spi.h new file +50 -0

50 lines not on this page · raw message

Signature · 1 line
1.6.0.2
#1 lore raw

PATCH RESEND 1/2 Add an SPI driver for PPC440EPx

Hi Steven,

cpu/ppc4xx/spi.c

Quoted 27 lines · Steven A. Falco

This patch adds a SPI driver for the PPC440EPx processor.

Signed-off-by Steven A. Falco <[email protected]>

---
Sorry - forgot the subject line, the first time.

 cpu/ppc4xx/Makefile |    1 +
 cpu/ppc4xx/spi.c    |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/4xx_spi.h   |   50 ++++++++++++++++++++
 3 files changed, 179 insertions(+), 0 deletions(-)
 create mode 100644 cpu/ppc4xx/spi.c
 create mode 100644 include/4xx_spi.h
cpu/ppc4xx/Makefile +1 -0
index 96ab5c6..bdebfb6 100644
@@ -54,6 +54,7 @@ COBJS	+= iop480_uart.o
 COBJS	+= ndfc.o
 COBJS	+= sdram.o
 COBJS	+= speed.o
+COBJS	+= spi.o
 COBJS	+= tlb.o
 COBJS	+= traps.o
 COBJS	+= usb.o
cpu/ppc4xx/spi.c +0 -0

Please put this in drivers/spi, and create a CONFIG for it that is appropriate. For example, CONFIG_PPC4xx_SPI if this controller is found on 405, 440 and 460 SOCs, CONFIG_PPC44x_SPI if only on 440 and CONFIG_PPC440EPX_SPI if only on 440EPX. Then follow the format found in drivers/spi/Makefile for conditionally compiling it in.

Quoted 28 lines

new file mode 100644
index 0000000..572244b

cpu/ppc4xx/spi.c new file +22 -0
@@ -0,0 +1,128 @@
+/*
+ * Copyright (C) 2008 Harris Corporation
+ * Author: Steven A. Falco <[email protected]>
+ *
+ * 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
+ */

cpu/ppc4xx/spi.c line 32

Quoted 10 lines
cpu/ppc4xx/spi.c new file +10 -0
+
+#include <common.h>
+#include <spi.h>
+#include <malloc.h>
+
+#include <asm/io.h>
+
+#include <4xx_spi.h>
+
+#if defined(CONFIG_HARD_SPI)

This shouldn't be necessary if you use the driver-specific CONFIG as I mentioned above.

cpu/ppc4xx/spi.c line 38

Quoted 7 lines
cpu/ppc4xx/spi.c new file +6 -0
+
+void spi_init (void )
+{
+	volatile u8 *cd = (volatile u8 *) SPI_CDM;
+	volatile u8 *md = (volatile u8 *) SPI_MODE;
+

Please use the built-in accessors found in include/asm-ppc/io.h instead (in_8 etc.). This will force some pretty big changes to your patch, so I won't mention them all.

include/4xx_spi.h

Quoted 92 lines
cpu/ppc4xx/spi.c new file +90 -0
+	*cd = 0;		/* Default to "go fast" */
+	*md = SPI_MODE_SPE;	/* Enable port */
+}
+
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+			unsigned int max_hz, unsigned int mode)
+{
+	volatile u8 *md = (volatile u8 *) SPI_MODE;
+	volatile u8 *cd = (volatile u8 *) SPI_CDM;
+	ulong opb = get_OPB_freq();
+	ulong divisor;
+	struct spi_slave *s;
+
+	if(!spi_cs_is_valid(bus, cs))
+		return NULL;
+
+	divisor = ((opb + (max_hz * 4) - 1) / (max_hz * 4)) - 1;
+	if(divisor > 255)
+		return NULL;
+
+	*cd = divisor;
+
+	if (!(s = malloc(sizeof(struct spi_slave))))
+		return NULL;
+
+	if (mode & SPI_CPHA)
+		*md &= ~SPI_MODE_SCP;
+	else
+		*md |= SPI_MODE_SCP;
+
+	if (mode & SPI_CPOL)
+		*md |= SPI_MODE_CI;
+	else
+		*md &= ~SPI_MODE_CI;
+
+	s->bus = bus;
+	s->cs = cs;
+
+	return s;
+}
+
+void spi_free_slave(struct spi_slave *slave)
+{
+	free(slave);
+}
+
+int spi_claim_bus(struct spi_slave *slave)
+{
+	return 0;
+}
+
+void spi_release_bus(struct spi_slave *slave)
+{
+}
+
+#define GO	*cr = SPI_CR_STR
+#define TXWAIT	while(*sr & SPI_SR_BSY)
+#define RXWAIT	while(!(*sr & SPI_SR_RBR))
+
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
+		const void *dout, void *din, unsigned long flags)
+{
+	volatile u8 *cr = (volatile u8 *) SPI_CR;
+	volatile u8 *sr = (volatile u8 *) SPI_SR;
+	volatile u8 *tx = (volatile u8 *) SPI_TXD;
+	volatile u8 *rx = (volatile u8 *) SPI_RXD;
+
+	const u8 *txd = dout;
+	u8 *rxd = din;
+	int ii;
+
+	if (flags & SPI_XFER_BEGIN)
+		spi_cs_activate(slave);
+
+	/* Do a byte at a time */
+	for (ii = 0; ii < ((bitlen + 7) / 8); ii++) {
+		TXWAIT;
+		*tx = *txd++;
+		GO;
+		RXWAIT;
+		*rxd++ = *rx;
+	}
+
+	if (flags & SPI_XFER_END)
+		spi_cs_deactivate(slave);
+
+	return 0;
+}
+
+#endif /* CONFIG_HARD_SPI */
include/4xx_spi.h +0 -0

Probably put this file in include/asm-ppc/

Quoted 34 lines

new file mode 100644
index 0000000..331de20

include/4xx_spi.h new file +28 -0
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2008 Harris Corporation
+ * Author: Steven A. Falco <[email protected]>
+ *
+ * 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
+ */
+
+#ifndef _4xx_spi_h_
+#define _4xx_spi_h_
+
+#if defined(CONFIG_440EPX)
+#define SPI_BASE_ADDR	(CONFIG_SYS_PERIPHERAL_BASE + 0x00000900)

include/4xx_spi.h line 38

Quoted 10 lines
include/4xx_spi.h new file +10 -0
+#endif
+
+#define SPI_REGISTERS_BASE_ADDRESS SPI_BASE_ADDR
+#define SPI_MODE	(SPI_REGISTERS_BASE_ADDRESS + 0x00)
+#define SPI_RXD		(SPI_REGISTERS_BASE_ADDRESS + 0x01)
+#define SPI_TXD		(SPI_REGISTERS_BASE_ADDRESS + 0x02)
+#define SPI_CR		(SPI_REGISTERS_BASE_ADDRESS + 0x03)
+#define SPI_SR		(SPI_REGISTERS_BASE_ADDRESS + 0x04)
+#define SPI_CDM		(SPI_REGISTERS_BASE_ADDRESS + 0x06)
+

If the controller is common to different chips and the register layout is similar, it's much cleaner to define a struct somewhere and reference a pointer to it rather than using macros like this. For example, please see include/asm-ppc/mpc8xxx_spi.h

include/4xx_spi.h line 49

Quoted 12 lines
include/4xx_spi.h new file +11 -0
+#define SPI_CR_STR	(0x01)
+
+#define SPI_SR_RBR	(0x01)
+#define SPI_SR_BSY	(0x02)
+
+#define SPI_MODE_IL	(0x01)
+#define SPI_MODE_CI	(0x02)
+#define SPI_MODE_RD	(0x04)
+#define SPI_MODE_SPE	(0x08)
+#define SPI_MODE_SCP	(0x10)
+

Some will argue that the brackets aren't needed around constants.

include/4xx_spi.h line 50

Quoted 2 lines
include/4xx_spi.h new file +1 -0
+#endif /* _4xx_spi_h_ */

regards,
Ben

#2 lore raw

PATCH RESEND 1/2 Add an SPI driver for PPC440EPx

In reply to Steven A. Falco

Hi Steve,

Quoted 5 lines · Steven A. Falco

This patch adds a SPI driver for the PPC440EPx processor.

Signed-off-by Steven A. Falco <[email protected]>

---
Sorry - forgot the subject line, the first time.

Please start the subject with "ppc4xx: " for 4xx related patches (e.g.: "ppc4xx: Add PPC4xx SPI driver"). And while looking at this subject, there shouldn't be much 440EPx specific stuff in this patch. So please name this patch PPC4xx instead of PPC440EPx, perhaps with a comment that it's "only" tested on 440EPx.

And it's always a good idea to CC the subsystem maintainer (custodian), in this case that's me. ;)

Some further comments below. And I second all of Ben's comments.

Quoted 13 lines
 cpu/ppc4xx/Makefile |    1 +
 cpu/ppc4xx/spi.c    |  128
+++++++++++++++++++++++++++++++++++++++++++++++++++ include/4xx_spi.h   |  
50 ++++++++++++++++++++
 3 files changed, 179 insertions(+), 0 deletions(-)
 create mode 100644 cpu/ppc4xx/spi.c
 create mode 100644 include/4xx_spi.h
cpu/ppc4xx/Makefile +0 -0
index 96ab5c6..bdebfb6 100644

cpu/ppc4xx/Makefile COBJS += iop480_uart.o · line 57

Quoted 4 lines
cpu/ppc4xx/Makefile +1 -0
@@ -54,6 +54,7 @@ COBJS	+= iop480_uart.o
 COBJS	+= ndfc.o
 COBJS	+= sdram.o
 COBJS	+= speed.o
+COBJS	+= spi.o

Yes, please move this to drivers/spi.

Quoted 33 lines
cpu/ppc4xx/Makefile +0 -0
 COBJS	+= tlb.o
 COBJS	+= traps.o
 COBJS	+= usb.o
cpu/ppc4xx/spi.c new file +24 -0
new file mode 100644
index 0000000..572244b
@@ -0,0 +1,128 @@
+/*
+ * Copyright (C) 2008 Harris Corporation
+ * Author: Steven A. Falco <[email protected]>
+ *
+ * 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>

cpu/ppc4xx/spi.c line 34

Quoted 10 lines
cpu/ppc4xx/spi.c new file +10 -0
+#include <spi.h>
+#include <malloc.h>
+
+#include <asm/io.h>
+
+#include <4xx_spi.h>
+
+#if defined(CONFIG_HARD_SPI)
+
+void spi_init (void )

Nitpick: No space after "void". You should probably run checkpatch over this patch to check for those kind of issues.

cpu/ppc4xx/spi.c line 37

Quoted 3 lines
cpu/ppc4xx/spi.c new file +3 -0
+{
+	volatile u8 *cd = (volatile u8 *) SPI_CDM;
+	volatile u8 *md = (volatile u8 *) SPI_MODE;

As Ben mentioned, don't use volatile pointer access. Instead use the in_8()... accessor functions.

Quoted 9 lines
cpu/ppc4xx/spi.c new file +9 -0
+
+	*cd = 0;		/* Default to "go fast" */
+	*md = SPI_MODE_SPE;	/* Enable port */
+}
+
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+			unsigned int max_hz, unsigned int mode)
+{
+	volatile u8 *md = (volatile u8 *) SPI_MODE;

cpu/ppc4xx/spi.c line 56

Quoted 10 lines
cpu/ppc4xx/spi.c new file +10 -0
+	volatile u8 *cd = (volatile u8 *) SPI_CDM;
+	ulong opb = get_OPB_freq();
+	ulong divisor;
+	struct spi_slave *s;
+
+	if(!spi_cs_is_valid(bus, cs))
+		return NULL;
+
+	divisor = ((opb + (max_hz * 4) - 1) / (max_hz * 4)) - 1;
+	if(divisor > 255)

Nitpick: Space after "if". Again checkpatch will help here.

include/4xx_spi.h

Quoted 77 lines
cpu/ppc4xx/spi.c new file +72 -0
+		return NULL;
+
+	*cd = divisor;
+
+	if (!(s = malloc(sizeof(struct spi_slave))))
+		return NULL;
+
+	if (mode & SPI_CPHA)
+		*md &= ~SPI_MODE_SCP;
+	else
+		*md |= SPI_MODE_SCP;
+
+	if (mode & SPI_CPOL)
+		*md |= SPI_MODE_CI;
+	else
+		*md &= ~SPI_MODE_CI;
+
+	s->bus = bus;
+	s->cs = cs;
+
+	return s;
+}
+
+void spi_free_slave(struct spi_slave *slave)
+{
+	free(slave);
+}
+
+int spi_claim_bus(struct spi_slave *slave)
+{
+	return 0;
+}
+
+void spi_release_bus(struct spi_slave *slave)
+{
+}
+
+#define GO	*cr = SPI_CR_STR
+#define TXWAIT	while(*sr & SPI_SR_BSY)
+#define RXWAIT	while(!(*sr & SPI_SR_RBR))
+
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
+		const void *dout, void *din, unsigned long flags)
+{
+	volatile u8 *cr = (volatile u8 *) SPI_CR;
+	volatile u8 *sr = (volatile u8 *) SPI_SR;
+	volatile u8 *tx = (volatile u8 *) SPI_TXD;
+	volatile u8 *rx = (volatile u8 *) SPI_RXD;
+
+	const u8 *txd = dout;
+	u8 *rxd = din;
+	int ii;
+
+	if (flags & SPI_XFER_BEGIN)
+		spi_cs_activate(slave);
+
+	/* Do a byte at a time */
+	for (ii = 0; ii < ((bitlen + 7) / 8); ii++) {
+		TXWAIT;
+		*tx = *txd++;
+		GO;
+		RXWAIT;
+		*rxd++ = *rx;
+	}
+
+	if (flags & SPI_XFER_END)
+		spi_cs_deactivate(slave);
+
+	return 0;
+}
+
+#endif /* CONFIG_HARD_SPI */
include/4xx_spi.h new file +0 -0
new file mode 100644
index 0000000..331de20

I would prefer: include/asm-ppc/ppc4xx-spi.h instead.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

#3 lore raw

PATCH RESEND 1/2 Add an SPI driver for PPC440EPx

In reply to Steven A. Falco

Dear Steven,

Quoted 6 lines · In message <[email protected]> you wrote:

This patch adds a SPI driver for the PPC440EPx processor.

Signed-off-by Steven A. Falco <[email protected]>

---
Sorry - forgot the subject line, the first time.

Quoted 6 lines
 cpu/ppc4xx/Makefile |    1 +
 cpu/ppc4xx/spi.c    |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/4xx_spi.h   |   50 ++++++++++++++++++++
 3 files changed, 179 insertions(+), 0 deletions(-)
 create mode 100644 cpu/ppc4xx/spi.c
 create mode 100644 include/4xx_spi.h

Seems you are adding a bare driver here, without any indication of (potential) users of this driver.

Policy is not to add code that is not really used anywhere.

Do you intend to submit other patches which will require this driver, or is this driver there "just for the fun of it" ?

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You are an excellent tactician, Captain. You let your second in  com-
mand attack while you sit and watch for weakness.
	-- Khan Noonian Singh, "Space Seed", stardate 3141.9