On Fri, Jul 26, 2013 at 10:15 AM, Sourav Poddar <sourav.poddar@xxxxxx> wrote: > + > +static inline void ti_qspi_read_data(struct ti_qspi *qspi, > + unsigned long reg, int wlen, u8 **rxbuf) > +{ > + switch (wlen) { > + case 8: > + **rxbuf = readw(qspi->base + reg); > + dev_dbg(qspi->dev, "rx done, read %02x\n", *(*rxbuf-1)); > + *rxbuf += 1; You still have readw() paired with 8 bit length, readb() with 16, and so on. I do not think your 16 and 32 bit cases will even work. Since **rxbuf will always be an pointer to an 8-bit value, it will always just use the lower 8 bits of the register. You also always increment the address by 1. And the printout, which is the same in every case so maybe you shouldn't duplicate it, isn't correct. It will print out the previous byte in the buffer. It will possibly crash on the first byte, since you will be reading before the beginning of the buffer. You probably only tested this with 8 bit word length. Since that has readw(), it probably does not matter what size is used for the read. > +static int ti_qspi_setup(struct spi_device *spi) > +{ > + > + clk_ctrl_reg = ti_qspi_read(qspi, QSPI_SPI_CLOCK_CNTRL_REG); > + > + clk_ctrl_reg &= ~QSPI_CLK_EN; > + > + /* disable SCLK */ > + ti_qspi_write(qspi, clk_ctrl_reg, QSPI_SPI_CLOCK_CNTRL_REG); This looks like to are still not taking into account that setup() can be called while a transfer for another slave is in progress. > + > + /* setup command reg */ > + qspi->cmd = 0; > + qspi->cmd |= QSPI_EN_CS(spi->chip_select); > + qspi->cmd |= QSPI_FLEN(frame_length); > + > + list_for_each_entry(t, &m->transfers, transfer_list) { > + qspi->cmd |= QSPI_WLEN(t->bits_per_word); > + qspi->cmd |= QSPI_WC_CMD_INT_EN; Why is the QSPI_WC_CMD_INT_EN bit set inside the loop but the QSPI_EN_CS() and QSPI_FLEN() bits are set outside the loop? > + > + if (!of_property_read_u32(np, "ti,num-cs", &num_cs)) > + master->num_chipselect = num_cs; You are still not using standard "num-cs" property? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html