From patchwork Thu May 14 16:52:23 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Joakim Tjernlund X-Patchwork-Id: 27216 X-Patchwork-Delegate: galak@kernel.crashing.org Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id CCD85B7080 for ; Fri, 15 May 2009 02:52:59 +1000 (EST) Received: by ozlabs.org (Postfix) id 888F7DE1BD; Fri, 15 May 2009 02:52:55 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from ozlabs.org (localhost [127.0.0.1]) by ozlabs.org (Postfix) with ESMTP id 85A90DE1BC for ; Fri, 15 May 2009 02:52:55 +1000 (EST) X-Original-To: linuxppc-dev@ozlabs.org Delivered-To: linuxppc-dev@ozlabs.org Received: from gw1.transmode.se (gw1.transmode.se [213.115.205.20]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id ACBB5DE05A for ; Fri, 15 May 2009 02:52:28 +1000 (EST) Received: from sesr04.transmode.se (sesr04.transmode.se [192.168.201.15]) by gw1.transmode.se (Postfix) with ESMTP id A6617258898; Thu, 14 May 2009 18:53:53 +0200 (CEST) In-Reply-To: References: <4A0BD1DB.4090305@doredevelopment.dk> <20090514083453.GB3043@pengutronix.de> <4A0BDAD6.5020005@doredevelopment.dk> Subject: Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg X-KeepSent: 3CD158BA:04CCFBB5-C12575B6:005911D5; type=4; name=$KeepSent To: Esben Haabendal X-Mailer: Lotus Notes Release 8.5 December 05, 2008 Message-ID: From: Joakim Tjernlund Date: Thu, 14 May 2009 18:52:23 +0200 X-MIMETrack: Serialize by Router on sesr04/Transmode(Release 8.5 HF407|May 07, 2009) at 2009-05-14 18:52:23 MIME-Version: 1.0 Cc: linuxppc-dev@ozlabs.org X-BeenThere: linuxppc-dev@ozlabs.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@ozlabs.org Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@ozlabs.org Esben Haabendal wrote on 14/05/2009 12:17:48: > On Thu, May 14, 2009 at 11:58 AM, Joakim Tjernlund > wrote: > >> > >> The problem is that after the STOP condition, the following i2c_msg will be > >> attempted with a repeated START, which according to specification will > >> cause a MAL, which also happens.  My MPC8360ERM reads: > >> > >> "Attempting a repeated START at the wrong time (or if the bus is owned > >> by another master), results in loss of arbitration." > >> > >> Which I know read as it that we must own the I2C bus when using RSTA. > > > > I agree with the theory, but isn't the problem here that STOP is performed in > > the middle of this transaction? > > Remove the STOP and RSTA will work I think. > > That was also my first idea. > > But at least for my CS4265, reads does not work without a STOP following the > last byte. I guess I am not the first to experience this, as indicated > by the current > i2c-mpc.c implementation. > As far as I can understand the I2C specification, a STOP should not be required > after reads, but I am not sure. > > The cost of the additional STOP is really small. The only issue I can see, > is that with the STOP in the midle of an i2c_transfer, the I2C bus is released. > This naturally also happens with the current implementation, so I believe > that this patch is a clean improvement. > > Anyways, if someone can figure out how to achive this without STOP, > I will be happy (re)test with my hardware setup here. From just playing a little, I think the below patch will get you far. However, I had to change this too to make it stable: static void mpc_i2c_start(struct mpc_i2c *i2c) { /* Clear arbitration */ writeb(0, i2c->base + MPC_I2C_SR); #if 1 // fix, not sure why writeccr(i2c, 0); udelay(5); #endif /* Start with MEN */ writeccr(i2c, CCR_MEN); } PS. I added back the linuxppc list, let it stay. diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c index 6c1cddd..d7edcd2 100644 --- a/drivers/i2c/busses/i2c-mpc.c +++ b/drivers/i2c/busses/i2c-mpc.c @@ -189,9 +189,6 @@ static int mpc_write(struct mpc_i2c *i2c, int target, unsigned timeout = i2c->adap.timeout; u32 flags = restart ? CCR_RSTA : 0; - /* Start with MEN */ - if (!restart) - writeccr(i2c, CCR_MEN); /* Start as master */ writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags); /* Write target byte */ @@ -220,9 +217,6 @@ static int mpc_read(struct mpc_i2c *i2c, int target, int i, result; u32 flags = restart ? CCR_RSTA : 0; - /* Start with MEN */ - if (!restart) - writeccr(i2c, CCR_MEN); /* Switch to read - restart */ writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags); /* Write target address byte - this time with the read flag set */ @@ -241,18 +235,15 @@ static int mpc_read(struct mpc_i2c *i2c, int target, readb(i2c->base + MPC_I2C_DR); } - for (i = 0; i < length; i++) { + for (i = length; i ; --i) { result = i2c_wait(i2c, timeout, 0); if (result < 0) return result; - /* Generate txack on next to last byte */ - if (i == length - 2) + /* Generate txack on next to last byte(s) */ + if (i <= 2) writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_TXAK); - /* Generate stop on last byte */ - if (i == length - 1) - writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_TXAK); - data[i] = readb(i2c->base + MPC_I2C_DR); + data[length-i] = readb(i2c->base + MPC_I2C_DR); } return length;