MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Joakim Tjernlund]

So I upgraded my tree for our custom mpc8321 boards.
I2c and ethernet stopped working.

Ethernet: I am using the ucc_geth driver and now
it seems like it won't TX any pkgs.
ethtool -S eth0  shows that the RX countes are increasing but
the TX ones stays a zero. 

Secondly i2c, I have this in my platform driver:
static struct i2c_board_info __initdata tmcu_i2c_devices[] = {
	{ I2C_BOARD_INFO("rtc-ds1307", 0x68),
	  .type = "ds1337",
	},
};

int __init tmcu_i2c_board_devs(void)
{
	int ret;
	ret = i2c_register_board_info(0, tmcu_i2c_devices,
				      ARRAY_SIZE(tmcu_i2c_devices));
	return ret;
}
arch_initcall(tmcu_i2c_board_devs);

This no longer works, but if I add
rtc@68 {
		compatible = "dallas,ds1337";
		reg = <68>;
	};
to my device tree so the whole I2c node looks like this:
	i2c@3000 {
		#address-cells = <1>;
		#size-cells = <0>;
		cell-index = <0>;
		device_type = "i2c";
		compatible = "fsl-i2c";
		reg = <3000 100>;
		interrupts = <e 8>;
		interrupt-parent = <&ipic>;
		dfsrr;

		rtc@68 {
			compatible = "dallas,ds1337";
			reg = <68>;
		};
	};
it works again. Is my static init of the rtc device faulty or is this
a bug in the kernel?

 Jocke

Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Joakim Tjernlund]

On Mon, 2008-10-13 at 14:01 +0200, Joakim Tjernlund wrote:
> So I upgraded my tree for our custom mpc8321 boards.
> I2c and ethernet stopped working.
> 
> Ethernet: I am using the ucc_geth driver and now
> it seems like it won't TX any pkgs.
> ethtool -S eth0  shows that the RX countes are increasing but
> the TX ones stays a zero. 

The ethernet problem turned out to be .bd_mem_part = MEM_PART_MURAM
not working anymore.
The default .bd_mem_part = MEM_PART_SYSTEM works. No idea as
to what caused this though.

    Jocke

Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Scott Wood]

On Mon, Oct 13, 2008 at 02:01:55PM +0200, Joakim Tjernlund wrote:
> Secondly i2c, I have this in my platform driver:
> static struct i2c_board_info __initdata tmcu_i2c_devices[] = {
> 	{ I2C_BOARD_INFO("rtc-ds1307", 0x68),
> 	  .type = "ds1337",
> 	},
> };
> 
> int __init tmcu_i2c_board_devs(void)
> {
> 	int ret;
> 	ret = i2c_register_board_info(0, tmcu_i2c_devices,
> 				      ARRAY_SIZE(tmcu_i2c_devices));
> 	return ret;
> }
> arch_initcall(tmcu_i2c_board_devs);

i2c-mpc.c now registers as a dynamically-numbered bus; you need to either
use the device tree, or call i2c_new_device().

-Scott

Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Joakim Tjernlund]

On Mon, 2008-10-13 at 11:31 -0500, Scott Wood wrote:
> On Mon, Oct 13, 2008 at 02:01:55PM +0200, Joakim Tjernlund wrote:
> > Secondly i2c, I have this in my platform driver:
> > static struct i2c_board_info __initdata tmcu_i2c_devices[] = {
> > 	{ I2C_BOARD_INFO("rtc-ds1307", 0x68),
> > 	  .type = "ds1337",
> > 	},
> > };
> > 
> > int __init tmcu_i2c_board_devs(void)
> > {
> > 	int ret;
> > 	ret = i2c_register_board_info(0, tmcu_i2c_devices,
> > 				      ARRAY_SIZE(tmcu_i2c_devices));
> > 	return ret;
> > }
> > arch_initcall(tmcu_i2c_board_devs);
> 
> i2c-mpc.c now registers as a dynamically-numbered bus; you need to either
> use the device tree, or call i2c_new_device().
> 
> -Scott

