On Thursday 01 August 2013 12:09 AM, Trent Piepho wrote:
On Tue, Jul 30, 2013 at 10:47 PM, Sourav Poddar<sourav.poddar@xxxxxx> wrote:Test details: ------------- Tested this on dra7 board. Test1: Ran mtd_stesstest for 40000 iterations. - All iterations went through without failure. Test2: Use mtd utilities: - flash_erase to erase the flash device - nanddump to read data back. - nandwrite to write to the data flash. diff between the write and read data shows zero.You've obviously never tested word lengths other than 8, because...+static inline void ti_qspi_read_data(struct ti_qspi *qspi, + unsigned long reg, int wlen, u8 **rxbuf) +{ + switch (wlen) { + case 8: + **rxbuf = readb(qspi->base + reg); + dev_vdbg(qspi->dev, "rx done, read %02x\n", *(*rxbuf)); + *rxbuf += 1; + break; + case 16: + **rxbuf = readw(qspi->base + reg);*rxbuf is a u8*. This means when you assign to **rxbuf the type of the lvalue is u8. 8 bits. It does not matter what type the rvalue is, u8, u16, or u32, the result will always be truncated to 8 bits.
May be, I can typecast the lvalue correspondingly before assigning.
IMHO, I toss the design of ti_qspi_read/write_data(). They look like a function to read a generic register, yet it only makes sense with QSPI_SPI_DATA_REG. Passing the pointer by address so it can be incremented is ugly. And doesn't work at all, since the type of the pointer isn't going to be the same for different word lengths. The case statement inside the inner loop is inefficient. Each case is largely duplicated code.
Yes, type of word length is not going to be the same. Hence, I kept it as u8, then increment the pointer accordingly according to the case to which it belongs. Due to the different kind of variants used(read(b/w/l), each case has to be replicated. -- 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
