[PATCH] CS4270 node is misplaced in the MPC8610 device tree

[PATCH] CS4270 node is misplaced in the MPC8610 device tree

[Timur Tabi]

The CS4270 is using the second I2C bus, not the first, on the Freescale
MPC8610 HPCD, so its node in the device tree belongs under 'i2c@3100'
and not 'i2c@3000'.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

Kumar, this is a must-fix for 2.6.26.  

The reason this didn't show up before is because the CS4270 driver
is an old-style driver, and so it doesn't actually use the device tree
to determine the I2C bus/address.

 arch/powerpc/boot/dts/mpc8610_hpcd.dts |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8610_hpcd.dts b/arch/powerpc/boot/dts/mpc8610_hpcd.dts
index 08a780d..771f169 100644
--- a/arch/powerpc/boot/dts/mpc8610_hpcd.dts
+++ b/arch/powerpc/boot/dts/mpc8610_hpcd.dts
@@ -124,13 +124,6 @@
 			interrupts = <43 2>;
 			interrupt-parent = <&mpic>;
 			dfsrr;
-
-			cs4270:codec@4f {
-				compatible = "cirrus,cs4270";
-				reg = <0x4f>;
-				/* MCLK source is a stand-alone oscillator */
-				clock-frequency = <12288000>;
-			};
 		};
 
 		i2c@3100 {
@@ -142,6 +135,13 @@
 			interrupts = <43 2>;
 			interrupt-parent = <&mpic>;
 			dfsrr;
+
+			cs4270:codec@4f {
+				compatible = "cirrus,cs4270";
+				reg = <0x4f>;
+				/* MCLK source is a stand-alone oscillator */
+				clock-frequency = <12288000>;
+			};
 		};
 
 		serial0: serial@4500 {
-- 
1.5.5

Re: [PATCH] CS4270 node is misplaced in the MPC8610 device tree

[Timur Tabi]

Timur Tabi wrote:
> The CS4270 is using the second I2C bus, not the first, on the Freescale
> MPC8610 HPCD, so its node in the device tree belongs under 'i2c@3100'
> and not 'i2c@3000'.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>

Please disregard this patch.  It turns out that that both I2C controllers are
connected to the same bus, so it technically doesn't matter which node a device
is a child of.  However, tests show that sometimes one parent works, and
sometimes the other parent works.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] CS4270 node is misplaced in the MPC8610 device tree

[Anton Vorontsov]

On Mon, May 19, 2008 at 01:20:32PM -0500, Timur Tabi wrote:
> Timur Tabi wrote:
> > The CS4270 is using the second I2C bus, not the first, on the Freescale
> > MPC8610 HPCD, so its node in the device tree belongs under 'i2c@3100'
> > and not 'i2c@3000'.
> > 
> > Signed-off-by: Timur Tabi <timur@freescale.com>
> 
> Please disregard this patch.  It turns out that that both I2C controllers are
> connected to the same bus, so it technically doesn't matter which node a device
> is a child of.  However, tests show that sometimes one parent works, and
> sometimes the other parent works.

Hm... this should be controlled by the PIXIS' BRDCFG0's I2CSPAN and
SERSEL bits:

0: I2C1 and I2C2 buses are separated. System (boot sequencer EEPROM)
   is on I2C1, all others are on I2C2. MPC8610 SPI functions cannot be used.
1: I2C1 and I2C2 are bridged. MPC8610 SPI functions may be used. All I2C
   devices may be accessed via I2C1 controller and/or I2C2 (I2C2 depends on
   SERSEL setting).

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH] CS4270 node is misplaced in the MPC8610 device tree

[Timur Tabi]

Anton Vorontsov wrote:

> Hm... this should be controlled by the PIXIS' BRDCFG0's I2CSPAN and
> SERSEL bits:

Since these pins should not have changed from one kernel version to another, it
doesn't explain how my device "jumped" from I2C2 to I2C1.  I'm debugging this now.

> 1: I2C1 and I2C2 are bridged. MPC8610 SPI functions may be used. All I2C
>    devices may be accessed via I2C1 controller and/or I2C2 (I2C2 depends on
>    SERSEL setting).

I believe this is wrong.  The chip documentation says that the pin controlled by
I2CSPAN selects which of the two inputs to use.  It does not bridge.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] CS4270 node is misplaced in the MPC8610 device tree

[Timur Tabi]

Timur Tabi wrote:
> Anton Vorontsov wrote:
> 
>> Hm... this should be controlled by the PIXIS' BRDCFG0's I2CSPAN and
>> SERSEL bits:
> 
> Since these pins should not have changed from one kernel version to another, it
> doesn't explain how my device "jumped" from I2C2 to I2C1.  I'm debugging this now.

I found the problem.  Your patch "86xx: mpc8610_hpcd: fix second serial port"
fixes it.

The original code moved the I2C devices from I2C1 to I2C2.  On R1 boards, the
default for I2CSPAN was 1 because the chip used on those boards would bridge the
two buses together if I2CSPAN==1.  On R3 boards, a different chip is used, and
I2CSPAN==1 means to use I2C2 instead of I2C1.  On R1 boards, the default for
BRDCFG0 is 0xBD, but on R3 boards it's 0xB8.  So when
mpc8610hpcd_set_monitor_port() wrote 0xBD to BRDCFG0, it moved the I2C devices
from I2C1 to I2C2.

I also had a hard time pinpointing this code because
mpc8610hpcd_set_monitor_port() is called only when the DIU is enabled.  Since I
don't have anything attached to my DIU port, I don't always enable the DIU.  So
some of my kernels had DIU support, and some of them didn't!  Those kernels that
have DIU enabled but not your patch will move the devices to I2C2.  I was
running one of those kernels when I posted my patch.

So in summary, this patch "CS4270 node is misplaced in the MPC8610 device tree"
should not be applied.  The I2C devices should stay on I2C1.

-- 
Timur Tabi
Linux kernel developer at Freescale