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.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
