Hi Felipe, On Thursday 18 July 2013 03:54 PM, Felipe Balbi wrote:
Hi, it might be just me, but ... On Thu, Jul 18, 2013 at 03:31:26PM +0530, Sourav Poddar wrote:+static inline unsigned long ti_qspi_readl_data(struct ti_qspi *qspi, + unsigned long reg, int wlen) +{ + switch (wlen) { + case 8: + return readw(qspi->base + reg); + break; + case 16: + return readb(qspi->base + reg); + break; + case 32: + return readl(qspi->base + reg); + break; + default: + return -EINVAL; + } +} + +static inline void ti_qspi_writel_data(struct ti_qspi *qspi, + unsigned long val, unsigned long reg, int wlen) +{ + switch (wlen) { + case 8: + writew(val, qspi->base + reg); + break; + case 16: + writeb(val, qspi->base + reg); + break; + case 32: + writeb(val, qspi->base + reg); + break; + default: + dev_dbg(qspi->dev, "word lenght out of range"); + break; + } +}because of these two functions you have the hability to read/write *more* than one byte, and yet ...+static void qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t) +{ + const u8 *txbuf; + int wlen, count; + + count = t->len; + txbuf = t->tx_buf; + wlen = t->bits_per_word; + + while (count--) { + dev_dbg(qspi->dev, "tx cmd %08x dc %08x data %02x\n", + qspi->cmd | QSPI_WR_SNGL, qspi->dc, *txbuf); + ti_qspi_writel_data(qspi, *txbuf++, QSPI_SPI_DATA_REG, wlen);you always increment by each byte. Here, if you used writel(), you wrote 4 bytes and should increment txbuf by 4.
hmm..got this point. Yes, my mistake, here I agree if wlen is not 8 bits txbuf++ is not valid.
Same goes for read_data(), below. Another thing. Even though your wlen might be 8 bits, if you write 4 bytes to write, you can save a few CPU cycles by using writel().
Do you mean 4 words of 8 bits?
You only use writew() if you have exactly 2 bytes to write and writeb() if you have exactly 1 byte to write. 3 bytes we'll be left as an exercise.
hmm..yes.
+static int ti_qspi_start_transfer_one(struct spi_master *master, + struct spi_message *m) +{ + struct ti_qspi *qspi = spi_master_get_devdata(master); + struct spi_device *spi = m->spi; + struct spi_transfer *t; + int status = 0, ret; + int frame_length; + + /* setup device control reg */ + qspi->dc = 0; + + if (spi->mode& SPI_CPHA) + qspi->dc |= QSPI_CKPHA(spi->chip_select); + if (spi->mode& SPI_CPOL) + qspi->dc |= QSPI_CKPOL(spi->chip_select); + if (spi->mode& SPI_CS_HIGH) + qspi->dc |= QSPI_CSPOL(spi->chip_select); + + frame_length = DIV_ROUND_UP(m->frame_length * spi->bits_per_word, + spi->bits_per_word);this calculation doesn't look correct. (m->frame_length * spi->bits_per_word) / spi->bits_per_word = m->frame_length What are you trying to achieve here ? frame_length should be counted in words right ? And we get that value in bytes. So what's the best calculation to convert bytes into words ? If you have 8 bits_per_word you don't need any calculation, but if you have 32 bits_per_word, you _do_ need something.
Yes, just derive this formulae with 8 bits per word in mind. Will change. It should be (m->frame_length * 8) / spi->bits_per_word
How will you achieve the number you want ? (hint: 1 byte == 8 bits) And btw, all of these mistakes pretty much tell me that this driver hasn't been tested. How have you tested this driver ?
After bootup, I checked for deive detting enumerated as /proc/mtd. After which I am using mtdutils(erase, dump and write utilied to check for the communication with the flash device.)
Is your spansion memory accessed with 8 bits_per_word only ?
Yes, most of the places is like that and data is sapmled in 8 bits. For some opcodes, we need to send 3 bytes addresses after instruction to the flash chip.
Is there anyway to use 32 bits_per_word with that device ? That would uncover quite a few mistakes in $subject.
-- 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
