Jon Smirl wrote:
> On 7/31/08, Trent Piepho <[EMAIL PROTECTED]> wrote:
>> On Thu, 31 Jul 2008, Jon Smirl wrote:
>> > As for the source clock, how about creating a new global like
>> > ppc_proc_freq called ppc_ipb_freq. The platform code can then set the
>> > right clock value into the variable. For mpc8xxxx get it from uboot.
>> > mpc5200 can easily compute it from ppc_proc_freq and checking how the
>> > ipb divider is set. That will move the clock problem out of the i2c
>> > driver.
>>
>>
>> There is a huge variation in where the I2C source clock comes from.
>> Sometimes it's the system bus, sometimes ethernet, sometimes SEC, etc. If
>> I look at u-boot (which might not be entirely correct or complete), I see:
>>
>> 83xx: 5 different clock sources
>> 85xx: 3 different clock sources
>> 86xx: 2 different clock sources
>>
>> But there's more. Sometimes the two I2C controllers don't use the same
>> clock! So even if you add 10 globals with different clocks, and then add
>> code to the mpc i2c driver so if can figure out which one to use given the
>> platform, it's still not enough because you need to know which controller
>> the device node is for.
>>
>> IMHO, what Timur suggested of having u-boot put the source clock into the
>> i2c node makes the most sense. U-boot has to figure this out, so why
>> duplicate the work?
>>
>> Here's my idea:
>>
>> [EMAIL PROTECTED] {
>> compatible = "fsl-i2c";
>> bus-frequency = <100000>;
>>
>> /* Either */
>> source-clock-frequency = <0>;
>> /* OR */
>> source-clock = <&ccb>;
>> };
>
> Can't we hide a lot of this on platforms where the source clock is not
> messed up? For example the mpc5200 doesn't need any of this, the
> needed frequency is already available in mpc52xx_find_ipb_freq().
> mpc5200 doesn't need any uboot change.
>
> Next would be normal mpc8xxx platforms where i2c is driven by a single
> clock, add a uboot filled in parameter in the root node (or I think it
> can be computed off of the ones uboot is already filling in). make a
> mpc8xxx_find_i2c_freq() function. May not need to change device tree
> and uboot.
>
> Finally use this for those days when the tea leaves were especially
> bad. Both a device tree and uboot change.
>
>> Except the i2c clock isn't always a based on an integer divider of the CCB
>> frequency. What's more, it's not always the same for both i2c controllers.
>> Suppose i2c #1 uses CCB times 2/3 and i2c #2 uses CCB/2, how would
>> fsl_get_i2c_freq() figure that out from bus-frequency and
>> i2c-clock-divider?
>
> If you get the CCB frequency from uboot and know the chip model, can't
> you compute these in the platform code? Then make a
> mpc8xxx_find_i2c_freq(cell_index).
We can, of course, but do we want to? #ifdef's are not acceptable for
Linux which means scanning the model property to get the divider from
some table. And when a new MPC model shows up, we need to update the
table. This can all be saved and avoided by adding a I2C clock source
divider or frequency property to the FDT. The FDT is to describe the
hardware and the fixed divider value is a property of it.
I'm in favor of a I2C node specific "divider" property because it does
not rely on a boot-loader filling in the real value. It's fixed for a
certain MPC model. And the I2C source clock frequency is then just:
fsl_get_sys_freq() / divider.
I'm quite sure that work for all MPCs, but at least for the one covered
by the i2c-mpc driver.
Furthermore, mpc52xx_find_ipb_freq() does the same as
fsl_get_sys_freq(). It looks up the value for the property
"bus-frequency" of the soc. We don't need a mpc8xxx_find_i2c_freq() but
a common fsl_get_i2c_freq() for all MPCs.
Wolfgang.
_______________________________________________
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
