Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Wolfgang Grandegger
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 mpc8

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Wolfgang Grandegger
Trent Piepho wrote: On Thu, 31 Jul 2008, Jon Smirl wrote: [...snip...] I don't see why we want to go through the trouble of having uboot tell us things about a chip that are fixed in stone. Obviously something like the frequency of the external crystal needs to be passed up, but why pass up

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Timur Tabi
On Thu, Jul 31, 2008 at 6:32 PM, Trent Piepho [EMAIL PROTECTED] wrote: The real problem, which kept me from making a patch to do this months ago, is that the source clock that the I2C freq divider applies to is different for just about every MPC8xxx platform. Not just a different value, but a

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Timur Tabi
On Thu, Jul 31, 2008 at 9:35 PM, Jon Smirl [EMAIL PROTECTED] wrote: I've having trouble with whether these clocks are a system resource or something that belongs to i2c. If they are a system resource then we should make nodes in the root and use a phandle in the i2c node to link to them. I

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Jon Smirl
On 8/1/08, Timur Tabi [EMAIL PROTECTED] wrote: On Thu, Jul 31, 2008 at 9:35 PM, Jon Smirl [EMAIL PROTECTED] wrote: I've having trouble with whether these clocks are a system resource or something that belongs to i2c. If they are a system resource then we should make nodes in the root

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Jon Smirl
On 8/1/08, Timur Tabi [EMAIL PROTECTED] wrote: On Thu, Jul 31, 2008 at 9:35 PM, Jon Smirl [EMAIL PROTECTED] wrote: I've having trouble with whether these clocks are a system resource or something that belongs to i2c. If they are a system resource then we should make nodes in the root

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Grant Likely
On Thu, Jul 31, 2008 at 6:46 PM, Trent Piepho [EMAIL PROTECTED] wrote: The i2c controller just uses some system clock that was handy. For each chip, the designers consult tea leaves to choose a system clock at random to connect to the i2c controller. heh; I thought it was the phase of the

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Grant Likely
On Fri, Aug 1, 2008 at 1:25 AM, Wolfgang Grandegger [EMAIL PROTECTED] wrote: 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

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Geert Uytterhoeven
On Thu, 31 Jul 2008, Trent Piepho wrote: The i2c controller just uses some system clock that was handy. For each chip, the designers consult tea leaves to choose a system clock at random to connect to the i2c controller. Oh, they decided which clock to choose at design time, not at power-on

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Timur Tabi
Jon Smirl wrote: What about the Efika which is mpc5200 based and doesn't use uboot? How does the Efika handle the dozen other properties that U-Boot normally initializes in the device tree? -- Timur Tabi Linux Kernel Developer @ Freescale ___

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Scott Wood
On Fri, Aug 01, 2008 at 08:17:25AM -0500, Timur Tabi wrote: On Thu, Jul 31, 2008 at 6:32 PM, Trent Piepho [EMAIL PROTECTED] wrote: Sometimes the two i2c controllers don't even have the same clock. On which platform is that the case? I thought I had all 8[356]xx boards covered. Did I

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Jon Smirl
On 8/1/08, Timur Tabi [EMAIL PROTECTED] wrote: Jon Smirl wrote: What about the Efika which is mpc5200 based and doesn't use uboot? How does the Efika handle the dozen other properties that U-Boot normally initializes in the device tree? Efika is like the original OpenFirmware. It has a

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Trent Piepho
On Fri, 1 Aug 2008, Timur Tabi wrote: On Thu, Jul 31, 2008 at 6:32 PM, Trent Piepho [EMAIL PROTECTED] wrote: The real problem, which kept me from making a patch to do this months ago, is that the source clock that the I2C freq divider applies to is different for just about every MPC8xxx

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Timur Tabi
Trent Piepho wrote: All 83xx other than 832x. Never mind, I forgot that 83xx support for i2c1_clk was already in U-Boot: #if defined(CONFIG_MPC834X) i2c1_clk = tsec2_clk; #elif defined(CONFIG_MPC8360) i2c1_clk = csb_clk; #elif defined(CONFIG_MPC832X) i2c1_clk = enc_clk;

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-08-01 Thread Trent Piepho
On Fri, 1 Aug 2008, Jon Smirl wrote: I don't like the third choice. Keep a simple Linux driver for i2c and the platform, and then move all of the messy code into uboot. BTW, the messy code is about 10 lines. It's going to take more than 10 lines to hide those 10 lines. It's not being _moved_

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Trent Piepho
On Thu, 31 Jul 2008, Timur Tabi wrote: Jon Smirl wrote: Aren't the tables in the manual there just to make it easy for a human to pick out the line they want? For a computer you'd program the formula that was used to create the tables. Actually, the tables in the manuals are just an

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Trent Piepho
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 mpc8 get it from uboot. mpc5200 can easily compute it from ppc_proc_freq and

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Trent Piepho
On Thu, 31 Jul 2008, Grant Likely wrote: On Thu, Jul 31, 2008 at 09:54:48PM +0200, Wolfgang Grandegger wrote: Thinking more about it, it would be best to add the property i2c-clock-divider to the soc node and implement fsl_get_i2c_freq() in a similar way: [EMAIL PROTECTED] {

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Trent Piepho
On Thu, 31 Jul 2008, Grant Likely wrote: I'm a bit confused. The frequency of the I2C source clock and the real I2C clock frequency are two different things. The first one is common for all I2C devices, the second can be different. What properties would you like to use for defining both?

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Trent Piepho
On Thu, 31 Jul 2008, Timur Tabi wrote: Wolfgang Grandegger wrote: U-Boot does not (yet) use the FDT and it has therefore to use that ugly code to derive the I2C input clock frequency. I think its completely legal to put that hardware specific information into the FDT and get rid of such

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
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 mpc8 get it from uboot.

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Trent Piepho
On Thu, 31 Jul 2008, Jon Smirl wrote: Here's my idea: [EMAIL PROTECTED] { compatible = fsl-i2c; bus-frequency = 10; /* Either */ source-clock-frequency = 0; /* OR */

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Trent Piepho [EMAIL PROTECTED] wrote: On Thu, 31 Jul 2008, Jon Smirl wrote: Here's my idea: [EMAIL PROTECTED] { compatible = fsl-i2c; bus-frequency = 10; /* Either */

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Grant Likely
On Thu, Jul 31, 2008 at 09:19:51PM -0400, 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

Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT

2008-07-31 Thread Jon Smirl
On 7/31/08, Grant Likely [EMAIL PROTECTED] wrote: Your mixing up device tree layout with implementation details. Device tree layout must come first. mpc52xx_find_ipb_freq() is just a convenience function that walks up the device tree looking for a bus-frequency property. So, instead of