Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on FreescaleCPM1/CPM2 controllers

From: Stephen Rothwell
Date: Tue Mar 25 2008 - 20:42:00 EST


Hi Jochen,

Firstly, you should probably cc Dave Miller <davem@xxxxxxxxxxxxx> 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@xxxxxxxx> wrote:
>
> +++ b/drivers/of/i2c.c
> @@ -0,0 +1,113 @@
> +/*
> + * OF helpers for the I2C API
> + *
> + * Copyright (c) 2008 Jochen Friedrich <jochen@xxxxxxxx>
> + *
> + * Based on a previous patch from Jon Smirl <jonsmirl@xxxxxxxxx>
> + *
> + * 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.

> +struct i2c_driver_device {
> + char *of_device;
> + char *i2c_type;
> +};
> +
> +static struct i2c_driver_device i2c_devices[] = {
> + {"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.

> +void of_register_i2c_devices(struct i2c_adapter *adap,
> + struct device_node *adap_node)
> +{
> + void *result;
> + struct device_node *node = NULL;
> +
> + while ((node = 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.

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

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

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

Attachment: pgp00000.pgp
Description: PGP signature