From: Wolfgang Grandegger <wg@grandegger•com>
To: Jon Smirl <jonsmirl@gmail•com>
Cc: Trent Piepho <xyzzy@speakeasy•org>,
Linuxppc-dev@ozlabs•org, Linux I2C <i2c@lm-sensors•org>,
Scott Wood <scottwood@freescale•com>,
Timur Tabi <timur@freescale•com>
Subject: Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
Date: Fri, 01 Aug 2008 09:25:35 +0200 [thread overview]
Message-ID: <4892BA6F.3010000@grandegger.com> (raw)
In-Reply-To: <9e4733910807311819i60872285ga4829c841185fdc0@mail.gmail.com>
Jon Smirl wrote:
> On 7/31/08, Trent Piepho <xyzzy@speakeasy•org> 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:
>>
>> i2c@0 {
>> 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.
next prev parent reply other threads:[~2008-08-01 7:25 UTC|newest]
Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-25 7:37 [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT Wolfgang Grandegger
2008-07-25 8:51 ` Jochen Friedrich
2008-07-25 9:04 ` Wolfgang Grandegger
2008-07-25 13:12 ` Grant Likely
2008-07-25 14:21 ` Timur Tabi
2008-07-25 15:04 ` Jon Smirl
2008-07-25 15:23 ` Wolfgang Grandegger
2008-07-25 16:19 ` Timur Tabi
2008-07-27 1:27 ` Grant Likely
2008-07-31 11:51 ` Wolfgang Grandegger
2008-07-31 15:49 ` Jon Smirl
2008-07-31 15:55 ` Timur Tabi
2008-07-31 23:32 ` [i2c] " Trent Piepho
2008-08-01 13:17 ` Timur Tabi
2008-08-01 15:47 ` Scott Wood
2008-08-01 19:47 ` Trent Piepho
2008-08-01 19:50 ` Timur Tabi
2008-07-31 17:22 ` Wolfgang Grandegger
2008-07-31 17:31 ` Grant Likely
2008-07-31 17:51 ` Wolfgang Grandegger
2008-07-31 17:54 ` Timur Tabi
2008-07-31 18:07 ` Wolfgang Grandegger
2008-07-31 18:06 ` Timur Tabi
2008-07-31 18:07 ` Grant Likely
2008-07-31 18:10 ` Timur Tabi
2008-07-31 18:21 ` Grant Likely
2008-07-31 18:09 ` Grant Likely
2008-07-31 18:13 ` Timur Tabi
2008-07-31 18:28 ` Grant Likely
2008-07-31 18:35 ` Timur Tabi
2008-07-31 18:57 ` Jon Smirl
2008-07-31 19:01 ` Timur Tabi
2008-07-31 19:25 ` Grant Likely
2008-08-01 0:22 ` [i2c] " Trent Piepho
2008-08-01 1:19 ` Jon Smirl
2008-08-01 1:36 ` Trent Piepho
2008-08-01 1:44 ` Jon Smirl
2008-08-01 15:02 ` Timur Tabi
2008-08-01 16:05 ` Jon Smirl
2008-08-01 7:29 ` Wolfgang Grandegger
2008-08-01 2:03 ` Grant Likely
2008-08-01 2:35 ` Jon Smirl
2008-08-01 13:25 ` Timur Tabi
2008-08-01 14:28 ` Jon Smirl
2008-08-01 14:32 ` Jon Smirl
2008-08-01 21:14 ` Trent Piepho
2008-08-01 7:25 ` Wolfgang Grandegger [this message]
2008-08-01 14:38 ` Grant Likely
2008-07-31 19:01 ` Scott Wood
2008-07-31 19:08 ` Timur Tabi
2008-07-31 19:15 ` Scott Wood
2008-07-31 19:19 ` Timur Tabi
2008-07-31 19:21 ` Scott Wood
2008-07-31 19:22 ` Timur Tabi
2008-07-31 19:11 ` Jon Smirl
2008-07-31 19:14 ` Grant Likely
2008-07-31 19:24 ` Wolfgang Grandegger
2008-07-31 19:24 ` Timur Tabi
2008-07-31 19:54 ` Wolfgang Grandegger
2008-07-31 19:58 ` Timur Tabi
2008-07-31 20:17 ` Wolfgang Grandegger
2008-07-31 20:19 ` Timur Tabi
2008-07-31 20:28 ` Wolfgang Grandegger
2008-07-31 20:28 ` Timur Tabi
2008-07-31 20:30 ` Grant Likely
2008-07-31 20:32 ` Jon Smirl
2008-07-31 20:35 ` Grant Likely
2008-07-31 20:37 ` Timur Tabi
2008-07-31 20:48 ` Grant Likely
2008-07-31 20:55 ` Jon Smirl
2008-07-31 20:56 ` Scott Wood
2008-07-31 20:56 ` Timur Tabi
2008-07-31 21:03 ` Jon Smirl
2008-07-31 21:10 ` Timur Tabi
2008-07-31 21:14 ` Wolfgang Grandegger
2008-07-31 21:17 ` Timur Tabi
2008-08-01 1:16 ` [i2c] " Trent Piepho
2008-08-01 0:57 ` Trent Piepho
2008-07-31 20:35 ` Timur Tabi
2008-07-31 20:43 ` Jon Smirl
2008-07-31 20:44 ` Timur Tabi
2008-07-31 19:59 ` Grant Likely
2008-07-31 20:00 ` Timur Tabi
2008-07-31 20:20 ` Wolfgang Grandegger
2008-07-31 20:19 ` Timur Tabi
2008-08-01 0:46 ` [i2c] " Trent Piepho
2008-08-01 14:34 ` Grant Likely
2008-08-01 14:48 ` Geert Uytterhoeven
2008-07-31 17:35 ` Jon Smirl
2008-07-31 16:51 ` Grant Likely
2008-07-31 17:06 ` Jon Smirl
2008-07-31 17:36 ` Grant Likely
2008-07-31 17:47 ` Jon Smirl
2008-07-31 17:24 ` Wolfgang Grandegger
2008-07-25 15:34 ` Wolfgang Grandegger
2008-07-27 1:25 ` Grant Likely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4892BA6F.3010000@grandegger.com \
--to=wg@grandegger$(echo .)com \
--cc=Linuxppc-dev@ozlabs$(echo .)org \
--cc=i2c@lm-sensors$(echo .)org \
--cc=jonsmirl@gmail$(echo .)com \
--cc=scottwood@freescale$(echo .)com \
--cc=timur@freescale$(echo .)com \
--cc=xyzzy@speakeasy$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox