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

Ben Warren
2008-12-08T18:33:42Z

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