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

[Ben Warren]

Hi Steven,

Steven A. Falco wrote:
> This patch adds a SPI driver for the PPC440EPx processor.
>
> Signed-off-by: Steven A. Falco <sfalco at 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 at 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 at 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