Hi Jochen,

Firstly, you should probably cc Dave Miller <davem@davemloft.net> on
anything in drivers/of as Sparc is the other user of this stuff (just in
case they are interested).

On Tue, 25 Mar 2008 19:46:41 +0100 Jochen Friedrich <jochen@scram.de> wrote:
quoted text
>
> +++ b/drivers/of/i2c.c
> @@ -0,0 +1,113 @@
> +/*
> + * OF helpers for the I2C API
> + *
> + * Copyright (c) 2008 Jochen Friedrich <jochen@scram.de>
> + *
> + * Based on a previous patch from Jon Smirl <jonsmirl@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/i2c.h>
> +#include <asm/prom.h>

You should really include <linux/of.h> instead.

quoted text
> +struct i2c_driver_device {
> +	char    *of_device;
> +	char    *i2c_type;
> +};
> +
> +static struct i2c_driver_device i2c_devices[] =3D {
> +	{"dallas,ds1374", "rtc-ds1374",},
                                      ^
You don't need the comma before the brace (unless you are anticipating
that struct i2c_driver_device will change.  Even then, its not a big
thing.

quoted text
> +void of_register_i2c_devices(struct i2c_adapter *adap,
> +			     struct device_node *adap_node)
> +{
> +	void *result;
> +	struct device_node *node =3D NULL;
> +
> +	while ((node =3D of_get_next_child(adap_node, node))) {

	for_each_child_of_node(adap_node, node) {
And then you don't need to initialise "node" above.

quoted text
> +		info.irq =3D irq_of_parse_and_map(node, 0);
> +		if (info.irq =3D=3D NO_IRQ)
> +			info.irq =3D -1;
> +
> +		if (of_find_i2c_driver(node, &info) < 0)
> +			continue;

Do you need to clean up after the irq_of_parse_and_map() above?

--=20
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/