[U-Boot] [PATCH V4] I2C: mxc_i2c rework

[Jason Hui]

Hi, Marek,

On Wed, Aug 10, 2011 at 3:11 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
> On Wednesday, August 10, 2011 08:26:05 AM Jason Hui wrote:
>> On Wed, Aug 10, 2011 at 1:59 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
>> > On Saturday, July 30, 2011 12:52:53 PM Marek Vasut wrote:
>> >> On Saturday, July 30, 2011 08:42:19 AM Jason Hui wrote:
>> >> > Hi, Marek,
>> >> >
>> >> > On Sat, Jul 30, 2011 at 1:09 AM, Marek Vasut <marek.vasut at gmail.com>
> wrote:
>> >> > > On Friday, July 29, 2011 01:24:49 PM Jason Hui wrote:
>> >> > >> Hi, Marek,
>> >> > >>
>> >> > >> On Fri, Jul 29, 2011 at 5:32 PM, Marek Vasut
>> >> > >> <marek.vasut at 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 ;-)
>> >> > >>
>> >> > >> I don't think you did the right thing by chaning IPG_PERCLK to
>> >> > >> IPG_CLK As I said, the IPG_CLK is for IP register clock, while
>> >> > >> IPG_PERCLK is for i2c function clock.
>> >> > >>
>> >> > >> if you run clock command from mx51evk, you will get:
>> >> > >> ...
>> >> > >> ipg clock     : 66500000Hz
>> >> > >> ipg per clock : 665000000Hz
>> >> > >> MX51EVK U-Boot >
>> >> > >>
>> >> > >> It will give you that ipg per clock is 665M, which seems too big.
>> >> > >> It is due to we configure
>> >> > >> the pre-divider/pos-devider for perclk to zero, which leads to
>> >> > >> ipg_per clock to be same as pll2 clock.
>> >> > >> But I don't think this will have some issue.
>> >> > >
>> >> > > Yes it will, the divider will be computed to be maximum in all cases
>> >> > > ... you won't be able it divide 665MHz by anything from the table
>> >> > > 40-7 in MX51RM to achieve any reasonable frequency.
>> >> > >
>> >> > > On the contrary, 66.5MHz does give fine results.
>> >> >
>> >> > it that, we can change the ipg_perclk to low freq, but we should not
>> >> > change the i2c clock to ipg_clk, this is not correct.
>> >> >
>> >> > >> BTW, I have applied your patch and test on mx53evk board, it seems
>> >> > >> the i2c does not work correctly.
>> >> > >
>> >> > > Great. The clock used by the I2C module for this task are likely the
>> >> > > module_clock, which are -- like on MX51 -- 66.5MHz. What do you get
>> >> > > when you run the "clock" command on MX53EVK ?
>> >> >
>> >> > I'm not in the office for the whole next week, thus, I can't give you
>> >> > the clock output for
>> >> > mx53evk, but I'm sure the ipg_perclk on mx53evk is not set at 66.5Mhz.
>> >>
>> >> Yes, it's 33.3MHz on iMX53. I got a (remote) hand of a MX53 board. The
>> >> IPG clock, on the other hand, are 66.5MHz on both MX51 and MX53. Maybe
>> >> the issue with PMIC you're seeing is something else?
>> >>
>> >> btw. how did you obtain these results ? Can you check the FDR divider
>> >> value with and without this patch?
>>
>> You can use the clock command to get it. As for the FDR, you want me to
>> check it on i.mx53evk board right?
>
> Yes please. I hope to get my hands on mx53 system in the evening probably, but
> I'd still be grateful if you could test it.

The clock for i2c should be ipg_perclk, the linux kernel code has been
fixed by the following
commit:

commit 9d73242458d9a2fe26e2e240488063d414eacb1c
Author: Lothar Waßmann <LW at KARO-electronics.de>
Date:   Mon Jul 4 15:52:17 2011 +0200

    mach-mx5: fix the I2C clock parents

    The clock from which the I2C timing is derived is the ipg_perclk
not ipg_clk.

    I2C bus frequency was lower by a factor of ~8 due to the clock divider
    calculation being based on 66.5MHz IPG clock while the bus actually
    uses 8MHz ipg_perclk.

    Kernel version: 3.0.0-rc2 branch 'imx-for-next' of
git://git.pengutronix.de/git/imx/linux-2.6

    Signed-off-by: Lothar Waßmann <LW at KARO-electronics.de>
    Signed-off-by: Sascha Hauer <s.hauer at pengutronix.de>

Jason

>
>>
>> Jason
>>
>> >> Thanks
>> >>
>> >> > Jason
>> >> >
>> >> > > Thanks
>> >> > >
>> >> > >> After apply your patch:
>> >> > >> MX53EVK U-Boot > pmic dump  30
>> >> > >> PMIC ID: 0x0000ffff [Rev: unknown]
>> >> > >>
>> >> > >> 0x00: 0001ffff 00ffffff 0039ffff 0000ffff 00ffffff 0000ffff
>> >> > >> 0000ffff 0000ffff 0x08: 0000ffff 0000ffff 0000ffff 0000ffff
>> >> > >> 0000ffff 0000ffff 0000ffff 0040ffff 0x10: 0000ffff 0000ffff
>> >> > >> 0000ffff 0000ffff 0000ffff 0001ffff 0000ffff 0000ffff 0x18:
>> >> > >> 0045ffff 0045ffff 0000ffff 0080ffff 0021ffff 0000ffff 0002ffff
>> >> > >> 0000ffff 0x20: 0004ffff 0000ffff 0021ffff 0000ffff 0000ffff
>> >> > >> 0000ffff 0000ffff 0000ffff 0x28: 0000ffff 0000ffff 0000ffff
>> >> > >> 0000ffff 0000ffff 0004ffff 0000ffff 00aeffff
>> >> > >>
>> >> > >> The old:
>> >> > >> MX53EVK U-Boot > pmic dump 30
>> >> > >> PMIC ID: 0x000045d0 [Rev: 2.0]
>> >> > >>
>> >> > >> 0x00: 00015088 00ffffff 00395208 00000081 00fff7ff 0000401c
>> >> > >> 00000418 000045d0 0x08: 00000000 00000000 00000001 00000000
>> >> > >> 00000000 00000040 00000000 00400000 0x10: 00000000 00000000
>> >> > >> 00000000 00000000 00000011 0001ffff 00000000 00007fff 0x18:
>> >> > >> 00454a52 00456739 0000631a 0080739c 0021284a 00000a0a 00024fd0
>> >> > >> 000001d8 0x20: 00049208 00000000 00218000 00000000 00000000
>> >> > >> 00000000 00000000 00000000 0x28: 00000000 00000000 00000000
>> >> > >> 00008000 00000000 00046046 000001c0 00aeeaee
>> >> > >>
>> >> > >> Jason
>