[U-Boot] [PATCH 1/2][RESEND] Add an SPI driver for PPC440EPx

This patch adds a SPI driver for the PPC440EPx processor.
Signed-off-by: Steven A. Falco sfalco@harris.com --- 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
diff --git a/cpu/ppc4xx/Makefile b/cpu/ppc4xx/Makefile index 96ab5c6..bdebfb6 100644 --- a/cpu/ppc4xx/Makefile +++ b/cpu/ppc4xx/Makefile @@ -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 diff --git a/cpu/ppc4xx/spi.c b/cpu/ppc4xx/spi.c new file mode 100644 index 0000000..572244b --- /dev/null +++ b/cpu/ppc4xx/spi.c @@ -0,0 +1,128 @@ +/* + * Copyright (C) 2008 Harris Corporation + * Author: Steven A. Falco sfalco@harris.com + * + * 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 <4xx_spi.h> + +#if defined(CONFIG_HARD_SPI) + +void spi_init (void ) +{ + volatile u8 *cd = (volatile u8 *) SPI_CDM; + volatile u8 *md = (volatile u8 *) SPI_MODE; + + *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 */ diff --git a/include/4xx_spi.h b/include/4xx_spi.h new file mode 100644 index 0000000..331de20 --- /dev/null +++ b/include/4xx_spi.h @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2008 Harris Corporation + * Author: Steven A. Falco sfalco@harris.com + * + * 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) +#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) + +#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) + +#endif /* _4xx_spi_h_ */

Hi Steven,
Steven A. Falco wrote:
This patch adds a SPI driver for the PPC440EPx processor.
Signed-off-by: Steven A. Falco sfalco@harris.com
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
diff --git a/cpu/ppc4xx/Makefile b/cpu/ppc4xx/Makefile index 96ab5c6..bdebfb6 100644 --- a/cpu/ppc4xx/Makefile +++ b/cpu/ppc4xx/Makefile @@ -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 diff --git a/cpu/ppc4xx/spi.c b/cpu/ppc4xx/spi.c
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.
new file mode 100644 index 0000000..572244b --- /dev/null +++ b/cpu/ppc4xx/spi.c @@ -0,0 +1,128 @@ +/*
- Copyright (C) 2008 Harris Corporation
- Author: Steven A. Falco sfalco@harris.com
- 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 <4xx_spi.h>
+#if defined(CONFIG_HARD_SPI)
This shouldn't be necessary if you use the driver-specific CONFIG as I mentioned above.
+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.
- *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 */ diff --git a/include/4xx_spi.h b/include/4xx_spi.h
Probably put this file in include/asm-ppc/
new file mode 100644 index 0000000..331de20 --- /dev/null +++ b/include/4xx_spi.h @@ -0,0 +1,50 @@ +/*
- Copyright (C) 2008 Harris Corporation
- Author: Steven A. Falco sfalco@harris.com
- 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) +#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
+#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.
+#endif /* _4xx_spi_h_ */
regards, Ben

Hi Steve,
On Monday 08 December 2008, Steven A. Falco wrote:
This patch adds a SPI driver for the PPC440EPx processor.
Signed-off-by: Steven A. Falco sfalco@harris.com
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.
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
diff --git a/cpu/ppc4xx/Makefile b/cpu/ppc4xx/Makefile index 96ab5c6..bdebfb6 100644 --- a/cpu/ppc4xx/Makefile +++ b/cpu/ppc4xx/Makefile @@ -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.
COBJS += tlb.o COBJS += traps.o COBJS += usb.o diff --git a/cpu/ppc4xx/spi.c b/cpu/ppc4xx/spi.c new file mode 100644 index 0000000..572244b --- /dev/null +++ b/cpu/ppc4xx/spi.c @@ -0,0 +1,128 @@ +/*
- Copyright (C) 2008 Harris Corporation
- Author: Steven A. Falco sfalco@harris.com
- 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 <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.
+{
- 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.
- *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)
Nitpick: Space after "if". Again checkpatch will help here.
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 */ diff --git a/include/4xx_spi.h b/include/4xx_spi.h new file mode 100644 index 0000000..331de20 --- /dev/null +++ b/include/4xx_spi.h
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@denx.de =====================================================================

Dear Steven,
In message 493D5B65.6020305@harris.com you wrote:
This patch adds a SPI driver for the PPC440EPx processor.
Signed-off-by: Steven A. Falco sfalco@harris.com
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
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
participants (4)
-
Ben Warren
-
Stefan Roese
-
Steven A. Falco
-
Wolfgang Denk