From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marek.vasut@gmail.com>
Date: Fri, 29 Jul 2011 11:35:34 +0200
Subject: [U-Boot] [PATCH V3+] I2C: mxc_i2c rework
In-Reply-To: <CAFQ4atTS=ptKnNe_ntfA-ypV5Ctw+AeudHG5=pJYYNmju+f-EA@mail.gmail.com>
References: <1310594283-19819-1-git-send-email-marek.vasut@gmail.com>
	<CAFQ4atTS=ptKnNe_ntfA-ypV5Ctw+AeudHG5=pJYYNmju+f-EA@mail.gmail.com>
Message-ID: <201107291135.34868.marek.vasut@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Friday, July 29, 2011 08:55:14 AM Jason Hui wrote:
> Hi, Marek,
> 
> On Thu, Jul 14, 2011 at 5:58 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Rewrite the mxc_i2c driver.
> >  * This version is much closer to Linux implementation.
> >  * Fixes IPG_PERCLK being incorrectly used as clock source
> >  * Fixes behaviour of the driver on iMX51
> >  * Clean up coding style a bit ;-)
> 
> why you change i2c clock from IPG_PERCLK to IPG_CLK?

On MX51, PERCLK are those fast (680MHz) clock, that's not source of clock for 
I2C. The IPG_CLK (they are 68.5MHz iirc) are source for the I2C. Also, I 
discussed this with Stefano and we agreed this is likely a bug.
> 
> [...]
> 
> > +static void i2c_imx_set_clk(unsigned int rate)
> >  {
> > -       int freq;
> > +       struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
> > +       unsigned int i2c_clk_rate;
> > +       unsigned int div;
> >        int i;
> > 
> > +       /* Divider value calculation */
> >  #if defined(CONFIG_MX31)
> >        struct clock_control_regs *sc_regs =
> >                (struct clock_control_regs *)CCM_BASE;
> > 
> > -       freq = mx31_get_ipg_clk();
> > +       i2c_clk_rate = mx31_get_ipg_clk();
> >        /* start the required I2C clock */
> >        writel(readl(&sc_regs->cgr0) | (3 << I2C_CLK_OFFSET),
> >                &sc_regs->cgr0);
> >  #else
> > -       freq = mxc_get_clock(MXC_IPG_PERCLK);
> > +       i2c_clk_rate = mxc_get_clock(MXC_IPG_CLK);
> >  #endif
> 
> There are two clocks for i2c:
> 
> Peripheral clock (IPBus): source from  ipg_clk_root, which is for IP
> bus register read/write.
> Block clock: source from perclk_root, which is I2C function clock.
> 
> We need get perclk not ipg clock, right?

For divider, we need those slower ones, the IPG_CLK, not PERCLK.
> 
> BTW, do you test this driver on mx53?

No, I don't have one.

> 
> Jason