From patchwork Wed May 1 05:38:46 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dirk Behme X-Patchwork-Id: 240728 X-Patchwork-Delegate: jagannadh.teki@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 56F5B2C00CA for ; Wed, 1 May 2013 15:39:11 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 607F14A2B9; Wed, 1 May 2013 07:39:08 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aZ1Jj12L-ZEG; Wed, 1 May 2013 07:39:08 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 259A84A2AF; Wed, 1 May 2013 07:39:07 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id CA0D14A2AF for ; Wed, 1 May 2013 07:39:01 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8WDyvug8C84L for ; Wed, 1 May 2013 07:38:56 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 BL_NJABL=ERR(-1.5) (only DNSBL check requested) Received: from mail-bk0-f54.google.com (mail-bk0-f54.google.com [209.85.214.54]) by theia.denx.de (Postfix) with ESMTPS id 324964A2AE for ; Wed, 1 May 2013 07:38:48 +0200 (CEST) Received: by mail-bk0-f54.google.com with SMTP id q11so542934bkw.27 for ; Tue, 30 Apr 2013 22:38:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=tSQgz9I4ugDSEnh/sIiwUBYdguBrX6h9rRm/1SfaRGI=; b=ixV7mG7aFrO0S6hWBJ+4Dl4AhAYroigq2HpkgFb1BqbVP7rkIXq0U4cDyvehVMmCOE 8XOoo03rcIEfteVWRWv/PZkpxB4WuGXXFCfBm1Az2Bxeiye43RMokUUHvsH/X1MFYYyJ 3hXbug5I91xUSseAVoeoOnefOMbRBMJpPeZRE/86iOYYd/r72a5AK4q8g1ENK5TPc/e2 SBOVbwk0D5ru0XXVcaEgZ4CbZWhvmxjC7N4TgVAv51YnHYpZW8o4LfKRLwEgSaJDKZVc EuxRS5uqV2kZIPCDwN4UyN9yyetAzVEPA0HYczt0ORsGOyvxjiB+KuXY40yVv8SD+wzR rvbA== X-Received: by 10.204.200.139 with SMTP id ew11mr331805bkb.70.1367386728573; Tue, 30 Apr 2013 22:38:48 -0700 (PDT) Received: from [192.168.178.36] (p4FEE288D.dip0.t-ipconnect.de. [79.238.40.141]) by mx.google.com with ESMTPSA id es13sm190940bkc.8.2013.04.30.22.38.47 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 30 Apr 2013 22:38:47 -0700 (PDT) Message-ID: <5180AA66.7000008@gmail.com> Date: Wed, 01 May 2013 07:38:46 +0200 From: Dirk Behme User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Fabio Estevam , Fabio Estevam References: <1365548785-28684-1-git-send-email-festevam@gmail.com> <516A55E7.9000304@gmail.com> In-Reply-To: <516A55E7.9000304@gmail.com> Cc: u-boot@lists.denx.de, dirk.behme@de.bosch.com Subject: Re: [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de Am 14.04.2013 09:08, schrieb Dirk Behme: > Am 10.04.2013 01:06, schrieb Fabio Estevam: >> From: Fabio Estevam >> >> The glitch in the SPI clock line, which commit 3cea335c34 (spi: >> mxc_spi: Fix spi >> clock glitch durant reset) solved, is back now and itwas >> re-introduced by >> commit d36b39bf0d (spi: mxc_spi: Fix ECSPI reset handling). >> >> Actually the glitch is happening due to always toggling between >> slave mode >> and master mode by configuring the CHANNEL_MODE bits in this reset >> function. >> >> Since the spi driver only supports master mode, set the mode for all >> channels >> always to master mode in order to have a stable, "glitch-free" SPI >> clock line. >> >> Signed-off-by: Fabio Estevam >> --- >> Changes since v1: >> - Introduce MXC_CSPICTRL_MODE_MASK definition >> - Remove additional read of reg_ctrl >> >> arch/arm/include/asm/arch-mx5/imx-regs.h | 1 + >> arch/arm/include/asm/arch-mx6/imx-regs.h | 1 + >> drivers/spi/mxc_spi.c | 17 +++++++++-------- >> 3 files changed, 11 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h >> b/arch/arm/include/asm/arch-mx5/imx-regs.h >> index 249d15a..a71cc13 100644 >> --- a/arch/arm/include/asm/arch-mx5/imx-regs.h >> +++ b/arch/arm/include/asm/arch-mx5/imx-regs.h >> @@ -230,6 +230,7 @@ >> #define MXC_CSPICTRL_EN (1 << 0) >> #define MXC_CSPICTRL_MODE (1 << 1) >> #define MXC_CSPICTRL_XCH (1 << 2) >> +#define MXC_CSPICTRL_MODE_MASK (0xf << 4) >> #define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 12) >> #define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0xfff) << 20) >> #define MXC_CSPICTRL_PREDIV(x) (((x) & 0xF) << 12) >> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h >> b/arch/arm/include/asm/arch-mx6/imx-regs.h >> index eaa7439..d79ab2f 100644 >> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h >> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h >> @@ -346,6 +346,7 @@ struct cspi_regs { >> #define MXC_CSPICTRL_EN (1 << 0) >> #define MXC_CSPICTRL_MODE (1 << 1) >> #define MXC_CSPICTRL_XCH (1 << 2) >> +#define MXC_CSPICTRL_MODE_MASK (0xf << 4) >> #define MXC_CSPICTRL_CHIPSELECT(x) (((x) & 0x3) << 12) >> #define MXC_CSPICTRL_BITCOUNT(x) (((x) & 0xfff) << 20) >> #define MXC_CSPICTRL_PREDIV(x) (((x) & 0xF) << 12) >> diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c >> index 4c19e0b..20419e6 100644 >> --- a/drivers/spi/mxc_spi.c >> +++ b/drivers/spi/mxc_spi.c >> @@ -137,11 +137,15 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave >> *mxcs, unsigned int cs, >> return -1; >> } >> >> - /* Reset spi */ >> - reg_write(®s->ctrl, 0); >> - reg_write(®s->ctrl, MXC_CSPICTRL_EN); >> - >> - reg_ctrl = reg_read(®s->ctrl); >> + /* >> + * Reset SPI and set all CSs to master mode, if toggling >> + * between slave and master mode we might see a glitch >> + * on the clock line >> + */ >> + reg_ctrl = MXC_CSPICTRL_MODE_MASK; > > I was offline some days, giving me some time to think about this ;) > > Most probably it does no harm, but I somehow feel uncomfortable with > setting *all* CSs to master mode. Just because there might be already > (one, several?) CS at master mode and switching it back to the (reset > default) slave will give a glitch on the clock line. > > Wouldn't it be cleaner to keep the master mode only for the CS which > are already in master mode before? > > E.g. instead of > > reg_ctrl = MXC_CSPICTRL_MODE_MASK; > > from above something like > > reg_ctrl = reg_read(®s->ctrl) & MXC_CSPICTRL_MODE_MASK; > > ? > > And then ... > >> + reg_write(®s->ctrl, reg_ctrl); >> + reg_ctrl |= MXC_CSPICTRL_EN; >> + reg_write(®s->ctrl, reg_ctrl); >> >> /* >> * The following computation is taken directly from >> Freescale's code. >> @@ -174,9 +178,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave >> *mxcs, unsigned int cs, >> reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | >> MXC_CSPICTRL_POSTDIV(post_div); >> >> - /* always set to master mode */ >> - reg_ctrl |= 1 << (cs + 4); > > ... keeping thins line? What's about anything like below? Best regards Dirk From: Dirk Behme Date: Wed, 1 May 2013 07:28:38 +0200 Subject: [PATCH] spi: mxc_spi: Keep master mode only for configured channels To avoid a glitch on the clock line while resetting the SPI controller, the commit 0f1411bc8d (spi: mxc_spi: Set master mode for all channels) enables the master mode for all channels. Instead of enabling the master mode for all channels, keep only the channels which are already configured in master mode in this mode. To be able to switch additional channels to master mode, re-introduce the master mode enable which was removed by above commit, then. Signed-off-by: Dirk Behme --- drivers/spi/mxc_spi.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) reg_write(®s->ctrl, reg_ctrl); @@ -178,6 +178,9 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl & ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); + /* always set to master mode */ + reg_ctrl |= 1 << (cs + 4); + /* We need to disable SPI before changing registers */ reg_ctrl &= ~MXC_CSPICTRL_EN; diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 5bed858..843a1f2 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -138,11 +138,11 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, } /* - * Reset SPI and set all CSs to master mode, if toggling - * between slave and master mode we might see a glitch - * on the clock line + * Reset SPI and the CONREG, but keep the CSs which are already + * in master mode. If toggling between slave and master mode we might + * see a glitch on the clock line */ - reg_ctrl = MXC_CSPICTRL_MODE_MASK; + reg_ctrl = reg_read(®s->ctrl) & MXC_CSPICTRL_MODE_MASK; reg_write(®s->ctrl, reg_ctrl); reg_ctrl |= MXC_CSPICTRL_EN;