From patchwork Thu Jul 16 22:28:42 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Brownell X-Patchwork-Id: 35951 Received: from devils.ext.ti.com (devils.ext.ti.com [198.47.26.153]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n6GMUeHZ013999 for ; Thu, 16 Jul 2009 22:30:41 GMT Received: from dlep34.itg.ti.com ([157.170.170.115]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id n6GMSv2v024031; Thu, 16 Jul 2009 17:29:02 -0500 Received: from linux.omap.com (localhost [127.0.0.1]) by dlep34.itg.ti.com (8.13.7/8.13.7) with ESMTP id n6GMSsQo008356; Thu, 16 Jul 2009 17:28:56 -0500 (CDT) Received: from linux.omap.com (localhost [127.0.0.1]) by linux.omap.com (Postfix) with ESMTP id AE51980627; Thu, 16 Jul 2009 17:28:54 -0500 (CDT) X-Original-To: davinci-linux-open-source@linux.davincidsp.com Delivered-To: davinci-linux-open-source@linux.davincidsp.com Received: from dflp52.itg.ti.com (dflp52.itg.ti.com [128.247.22.96]) by linux.omap.com (Postfix) with ESMTP id 99BA080626 for ; Thu, 16 Jul 2009 17:28:53 -0500 (CDT) Received: from white.ext.ti.com (localhost [127.0.0.1]) by dflp52.itg.ti.com (8.13.7/8.13.7) with ESMTP id n6GMSrQ2021722 for ; Thu, 16 Jul 2009 17:28:53 -0500 (CDT) Received: from mail146-wa4-R.bigfish.com (mail-wa4.bigfish.com [216.32.181.113]) by white.ext.ti.com (8.13.7/8.13.7) with ESMTP id n6GMSlDS010893 for ; Thu, 16 Jul 2009 17:28:52 -0500 Received: from mail146-wa4 (localhost.localdomain [127.0.0.1]) by mail146-wa4-R.bigfish.com (Postfix) with ESMTP id A8BEF12A82A8 for ; Thu, 16 Jul 2009 22:28:47 +0000 (UTC) X-SpamScore: -8 X-BigFish: vps-8(zcb8kz9370K98dN18c1Jzz1202hzzz2dh5eh6bh177h61h) X-Spam-TCS-SCL: 0:0 X-FB-SS: 5, X-MS-Exchange-Organization-Antispam-Report: OrigIP: 209.191.125.208; Service: EHS Received: by mail146-wa4 (MessageSwitch) id 1247783324815018_3267; Thu, 16 Jul 2009 22:28:44 +0000 (UCT) Received: from n10.bullet.mail.mud.yahoo.com (n10.bullet.mail.mud.yahoo.com [209.191.125.208]) by mail146-wa4.bigfish.com (Postfix) with SMTP id 707B61A10059 for ; Thu, 16 Jul 2009 22:28:44 +0000 (UTC) Received: from [68.142.200.227] by n10.bullet.mail.mud.yahoo.com with NNFMP; 16 Jul 2009 22:28:44 -0000 Received: from [68.142.201.241] by t8.bullet.mud.yahoo.com with NNFMP; 16 Jul 2009 22:28:44 -0000 Received: from [127.0.0.1] by omp402.mail.mud.yahoo.com with NNFMP; 16 Jul 2009 22:28:43 -0000 X-Yahoo-Newman-Id: 988665.44481.bm@omp402.mail.mud.yahoo.com Received: (qmail 36224 invoked from network); 16 Jul 2009 22:28:43 -0000 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=s1024; d=pacbell.net; h=Received:X-YMail-OSG:X-Yahoo-Newman-Property:From:To:Subject:Date:User-Agent:Cc:References:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding:Content-Disposition:Message-Id; b=zt1Dc8y3DuF8bHSmi8VA4Wel+DF3+n3fHkTAzccXUW+XZffwZkcg2kHWQGyyYmmjLH6Fbsb+OCN2/3k77qSil5lhs6a8pos9W7CFXGylmKpEJRejLhu8a5rdTdNEPFu3eb7p0vEq83W2ItOXpEIuO5R+5bAy/DU27rwcoLV3SNc= ; Received: from unknown (HELO albert) (david-b@69.226.209.118 with plain) by smtp104.sbc.mail.sp1.yahoo.com with SMTP; 16 Jul 2009 22:28:42 -0000 X-YMail-OSG: wbqyUxwVM1n6fcn.NwHkR3_aYeDyIQsaF5A1n0qYvTu386P_MtQ6oVLbVbCAIo4cWLriuuZ1V_T04MZAGCZFvp49T_b3DD6QB0pAEAJJ1YiEf10ZzLE0Inc3hKHukChFL23IdrjgBv6rkkMZbts1a8MPZV9PA6zdP9jnnLTfQRgBNCpDWtqH4Egy9AWPfl4TGeWzESgiLGuwu4Xxmd2ox1wyKFns2I.JuNMs0GNVVOQtOU9pu7z_QzbK42SJVo5FWspucjqijMW3pLp4mMC6N2hV6MXk0aXUnFewSC0ssZe5rBRdBmX_eMZ9gdvQzswXFlgHaahxo5vc.Z6FzQ-- X-Yahoo-Newman-Property: ymail-3 From: David Brownell To: s-paulraj@ti.com Date: Thu, 16 Jul 2009 15:28:42 -0700 User-Agent: KMail/1.9.10 References: <1247690282-995-1-git-send-email-s-paulraj@ti.com> In-Reply-To: <1247690282-995-1-git-send-email-s-paulraj@ti.com> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200907161528.42522.david-b@pacbell.net> Cc: davinci-linux-open-source@linux.davincidsp.com Subject: Re: [PATCH 1/6] DaVinci: SPI Driver for DaVinci and DA8xx SOC's X-BeenThere: davinci-linux-open-source@linux.davincidsp.com X-Mailman-Version: 2.1.4 Precedence: list List-Id: davinci-linux-open-source.linux.davincidsp.com List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: davinci-linux-open-source-bounces@linux.davincidsp.com Errors-To: davinci-linux-open-source-bounces@linux.davincidsp.com On Wednesday 15 July 2009, s-paulraj@ti.com wrote: >  arch/arm/mach-davinci/include/mach/spi.h |   45 ++ >  drivers/spi/Kconfig                      |    7 + >  drivers/spi/Makefile                     |    1 + >  drivers/spi/davinci_spi.c                |  751 ++++++++++++++++++++++++++++++ >  drivers/spi/davinci_spi.h                |  163 +++++++ >  5 files changed, 967 insertions(+), 0 deletions(-) It's getting a lot closer. See the appended ... the issues I see remaining are partly performance (if you only set the FMT register in the setup code the per-message overhead will be a lot less) and partly functional (those per-device settings that are treated as global, or wrongly held back on v1 parts). Let me know if you plan to address either of those. - Dave ==================== CUT HERE Minor fixes: * get rid of global "sdev" ... driver should work with more than one controller * rename set_bits() as set_io_bits(), minimizing confusion with standard set_bit() call; likewise clear_bits() * Version 1 chips can also handle SPI_NO_CS * Remove duplicate or otherwise unneeded #includes Less-minor ones: * Remove confusion between buses (MOSI/MISO/SCK; one per SPI controller) and the chip-selects used to share them. * Fix test for SPI_CPHA ... Didn't touch: * All the stuff in davinci_spi_bufs_prep() which really belongs in davinci_spi_setup() ... it's just a slowdown, since the FMT register could be written just once. * The WDELAY and parity stuff in davinci_spi_bufs_prep() which doesn't kick in for version 1 controllers (even though they support it) and which is handled as *global* options instead of per-device ones (controls are in per-device FMT registers): (a) set it with other updates to FMT register (b) make those per-device policies --- drivers/spi/davinci_spi.c | 115 ++++++++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 56 deletions(-) --- a/drivers/spi/davinci_spi.c +++ b/drivers/spi/davinci_spi.c @@ -16,20 +16,14 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#include -#include -#include +#include #include #include -#include #include -#include #include #include -#include #include #include -#include #include #include @@ -38,7 +32,6 @@ #include "davinci_spi.h" -struct device *sdev; static void davinci_spi_rx_buf_u8(u32 data, struct davinci_spi *davinci_spi) { @@ -76,7 +69,7 @@ static u32 davinci_spi_tx_buf_u16(struct return data; } -static inline void set_bits(void __iomem *addr, u32 bits) +static inline void set_io_bits(void __iomem *addr, u32 bits) { u32 v = ioread32(addr); @@ -84,7 +77,7 @@ static inline void set_bits(void __iomem iowrite32(v, addr); } -static inline void clear_bits(void __iomem *addr, u32 bits) +static inline void clear_io_bits(void __iomem *addr, u32 bits) { u32 v = ioread32(addr); @@ -92,14 +85,14 @@ static inline void clear_bits(void __iom iowrite32(v, addr); } -static inline void set_fmt_bits(void __iomem *addr, u32 bits, int bus_num) +static inline void set_fmt_bits(void __iomem *addr, u32 bits, int cs_num) { - set_bits(addr + SPIFMT0 + (0x4 * bus_num), bits); + set_io_bits(addr + SPIFMT0 + (0x4 * cs_num), bits); } -static inline void clear_fmt_bits(void __iomem *addr, u32 bits, int bus_num) +static inline void clear_fmt_bits(void __iomem *addr, u32 bits, int cs_num) { - clear_bits(addr + SPIFMT0 + (0x4 * bus_num), bits); + clear_io_bits(addr + SPIFMT0 + (0x4 * cs_num), bits); } /* @@ -119,7 +112,7 @@ static void davinci_spi_chipselect(struc * line for the controller */ if (value == BITBANG_CS_INACTIVE) { - set_bits(davinci_spi->base + SPIDEF, CS_DEFAULT); + set_io_bits(davinci_spi->base + SPIDEF, CS_DEFAULT); data1_reg_val |= CS_DEFAULT << SPIDAT1_CSNR_SHIFT; iowrite32(data1_reg_val, davinci_spi->base + SPIDAT1); @@ -179,14 +172,14 @@ static int davinci_spi_setup_transfer(st hz = spi->max_speed_hz; clear_fmt_bits(davinci_spi->base, SPIFMT_CHARLEN_MASK, - spi->master->bus_num); + spi->chip_select); set_fmt_bits(davinci_spi->base, bits_per_word & 0x1f, - spi->master->bus_num); + spi->chip_select); prescale = ((clk_get_rate(davinci_spi->clk) / hz) - 1) & 0xff; - clear_fmt_bits(davinci_spi->base, 0x0000ff00, spi->master->bus_num); - set_fmt_bits(davinci_spi->base, prescale << 8, spi->master->bus_num); + clear_fmt_bits(davinci_spi->base, 0x0000ff00, spi->chip_select); + set_fmt_bits(davinci_spi->base, prescale << 8, spi->chip_select); return 0; } @@ -226,84 +219,90 @@ static int davinci_spi_bufs_prep(struct { int op_mode = 0; - /* configuraton parameter for SPI */ + /* + * Set up device-specific SPI configuration parameters. + * Most of these would normally be handled in spi_setup(), + * updating the per-chipselect FMT registers, but some of + * them use global controls like SPI_LOOP and SPI_READY. + */ + if (spi->mode & SPI_LSB_FIRST) set_fmt_bits(davinci_spi->base, SPIFMT_SHIFTDIR_MASK, - spi->master->bus_num); + spi->chip_select); else clear_fmt_bits(davinci_spi->base, SPIFMT_SHIFTDIR_MASK, - spi->master->bus_num); + spi->chip_select); if (spi->mode & SPI_CPOL) set_fmt_bits(davinci_spi->base, SPIFMT_POLARITY_MASK, - spi->master->bus_num); + spi->chip_select); else clear_fmt_bits(davinci_spi->base, SPIFMT_POLARITY_MASK, - spi->master->bus_num); + spi->chip_select); - if (!(spi->mode & SPI_CPOL)) + if (!(spi->mode & SPI_CPHA)) set_fmt_bits(davinci_spi->base, SPIFMT_PHASE_MASK, - spi->master->bus_num); + spi->chip_select); else clear_fmt_bits(davinci_spi->base, SPIFMT_PHASE_MASK, - spi->master->bus_num); + spi->chip_select); if (davinci_spi->version == SPI_VERSION_2) { clear_fmt_bits(davinci_spi->base, SPIFMT_WDELAY_MASK, - spi->master->bus_num); + spi->chip_select); set_fmt_bits(davinci_spi->base, - ((davinci_spi->pdata->wdelay << - SPIFMT_WDELAY_SHIFT) & - SPIFMT_WDELAY_MASK), - spi->master->bus_num); + (davinci_spi->pdata->wdelay + << SPIFMT_WDELAY_SHIFT) + & SPIFMT_WDELAY_MASK, + spi->chip_select); if (davinci_spi->pdata->odd_parity) set_fmt_bits(davinci_spi->base, SPIFMT_ODD_PARITY_MASK, - spi->master->bus_num); + spi->chip_select); else clear_fmt_bits(davinci_spi->base, SPIFMT_ODD_PARITY_MASK, - spi->master->bus_num); + spi->chip_select); if (davinci_spi->pdata->parity_enable) set_fmt_bits(davinci_spi->base, SPIFMT_PARITYENA_MASK, - spi->master->bus_num); + spi->chip_select); else clear_fmt_bits(davinci_spi->base, SPIFMT_PARITYENA_MASK, - spi->master->bus_num); + spi->chip_select); if (davinci_spi->pdata->wait_enable) set_fmt_bits(davinci_spi->base, SPIFMT_WAITENA_MASK, - spi->master->bus_num); + spi->chip_select); else clear_fmt_bits(davinci_spi->base, SPIFMT_WAITENA_MASK, - spi->master->bus_num); + spi->chip_select); if (davinci_spi->pdata->timer_disable) set_fmt_bits(davinci_spi->base, SPIFMT_DISTIMER_MASK, - spi->master->bus_num); + spi->chip_select); else clear_fmt_bits(davinci_spi->base, SPIFMT_DISTIMER_MASK, - spi->master->bus_num); + spi->chip_select); } /* Clock internal */ if (davinci_spi->pdata->clk_internal) - set_bits(davinci_spi->base + SPIGCR1, + set_io_bits(davinci_spi->base + SPIGCR1, SPIGCR1_CLKMOD_MASK); else - clear_bits(davinci_spi->base + SPIGCR1, + clear_io_bits(davinci_spi->base + SPIGCR1, SPIGCR1_CLKMOD_MASK); /* master mode default */ - set_bits(davinci_spi->base + SPIGCR1, SPIGCR1_MASTER_MASK); + set_io_bits(davinci_spi->base + SPIGCR1, SPIGCR1_MASTER_MASK); if (davinci_spi->pdata->intr_level) iowrite32(SPI_INTLVL_1, davinci_spi->base + SPILVL); @@ -311,12 +310,16 @@ static int davinci_spi_bufs_prep(struct iowrite32(SPI_INTLVL_0, davinci_spi->base + SPILVL); /* - * Standard SPI mode uses 4 pins, with chipselect - * 3 pin SPI is a 4 pin variant without CS (SPI_NO_CS) + * Version 1 hardware supports two basic SPI modes: + * - Standard SPI mode uses 4 pins, with chipselect + * - 3 pin SPI is a 4 pin variant without CS (SPI_NO_CS) * (distinct from SPI_3WIRE, with just one data wire; * or similar variants without MOSI or without MISO) - * 4 pin with enable is (SPI_READY | SPI_NO_CS) - * 5 pin SPI variant is standard SPI plus SPI_READY + * + * Version 2 hardware supports an optional handshaking signal, + * so it can support two more modes: + * - 5 pin SPI variant is standard SPI plus SPI_READY + * - 4 pin with enable is (SPI_READY | SPI_NO_CS) */ op_mode = SPIPC0_DIFUN_MASK @@ -330,10 +333,10 @@ static int davinci_spi_bufs_prep(struct iowrite32(op_mode, davinci_spi->base + SPIPC0); if (spi->mode & SPI_LOOP) - set_bits(davinci_spi->base + SPIGCR1, + set_io_bits(davinci_spi->base + SPIGCR1, SPIGCR1_LOOPBACK_MASK); else - clear_bits(davinci_spi->base + SPIGCR1, + clear_io_bits(davinci_spi->base + SPIGCR1, SPIGCR1_LOOPBACK_MASK); return 0; @@ -342,6 +345,7 @@ static int davinci_spi_bufs_prep(struct static int davinci_spi_check_error(struct davinci_spi *davinci_spi, int int_status) { + struct device *sdev = davinci_spi->bitbang.master->dev.parent; if (int_status & SPIFLG_TIMEOUT_MASK) { dev_dbg(sdev, "SPI Time-out Error\n"); @@ -417,7 +421,7 @@ static int davinci_spi_bufs_pio(struct s return ret; /* Enable SPI */ - set_bits(davinci_spi->base + SPIGCR1, SPIGCR1_SPIENA_MASK); + set_io_bits(davinci_spi->base + SPIGCR1, SPIGCR1_SPIENA_MASK); iowrite32(0 | (pdata->c2tdelay << SPI_C2TDELAY_SHIFT) | (pdata->t2cdelay << SPI_T2CDELAY_SHIFT), @@ -427,7 +431,7 @@ static int davinci_spi_bufs_pio(struct s data1_reg_val = pdata->cs_hold << SPIDAT1_CSHOLD_SHIFT; tmp = ~(0x1 << spi->chip_select); - clear_bits(davinci_spi->base + SPIDEF, ~tmp); + clear_io_bits(davinci_spi->base + SPIDEF, ~tmp); data1_reg_val |= tmp << SPIDAT1_CSNR_SHIFT; @@ -437,7 +441,7 @@ static int davinci_spi_bufs_pio(struct s /* Determine the command to execute READ or WRITE */ if (t->tx_buf) { - clear_bits(davinci_spi->base + SPIINT, SPIINT_MASKALL); + clear_io_bits(davinci_spi->base + SPIINT, SPIINT_MASKALL); while (1) { tx_data = davinci_spi->get_tx(davinci_spi); @@ -490,7 +494,7 @@ static int davinci_spi_bufs_pio(struct s int i; for (i = 0; i < davinci_spi->count; i++) { - set_bits(davinci_spi->base + SPIINT, + set_io_bits(davinci_spi->base + SPIINT, SPIINT_BITERR_INTR | SPIINT_OVRRUN_INTR | SPIINT_RX_INTR); @@ -592,7 +596,6 @@ static int davinci_spi_probe(struct plat } dev_set_drvdata(&pdev->dev, master); - sdev = &pdev->dev; davinci_spi = spi_master_get_devdata(master); if (davinci_spi == NULL) { @@ -659,9 +662,9 @@ static int davinci_spi_probe(struct plat davinci_spi->bitbang.setup_transfer = davinci_spi_setup_transfer; davinci_spi->bitbang.txrx_bufs = davinci_spi_bufs_pio; - davinci_spi->bitbang.flags = SPI_LSB_FIRST | SPI_LOOP; + davinci_spi->bitbang.flags = SPI_NO_CS | SPI_LSB_FIRST | SPI_LOOP; if (davinci_spi->version == SPI_VERSION_2) - davinci_spi->bitbang.flags |= SPI_NO_CS | SPI_READY; + davinci_spi->bitbang.flags |= SPI_READY; davinci_spi->version = pdata->version; davinci_spi->get_rx = davinci_spi_rx_buf_u8;