Thanks, but is this really so? Reading include/linux/i2c.h makes me
wonder if not the i2c_register_board_info() should still work.
Calling i2c_new_device() from an initcall() procedure does not
seem easy/possible, you need the adapter to do that. Maybe I just
missing something obvious?

 Jocke

Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Scott Wood]

Joakim Tjernlund wrote:
> On Mon, 2008-10-13 at 11:31 -0500, Scott Wood wrote:
>> i2c-mpc.c now registers as a dynamically-numbered bus; you need to either
>> use the device tree, or call i2c_new_device().
> 
> Thanks, but is this really so? Reading include/linux/i2c.h makes me
> wonder if not the i2c_register_board_info() should still work.

Why?  Note that the "add-on" versus "mainboard" distinction in the 
comment is just an example; the actual distinction is 
dynamically-numbered versus statically-numbered.  Since the controller 
itself is probed from the device tree, there's no good way of statically 
assigning a bus number to it.

> Calling i2c_new_device() from an initcall() procedure does not
> seem easy/possible, you need the adapter to do that. Maybe I just
> missing something obvious?

The obvious and easy way is to just use the device tree.

If that isn't possible (such as due to device trees embedded in existing 
firmware), you can find the adapter device as a child of the of_device.

Finding the of_device from the device_node may be difficult, though.
We could have i2c-mpc set node->data to the of_device (or maybe the 
adapter struct), or maybe should add an of_device member of device_node 
that gets filled in by of_platform?

-Scott

RE: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Joakim Tjernlund]

> -----Original Message-----
> From: Scott Wood [mailto:scottwood@freescale.com]
> Sent: den 13 oktober 2008 20:31
> To: joakim.tjernlund@transmode.se
> Cc: 'linuxppc-dev Development'
> Subject: Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27
> 
> Joakim Tjernlund wrote:
> > On Mon, 2008-10-13 at 11:31 -0500, Scott Wood wrote:
> >> i2c-mpc.c now registers as a dynamically-numbered bus; you need to either
> >> use the device tree, or call i2c_new_device().
> >
> > Thanks, but is this really so? Reading include/linux/i2c.h makes me
> > wonder if not the i2c_register_board_info() should still work.
> 
> Why?  Note that the "add-on" versus "mainboard" distinction in the
> comment is just an example; the actual distinction is
> dynamically-numbered versus statically-numbered.  Since the controller
> itself is probed from the device tree, there's no good way of statically
> assigning a bus number to it.

Because all the kernel comments I can see still implies that this should work
and because this was the only way in earlier releases to add an i2c device.

> 
> > Calling i2c_new_device() from an initcall() procedure does not
> > seem easy/possible, you need the adapter to do that. Maybe I just
> > missing something obvious?
> 
> The obvious and easy way is to just use the device tree.
> 
> If that isn't possible (such as due to device trees embedded in existing
> firmware), you can find the adapter device as a child of the of_device.

Possible yes, safe no. As is now I cannot use 2.6.27 on my current u-boot. 
and that may cause trouble in the field.

I always had the impression that OF was an optional add on, but now it seems
that it is mandatory for i2c?
 
> Finding the of_device from the device_node may be difficult, though.
> We could have i2c-mpc set node->data to the of_device (or maybe the
> adapter struct), or maybe should add an of_device member of device_node
> that gets filled in by of_platform?

hmm, no easy way out then? I need a way to do this from my board code without
adding new stuff to the driver. A hack that only works in 2.6.27 will do.

 Jocke

Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Scott Wood]

Joakim Tjernlund wrote:
> Because all the kernel comments I can see still implies that this should work

Which kernel comments?

> and because this was the only way in earlier releases to add an i2c device.

We've supported enumerating i2c-mpc devices using the device tree for 
almost as long as there's been new-style i2c devices.

>> The obvious and easy way is to just use the device tree.
>>
>> If that isn't possible (such as due to device trees embedded in existing
>> firmware), you can find the adapter device as a child of the of_device.
> 
> Possible yes, safe no. As is now I cannot use 2.6.27 on my current u-boot. 

Why not?  U-boot allows you to pass in a device tree dynamically.

