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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
