[U-Boot] [PATCH v2] andes_spi: add andes_spi interface

[Mike Frysinger]

On Mon, Apr 25, 2011 at 02:50, Macpaul Lin wrote:
> +void spi_init()

please use "(void)"

> +void andes_spi_spit_en(struct andes_spi_slave *ds)

mark static

> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
> +                       unsigned int max_hz, unsigned int mode)
> +{
> ...
> +       ds->freq = max_hz;

please add a comment that the hardware doesnt allow changing of
frequency and so the user requested speed is always ignored

> +static int andes_spi_read(struct spi_slave *slave, unsigned int len,
> +                           u8 *rxp, unsigned long flags)
> +{
> +       unsigned int i = 0, left = 0;
> +       unsigned int data = 0;

no point in setting any of these vars to 0

> +static int andes_spi_write(struct spi_slave *slave, unsigned int wlen,
> +                       unsigned int rlen, const u8 *txp, unsigned long flags)
> +{
> +       unsigned int data = 0;
> ...
> +       while (wlen > 0) {
> ...
> +               for (i = 0; i < left; i++) {
> +                       debug("%x ", *txp);
> +                       data |= *txp++ << (i * 8);
> +                       wlen--;
> +               }
> ...
> +               data = 0;
> ...
> +       }

this is a funky way of initializing "data".  it'd make more sense to
have "data = 0;" right above the for loop. and drop the other ones.
-mike