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

Reply via email to