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

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to