From patchwork Fri Mar 29 15:19:41 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Trent Piepho X-Patchwork-Id: 2365721 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id F01BD3FD8C for ; Fri, 29 Mar 2013 15:24:57 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ULb7s-00083Q-1G; Fri, 29 Mar 2013 15:21:57 +0000 Received: from mail-pb0-f44.google.com ([209.85.160.44]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ULb66-0007XU-An for linux-arm-kernel@lists.infradead.org; Fri, 29 Mar 2013 15:20:08 +0000 Received: by mail-pb0-f44.google.com with SMTP id wz17so295648pbc.31 for ; Fri, 29 Mar 2013 08:20:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=7K6znmuceN+ttCDlIwaa6HxewtVUivleH66KGMcLjWw=; b=skEpS4sEUqRJZ6epfKgTvuWNkoBYH8bl2Y4TFqfQ+qA62zX8H79b2ARt28gg3tK8b1 R0/KicgOlyDbXpc/qWQK8Or+SkxVvqPPDGp7MHtnc3VdG1a3sTRUiYKvH4+G2TWq9ZgT 0OmOCISvcatFSJVB/Y+NDowqpq/kyYJ6wLr8kE3Y42svMEtKCwOpTR5J/ZNtpBfHz6Zg 9IHH2VivEgDxhVq9kUubZbu3jb4F9BCCj+dmU9ezyHN5dhZ6AAcPqfVKuzFmUKWDbYKO e1uN14+tweYa/4+M4c7Oq460KcrdnCNYCx/RlKtTPmhK5YDRyPHI1AaJljVFoEqc0W17 vfZg== X-Received: by 10.68.88.37 with SMTP id bd5mr4042013pbb.209.1364570404194; Fri, 29 Mar 2013 08:20:04 -0700 (PDT) Received: from localhost.localdomain (174-31-195-141.tukw.qwest.net. [174.31.195.141]) by mx.google.com with ESMTPS id tf8sm3140651pbc.42.2013.03.29.08.20.02 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 29 Mar 2013 08:20:03 -0700 (PDT) From: Trent Piepho To: linux-arm-kernel@lists.infradead.org, spi-devel-general@lists.sourceforge.net Subject: [PATCH 5/5] spi/mxs: Fix multiple slave bug and don't set clock for each xfer Date: Fri, 29 Mar 2013 08:19:41 -0700 Message-Id: <1364570381-17605-5-git-send-email-tpiepho@gmail.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1364570381-17605-1-git-send-email-tpiepho@gmail.com> References: <1364570381-17605-1-git-send-email-tpiepho@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130329_112006_504820_C58B7297 X-CRM114-Status: GOOD ( 18.65 ) X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [209.85.160.44 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (tpiepho[at]gmail.com) -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -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 Cc: Marek Vasut , Fabio Estevam , Trent Piepho , Shawn Guo X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Despite many warnings in the SPI documentation and code, the spi-mxs driver sets shared chip registers in the ->setup method. This method can be called when transfers are in progress on other slaves controlled by the master. Setting registers or any other shared state will corrupt those transfers. So fix mxs_spi_setup() to not call mxs_spi_setup_transfer(). Now that mxs_spi_setup_transfer() won't be called with a NULL transfer, since it's only purpose is to setup a transfer, the code can be simplified. mxs_spi_setup_transfer() would set the SSP SCK rate every time it was called, which is before each transfer. It is uncommon for the SCK rate to change between transfers and this causes unnecessary reprogramming of the clock registers. Changed to only set the rate when it has changed. This significantly speeds up short SPI messages, especially messages made up of many transfers. On an iMX287, using spidev with messages that consist of 511 transfers of 4 bytes each at an SCK of 48 MHz, the effective transfer rate more than doubles from about 290 KB/sec to 600 KB/sec. Signed-off-by: Trent Piepho Cc: Marek Vasut Cc: Fabio Estevam Cc: Shawn Guo --- drivers/spi/spi-mxs.c | 54 ++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/drivers/spi/spi-mxs.c b/drivers/spi/spi-mxs.c index fc52f78..b60baab 100644 --- a/drivers/spi/spi-mxs.c +++ b/drivers/spi/spi-mxs.c @@ -66,44 +66,47 @@ struct mxs_spi { struct mxs_ssp ssp; struct completion c; + unsigned int sck; /* Rate requested (vs actual) */ }; static int mxs_spi_setup_transfer(struct spi_device *dev, - struct spi_transfer *t) + const struct spi_transfer *t) { struct mxs_spi *spi = spi_master_get_devdata(dev->master); struct mxs_ssp *ssp = &spi->ssp; - uint8_t bits_per_word; - uint32_t hz = 0; - - bits_per_word = dev->bits_per_word; - if (t && t->bits_per_word) - bits_per_word = t->bits_per_word; + const unsigned int bits_per_word = t->bits_per_word ? : dev->bits_per_word; + const unsigned int hz = t->speed_hz ? min(dev->max_speed_hz, t->speed_hz) : + dev->max_speed_hz; if (bits_per_word != 8) { - dev_err(&dev->dev, "%s, unsupported bits_per_word=%d\n", - __func__, bits_per_word); + dev_err(&dev->dev, "Unsupported bits per word of %d\n", + bits_per_word); return -EINVAL; } - hz = dev->max_speed_hz; - if (t && t->speed_hz) - hz = min(hz, t->speed_hz); if (hz == 0) { - dev_err(&dev->dev, "Cannot continue with zero clock\n"); + dev_err(&dev->dev, "SPI clock rate of zero not possible\n"); return -EINVAL; } - mxs_ssp_set_clk_rate(ssp, hz); + if (hz != spi->sck) { + mxs_ssp_set_clk_rate(ssp, hz); + /* Save requested value, not actual value from ssp->clk_rate. + * Otherwise we would set the rate again each trasfer when + * actual is not quite the same as requested. */ + spi->sck = hz; + /* Perhaps we should return an error if the actual clock is + * nowhere close? */ + } writel(BM_SSP_CTRL0_LOCK_CS, ssp->base + HW_SSP_CTRL0 + STMP_OFFSET_REG_SET); + writel(BF_SSP_CTRL1_SSP_MODE(BV_SSP_CTRL1_SSP_MODE__SPI) | - BF_SSP_CTRL1_WORD_LENGTH - (BV_SSP_CTRL1_WORD_LENGTH__EIGHT_BITS) | - ((dev->mode & SPI_CPOL) ? BM_SSP_CTRL1_POLARITY : 0) | - ((dev->mode & SPI_CPHA) ? BM_SSP_CTRL1_PHASE : 0), - ssp->base + HW_SSP_CTRL1(ssp)); + BF_SSP_CTRL1_WORD_LENGTH(BV_SSP_CTRL1_WORD_LENGTH__EIGHT_BITS) | + ((dev->mode & SPI_CPOL) ? BM_SSP_CTRL1_POLARITY : 0) | + ((dev->mode & SPI_CPHA) ? BM_SSP_CTRL1_PHASE : 0), + ssp->base + HW_SSP_CTRL1(ssp)); writel(0x0, ssp->base + HW_SSP_CMD0); writel(0x0, ssp->base + HW_SSP_CMD1); @@ -113,21 +116,16 @@ static int mxs_spi_setup_transfer(struct spi_device *dev, static int mxs_spi_setup(struct spi_device *dev) { - int err = 0; - if (!dev->bits_per_word) dev->bits_per_word = 8; - if (dev->mode & ~(SPI_CPOL | SPI_CPHA)) + if (dev->bits_per_word != 8) return -EINVAL; - err = mxs_spi_setup_transfer(dev, NULL); - if (err) { - dev_err(&dev->dev, - "Failed to setup transfer, error = %d\n", err); - } + if (dev->mode & ~(SPI_CPOL | SPI_CPHA)) + return -EINVAL; - return err; + return 0; } static u32 mxs_spi_cs_to_reg(unsigned cs)