From patchwork Tue Jul 19 12:21:22 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas De Schampheleire X-Patchwork-Id: 988802 Received: from lists.sourceforge.net (lists.sourceforge.net [216.34.181.88]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p6JCMDke022398 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 19 Jul 2011 12:22:34 GMT Received: from localhost ([127.0.0.1] helo=sfs-ml-3.v29.ch3.sourceforge.com) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1Qj9JS-0008Qf-Do; Tue, 19 Jul 2011 12:22:10 +0000 Received: from sog-mx-4.v43.ch3.sourceforge.com ([172.29.43.194] helo=mx.sourceforge.net) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1Qj9JR-0008QX-MM for spi-devel-general@lists.sourceforge.net; Tue, 19 Jul 2011 12:22:09 +0000 Received-SPF: pass (sog-mx-4.v43.ch3.sourceforge.com: domain of gmail.com designates 74.125.82.53 as permitted sender) client-ip=74.125.82.53; envelope-from=patrickdepinguin+spidevel@gmail.com; helo=mail-ww0-f53.google.com; Received: from mail-ww0-f53.google.com ([74.125.82.53]) by sog-mx-4.v43.ch3.sourceforge.com with esmtps (TLSv1:RC4-SHA:128) (Exim 4.76) id 1Qj9JQ-0008Ah-Q1 for spi-devel-general@lists.sourceforge.net; Tue, 19 Jul 2011 12:22:09 +0000 Received: by wwf26 with SMTP id 26so3592463wwf.10 for ; Tue, 19 Jul 2011 05:22:02 -0700 (PDT) Received: by 10.227.207.15 with SMTP id fw15mr6747543wbb.66.1311078122485; Tue, 19 Jul 2011 05:22:02 -0700 (PDT) Received: from localhost6.localdomain6 (94-227-97-70.access.telenet.be [94.227.97.70]) by mx.google.com with ESMTPS id fo2sm4272227wbb.31.2011.07.19.05.22.00 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 19 Jul 2011 05:22:01 -0700 (PDT) MIME-Version: 1.0 Subject: [PATCH] FSL eSPI driver: fix byte-vs-character and receive buffer problems X-Mercurial-Node: 60740d4b3d09a322a9758fb354f85316bed3fa40 Message-Id: <60740d4b3d09a322a975.1311078082@localhost6.localdomain6> User-Agent: Mercurial-patchbomb/1.6.3 Date: Tue, 19 Jul 2011 14:21:22 +0200 From: Thomas De Schampheleire To: Mingkai Hu , kumar.gala@freescale.com, spi-devel-general@lists.sourceforge.net X-Spam-Score: -1.6 (-) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (patrickdepinguin+spidevel[at]gmail.com) -0.0 SPF_PASS SPF: sender matches SPF record -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature X-Headers-End: 1Qj9JQ-0008Ah-Q1 Cc: derek.kwindt@alcatel-lucent.com, ronny.meeus@gmail.com X-BeenThere: spi-devel-general@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list List-Id: Linux SPI core/device drivers discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: spi-devel-general-bounces@lists.sourceforge.net X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 19 Jul 2011 12:22:35 +0000 (UTC) The Freescale eSPI driver has several problems related to transaction counting and receive buffer offset. - The len field in struct spi_transfer is counted in bytes, while the TRANLEN field in the eSPI controller is counted in characters. The relation between both is given by the bits_per_word variable. The original driver always used t->len for TRANLEN, which is not correct when bits_per_word is larger than 8. - The interrupt handler allowed the remaining bytes (mspi->len) to become negative, due to an unconditional -4 even when the number of bytes received was less. Additionally, the interrupt handler incorrectly assumed that with each interrupt only one character was transmitted. This was not the case since fsl_espi_cpu_bufs() always transmits in blocks of 32 bit, which is always more than one character (the exact number depending on bits_per_word). - In fsl_espi_do_one_msg(), the driver sets the length of the transfer being prepared as (n_tx + n_rx), which is incorrect since these are full-duplex transactions. Moreover, the bytes received from the controller where written at the wrong offset in the driver's receive buffer. Specifically, they were written n_tx too far. Signed-off-by: Thomas De Schampheleire --- Note that this patch is against an earlier version of the eSPI driver than the one which is currently in the Linux tree. The aspects of the driver that I changed are still relevant for the new version though. I would like to get some feedback from the original author (Mingkai Hu) or others, to verify my changes in other conditions. I have tested 8 and 16 bit transfers on a temperature sensor and an EEPROM device, but do not have devices accepting other bits_per_word values. spi_fsl_espi.c | 37 +++++++++++++++++++++++++------------ spi_fsl_lib.h | 1 + 2 files changed, 26 insertions(+), 12 deletions(-) ------------------------------------------------------------------------------ Magic Quadrant for Content-Aware Data Loss Prevention Research study explores the data loss prevention market. Includes in-depth analysis on the changes within the DLP market, and the criteria used to evaluate the strengths and weaknesses of these DLP solutions. http://www.accelacomm.com/jaw/sfnl/114/51385063/ diff --git a/drivers/spi/spi_fsl_espi.c b/drivers/spi/spi_fsl_espi.c --- a/drivers/spi/spi_fsl_espi.c +++ b/drivers/spi/spi_fsl_espi.c @@ -212,7 +212,7 @@ bool is_dma_mapped) { struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(spi->master); - unsigned int len = t->len; + unsigned int count; u8 bits_per_word; int ret; @@ -221,23 +221,31 @@ bits_per_word = t->bits_per_word; mpc8xxx_spi->len = t->len; - len = roundup(len, 4) / 4; - mpc8xxx_spi->tx = t->tx_buf; mpc8xxx_spi->rx = t->rx_buf; + mpc8xxx_spi->bits_per_word = bits_per_word; INIT_COMPLETION(mpc8xxx_spi->done); + /* Convert between t->len (in bytes) and count (in characters) */ + if (bits_per_word <= 8) { + count = t->len; + } else if (bits_per_word <= 16) { + count = t->len >> 1; + } else { + return -EINVAL; + } + /* Set SPCOM[CS] and SPCOM[TRANLEN] field */ - if ((t->len - 1) > SPCOM_TRANLEN_MAX) { + if ((count - 1) > SPCOM_TRANLEN_MAX) { dev_err(mpc8xxx_spi->dev, "Transaction length (%d)" - " beyond the SPCOM[TRANLEN] field\n", t->len); + " beyond the SPCOM[TRANLEN] field\n", count); return -EINVAL; } mpc8xxx_spi_write_reg(&mpc8xxx_spi->espi_base->command, - (SPCOM_CS(spi->chip_select) | SPCOM_TRANLEN(t->len - 1))); + (SPCOM_CS(spi->chip_select) | SPCOM_TRANLEN(count - 1))); - ret = fsl_espi_cpu_bufs(mpc8xxx_spi, t, len); + ret = fsl_espi_cpu_bufs(mpc8xxx_spi, t, count); if (ret) return ret; @@ -301,7 +309,7 @@ } } - trans.len = n_tx + n_rx; + trans.len = n_tx; trans.tx_buf = local_buf; trans.rx_buf = local_buf + n_tx; spi_message_add_tail(&trans, &message); @@ -324,7 +332,7 @@ m->actual_length += t->len; if (rx_buf) - memcpy(rx_buf, t->rx_buf + n_tx, n_rx); + memcpy(rx_buf, t->rx_buf, n_rx); if (t->delay_usecs) udelay(t->delay_usecs); @@ -402,6 +410,7 @@ if (mspi->len >= 4) { rx_data = mpc8xxx_spi_read_reg( &mspi->espi_base->receive); + mspi->len -= 4; } else { tmp = mspi->len; rx_data = 0; @@ -412,10 +421,9 @@ } rx_data <<= (4 - mspi->len) * 8; + mspi->len = 0; } - mspi->len -= 4; - if (mspi->rx) mspi->get_rx(rx_data, mspi); } @@ -430,7 +438,12 @@ /* Clear the events */ mpc8xxx_spi_write_reg(&mspi->espi_base->event, events); - mspi->count -= 1; + if (mspi->bits_per_word <= 8) { + mspi->count = mspi->len; + } else if (mspi->bits_per_word <= 16) { + mspi->count = mspi->len >> 1; + } + if (mspi->count) { u32 word = mspi->get_tx(mspi); diff --git a/drivers/spi/spi_fsl_lib.h b/drivers/spi/spi_fsl_lib.h --- a/drivers/spi/spi_fsl_lib.h +++ b/drivers/spi/spi_fsl_lib.h @@ -56,6 +56,7 @@ void (*spi_remove) (struct mpc8xxx_spi *mspi); unsigned int count; + u8 bits_per_word; unsigned int irq; unsigned nsecs; /* (clock cycle time)/2 */