25 Apr
2011
25 Apr
'11
9:03 a.m.
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