linux-kernel - Re: [PATCHv4 2/3] drivers: spi: Add qspi flash controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 19 Jul 2013 15:20:29 +0300
From:	Felipe Balbi <balbi@...com>
To:	Sourav Poddar <sourav.poddar@...com>
CC:	<balbi@...com>, <broonie@...nel.org>,
	<spi-devel-general@...ts.sourceforge.net>,
	<grant.likely@...aro.org>, <rnayak@...com>,
	<linux-omap@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv4 2/3] drivers: spi: Add qspi flash controller

Hi,

On Fri, Jul 19, 2013 at 05:18:47PM +0530, Sourav Poddar wrote:
> On Thursday 18 July 2013 04:54 PM, Felipe Balbi wrote:
> >On Thu, Jul 18, 2013 at 04:48:41PM +0530, Sourav Poddar wrote:
> >>>>+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?
> >yeah. Say you have wlen = 8 but the transfer length is 8 bytes (64
> >bits). If you use writeb(), you will do 8 writes, if you use writel()
> >you'll do 2 writes.
> >
> Just some more findings on this, after wlen bits are transferred we
> need an WC interrupt.
> So, if I try to pack 4 words of 8bits and use readl/writel, there
> will be an interrupt after every
> wlen bits transferred and things will get screwd up.

they will not if you throttle the IRQs or add some knowledge about the
FIFO sizes. I mean, there are ways to get this working.

> So, for 8 bits word we need to use readb, for 16 bits word readw.

not entirely true...

I mean, you have a 32-bit FIFO, why would underutilize the FIFO by
always writing 8-bits ? Write 32 bits, when you get the first word
completion, you know you have 8-bits of space in the FIFO, then you
can fill those 8 bits with new data. (all of this is assuming word
length is 8-bits, clearly if it is more, then you need to change the
assumptions).

Another thing which might help, is checking if the HW will even access
the other DATA registers even if word length is <= 32 bits. If it does,
then you have a total of 128 bits to shift data into, which can save you
a lot of time filling up FIFOs and waiting for them to empty.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)