From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dirk Behme Date: Wed, 11 Jul 2012 08:27:22 +0200 Subject: [U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start In-Reply-To: <4FA6986D.2080004@denx.de> References: <1335324807-16177-1-git-send-email-troy.kisky@boundarydevices.com> <201205052336.09910.marex@denx.de> <4FA5A91C.5030305@boundarydevices.com> <201205060106.48556.marex@denx.de> <4FA6986D.2080004@denx.de> Message-ID: <4FFD1CCA.1020107@de.bosch.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 06.05.2012 17:27, Stefano Babic wrote: > On 06/05/2012 01:06, Marek Vasut wrote: >> Dear Troy Kisky, >> >>> On 5/5/2012 2:36 PM, Marek Vasut wrote: >>>> 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 :) >>> I haven't looked at every reference manual, but if some oddball chip >>> needs it reinitialized after >>> a software reset, then the fix should go into the i2c_reset function, >>> not i2c_imx_start. >>> And this patch would not be introducing a regression for the oddball >>> chip anyway. > > Ok, everything was already clear, and I can also add my: > > Acked-by: Stefano Babic Same for this one: Is this applied anywhere? It doesn't seem to be part of v2012.07-rc1? Best regards Dirk