> I always had the impression that OF was an optional add on, but now it seems
> that it is mandatory for i2c?

It's mandatory for device drivers such as i2c-mpc that expect it.

>> Finding the of_device from the device_node may be difficult, though.
>> We could have i2c-mpc set node->data to the of_device (or maybe the
>> adapter struct), or maybe should add an of_device member of device_node
>> that gets filled in by of_platform?
> 
> hmm, no easy way out then? I need a way to do this from my board code without
> adding new stuff to the driver. A hack that only works in 2.6.27 will do.

You could add the node to the device tree from platform code, if you run 
before the i2c adapter's probe() runs.  Or, if you really want to stick 
with your current way of doing things, and are willing to own both 
pieces if it breaks, you can comment out this test in 
i2c_register_adapter, and always call i2c_scan_static_board_info:

if (adap->nr < __i2c_first_dynamic_bus_num)
	i2c_scan_static_board_info(adap);

-Scott

Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Joakim Tjernlund]

On Mon, 2008-10-13 at 14:03 -0500, Scott Wood wrote:
> Joakim Tjernlund wrote:
> > Because all the kernel comments I can see still implies that this should work
> 
> Which kernel comments?

The one already mentioned and in i2c-boardinfo.c

> 
> > and because this was the only way in earlier releases to add an i2c device.
> 
> We've supported enumerating i2c-mpc devices using the device tree for 
> almost as long as there's been new-style i2c devices.

Don't know how long that has been but looking at the mpc832x_mds dts
file it has been there less than a year.

> 
> >> The obvious and easy way is to just use the device tree.
> >>
> >> If that isn't possible (such as due to device trees embedded in existing
> >> firmware), you can find the adapter device as a child of the of_device.
> > 
> > Possible yes, safe no. As is now I cannot use 2.6.27 on my current u-boot. 
> 
> Why not?  U-boot allows you to pass in a device tree dynamically.

I don't use a dynamic device tree, mine is built in.

> 
> > I always had the impression that OF was an optional add on, but now it seems
> > that it is mandatory for i2c?
> 
> It's mandatory for device drivers such as i2c-mpc that expect it.

This is the main point, up to now I belived that OF only drivers was a
bug that should be fixed but since no one is supports my view I guess
I have misunderstod. 

> 
> >> Finding the of_device from the device_node may be difficult, though.
> >> We could have i2c-mpc set node->data to the of_device (or maybe the
> >> adapter struct), or maybe should add an of_device member of device_node
> >> that gets filled in by of_platform?
> > 
> > hmm, no easy way out then? I need a way to do this from my board code without
> > adding new stuff to the driver. A hack that only works in 2.6.27 will do.
> 
> You could add the node to the device tree from platform code, if you run 
> before the i2c adapter's probe() runs.  Or, if you really want to stick 
> with your current way of doing things, and are willing to own both 
> pieces if it breaks, you can comment out this test in 
> i2c_register_adapter, and always call i2c_scan_static_board_info:
> 
> if (adap->nr < __i2c_first_dynamic_bus_num)
> 	i2c_scan_static_board_info(adap);

No, I don't want to hack general i2c code. I guess I will
just have to update our boot too.

 Jocke

Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Scott Wood]

On Tue, Oct 14, 2008 at 09:55:31AM +0200, Joakim Tjernlund wrote:
> On Mon, 2008-10-13 at 14:03 -0500, Scott Wood wrote:
> > Joakim Tjernlund wrote:
> > > Because all the kernel comments I can see still implies that this should work
> > 
> > Which kernel comments?
> 
> The one already mentioned and in i2c-boardinfo.c

I already addressed the one already mentioned (it was sloppy terminology
in the comment, written from the perspective of devtree-less arches). 
The latter is even less clear, as it doesn't mention dynamic buses at
all.

Feel free to submit a documentation patch clearing it up. :-)

> > > and because this was the only way in earlier releases to add an i2c device.
> > 
> > We've supported enumerating i2c-mpc devices using the device tree for 
> > almost as long as there's been new-style i2c devices.
> 
> Don't know how long that has been but looking at the mpc832x_mds dts
> file it has been there less than a year.

