linux-kernel - Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2 controllers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20080326114134.495f5fe5.sfr@canb.auug.org.au>
Date:	Wed, 26 Mar 2008 11:41:34 +1100
From:	Stephen Rothwell <sfr@...b.auug.org.au>
To:	Jochen Friedrich <jochen@...am.de>
Cc:	Jean Delvare <khali@...ux-fr.org>, linux-kernel@...r.kernel.org,
	linuxppc-dev list <linuxppc-dev@...abs.org>,
	i2c@...sensors.org, Scott Wood <scottwood@...escale.com>
Subject: Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale
 CPM1/CPM2 controllers

Hi Jochen,

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

> +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@...b.auug.org.au
http://www.canb.auug.org.au/~sfr/

Content of type "application/pgp-signature" skipped