[U-Boot] [PATCH v2] spi: add altera spi controller support

Mike Frysinger
2010-03-23T21:29:06Z

On Monday 22 March 2010 23:36:19 Thomas Chou wrote:
> +#include <common.h>
> +#include <malloc.h>
> +#include <spi.h>
> +#include <asm/io.h>
> +#include <nios2-io.h>

side note, but am i the only one who thinks nios headers in include/ is bad
mojo ?

> +static nios_spi_t *nios_spi = (nios_spi_t *)CONFIG_SYS_SPI_BASE;

shouldnt this be based on the bus # so that you can support multiple ones at
runtime ? i.e. you add a regs member to the spi slave struct and operate off
that at runtime.

> +void spi_init (void)

i think the spaces should be consumed before the paren per LKML style

> +{
> + /* empty read buffer */
> + if (readl (&nios_spi->status) & NIOS_SPI_RRDY)
> + readl (&nios_spi->rxdata);
> + return;
> +}

useless return ...

> +struct spi_slave *spi_setup_slave (unsigned int bus, unsigned int cs,
> + unsigned int max_hz, unsigned int mode)
> +{
> + struct spi_slave *slave;
> +
> + slave = malloc (sizeof(struct spi_slave));

sizeof(*slave)

> + if (!slave)
> + return NULL;
> +
> + slave->bus = bus;
> + slave->cs = cs;
> +
> + return slave;
> +}

shouldnt you be validating the bus/cs values here ? presumably you only have
1 bus atm, so bus != 0 is an error. and the cs is being used to write to an
"unsigned slaveselect", so the range of values is at most 0...31 inclusive
(assuming your hardware can actually support 32 CS's).

> +int spi_claim_bus (struct spi_slave *slave)
> +{
> + return 0;
> +}
> +
> +void spi_release_bus (struct spi_slave *slave)
> +{
> + return;
> +}

i'm pretty sure you should be programming either nios_spi->slaveselect or
nios_spi->control here rather than in the spi_xfer func ...

> +int spi_xfer (struct spi_slave *slave, unsigned int bitlen, const void
> *dout, + void *din, unsigned long flags)
> +{
> + int i, iter = bitlen >> 3;
> + const uchar *txp = dout;
> + uchar *rxp = din;
> + uchar d;
> +
> + if (flags & SPI_XFER_BEGIN) {
> + writel (1 << slave->cs, &nios_spi->slaveselect);
> + writel (NIOS_SPI_SSO, &nios_spi->control);
> + }
> +
> + for (i = 0; i < iter; i++) {
> + writel (txp ? txp[i] : 0, &nios_spi->txdata);
> + while (!(readl (&nios_spi->status) & NIOS_SPI_RRDY))
> + ;
> + d = readl (&nios_spi->rxdata);
> + if (rxp)
> + rxp[i] = d;
> + }
> + if (flags & SPI_XFER_END)
> + writel (0, &nios_spi->control);
> +
> + return 0;
> +}

since you only support multiples of 8 bytes here, your code should return an
error when (bitlen & 0x7)

your XFER_END should force the CS to go low ... is that what the control does,
or is it the slaveselect ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100323/c7acb0fe/attachment.pgp