commit d13ae8620dfdedfa7e9ab6d1eec294adc0516065 added i2c device probing
from the device tree in 7/21/07, less than 3 months after new-style i2c
devices were added to the i2c core.  There were patches floating around
the lists for longer than that.

> > Why not?  U-boot allows you to pass in a device tree dynamically.
> 
> I don't use a dynamic device tree, mine is built in.

So do a fixup in your board code.

I'm not sure how you would even go about building a device tree into
U-boot currently, though.

-Scott

Re: MPC8321, ethernet and i2c problems after upgrade from 2.6.25 to 2.6.27

[Joakim Tjernlund]

On Tue, 2008-10-14 at 10:27 -0500, Scott Wood wrote:
> On Tue, Oct 14, 2008 at 09:55:31AM +0200, Joakim Tjernlund wrote:
> > On Mon, 2008-10-13 at 14:03 -0500, Scott Wood wrote:
> > > Joakim Tjernlund wrote:
> > > > Because all the kernel comments I can see still implies that this should work
> > > 
> > > Which kernel comments?
> > 
> > The one already mentioned and in i2c-boardinfo.c
> 
> I already addressed the one already mentioned (it was sloppy terminology
> in the comment, written from the perspective of devtree-less arches). 
> The latter is even less clear, as it doesn't mention dynamic buses at
> all.
> 
> Feel free to submit a documentation patch clearing it up. :-)
> 
> > > > and because this was the only way in earlier releases to add an i2c device.
> > > 
> > > We've supported enumerating i2c-mpc devices using the device tree for 
> > > almost as long as there's been new-style i2c devices.
> > 
> > Don't know how long that has been but looking at the mpc832x_mds dts
> > file it has been there less than a year.
> 
> commit d13ae8620dfdedfa7e9ab6d1eec294adc0516065 added i2c device probing
> from the device tree in 7/21/07, less than 3 months after new-style i2c
> devices were added to the i2c core.  There were patches floating around
> the lists for longer than that.
> 
> > > Why not?  U-boot allows you to pass in a device tree dynamically.
> > 
> > I don't use a dynamic device tree, mine is built in.
> 
> So do a fixup in your board code.

Just did and this is what I came up with:
/* This should be removed once we have the rtc in u-boot's
   device tree */
static struct device_node *new_node(const char *path,
		struct device_node *parent)
{
	struct device_node *np = kzalloc(sizeof(*np), GFP_KERNEL);

	if (!np)
		return NULL;
	np->full_name = kmalloc(strlen(path) + 1, GFP_KERNEL);
	if (!np->full_name) {
		kfree(np);
		return NULL;
	}
	strcpy(np->full_name, path);
	of_node_set_flag(np, OF_DYNAMIC);
	kref_init(&np->kref);
	np->parent = of_node_get(parent);
	return np;
}

static struct property prop_compatible = {
	.name "compatible",
	.value = "dallas,ds1337",
	.length = sizeof("dallas,ds1337"),
};
static const unsigned long rtc_addr = 0x68;
static struct property prop_reg = {
	.name "reg",
	.value = &rtc_addr,
	.length = sizeof(rtc_addr),
};

int __init tmcu_i2c_board_devs(void)
{
	int ret=0;
	struct device_node *np, *i2c_np, *rtc_node=NULL;

	i2c_np = of_find_node_by_name(NULL, "i2c");
	if (!i2c_np)
		return -1;
	np = of_find_node_by_name(i2c_np, "rtc");
	if (!np) {
		rtc_node = new_node("/rtc", i2c_np);
		prom_add_property(rtc_node, &prop_compatible);
		prom_add_property(rtc_node, &prop_reg);
		of_attach_node(rtc_node);
	}
	return ret;
}
arch_initcall(tmcu_i2c_board_devs);

Wasn't easy to find out how, strangely I could not find a generic
new_node() function, had to copy one.

> 
> I'm not sure how you would even go about building a device tree into
> U-boot currently, though.

That is easy :)