From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Sat, 5 May 2012 23:36:09 +0200 Subject: [U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start In-Reply-To: <4FA58BEE.8030006@boundarydevices.com> References: <1335324807-16177-1-git-send-email-troy.kisky@boundarydevices.com> <201205051508.24784.marex@denx.de> <4FA58BEE.8030006@boundarydevices.com> Message-ID: <201205052336.09910.marex@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Troy Kisky, > On 5/5/2012 6:08 AM, Marek Vasut wrote: > > Dear Troy Kisky, > > > >> On 4/24/2012 8:33 PM, Troy Kisky wrote: > >>> Other then being very weird, this code was also wrong. > >>> For example, say I set speed to 100K. I'll read back the speed > >>> as 85937. But the speed is really 85937.5, so we I reset > >>> the speed to 85937, I'll get 73660.7. After a couple of transactions > >>> my speed is now exactly 68750 so it will remain there. > >>> > >>> Signed-off-by: Troy Kisky > >>> --- > >>> > >>> drivers/i2c/mxc_i2c.c | 6 ------ > >>> 1 files changed, 0 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c > >>> index 416ffee..fc68062 100644 > >>> --- a/drivers/i2c/mxc_i2c.c > >>> +++ b/drivers/i2c/mxc_i2c.c > >>> @@ -231,12 +231,6 @@ int i2c_imx_start(void) > >>> > >>> struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE; > >>> unsigned int temp = 0; > >>> int result; > >>> > >>> - int speed = i2c_get_bus_speed(); > >>> - u8 clk_idx = i2c_imx_get_clk(speed); > >>> - u8 idx = i2c_clk_div[clk_idx][1]; > >>> - > >>> - /* Store divider value */ > >>> - writeb(idx,&i2c_regs->ifdr); > >>> > >>> /* Enable I2C controller */ > >>> writeb(0,&i2c_regs->i2sr); > >> > >> Marek would you care to ack/nak this? It is deleting code that you > >> added. > > > > Ok, who will set the controller speed if you remove this? > > i2c_init is the only function that writes the ifdr register after this > patch. And i2c_init() is called on every boot. Correct? > This is fine because this register is not affected by a software reset. I take it you verified this or that you're sure here :) > If this register were affected by a software reset, then the current code > would not work either, as i2c_imx_start is reading from this register > before trying(and often failing) to set it to the same value. Reading from ifdr? That seems indeed wrong :-( Well ... I have no objection then Acked-by: Marek Vasut > >> Thanks > >> Troy > > > > Best regards, > > Marek Vasut Best regards, Marek Vasut