[PATCH 0/2] Two I2C Chips drivers from Blackfin team

[PATCH 0/2] Two I2C Chips drivers from Blackfin team

[Bryan Wu]

Sorry for resending this 2 patches, because I delete that useless I2C ID
patch from my previous patch set.

Could you please review these patches?

Thanks
-Bryan



_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCH 1/2] [I2C]: add AD5252 chip driver

[Bryan Wu]

From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bryan Wu <cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/i2c/chips/Kconfig  |    7 ++
 drivers/i2c/chips/Makefile |    1 +
 drivers/i2c/chips/ad5252.c |  252 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c-id.h     |    1 +
 4 files changed, 261 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/chips/ad5252.c

diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
index 2da2edf..1222e36 100644
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -14,6 +14,13 @@ config DS1682
 	  This driver can also be built as a module.  If so, the module
 	  will be called ds1682.
 
+config SENSORS_AD5252
+	tristate "Analog Devices AD5252 and AD5251 Digital Potentiometers"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for Analog Devices and AD5252
+	  AD5251 chips.
+
 config SENSORS_EEPROM
 	tristate "EEPROM reader"
 	depends on EXPERIMENTAL
diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
index e47aca0..866def2 100644
--- a/drivers/i2c/chips/Makefile
+++ b/drivers/i2c/chips/Makefile
@@ -9,6 +9,7 @@
 # * I/O expander drivers go to drivers/gpio
 #
 
+obj-$(CONFIG_SENSORS_AD5252)	+= ad5252.o
 obj-$(CONFIG_DS1682)		+= ds1682.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
 obj-$(CONFIG_SENSORS_MAX6875)	+= max6875.o
diff --git a/drivers/i2c/chips/ad5252.c b/drivers/i2c/chips/ad5252.c
new file mode 100644
index 0000000..188795a
--- /dev/null
+++ b/drivers/i2c/chips/ad5252.c
@@ -0,0 +1,252 @@
+/*
+    ad5252.c - Part of lm_sensors, Linux kernel modules for hardware
+             monitoring
+
+    Driver for dual digitally controlled potentiometers
+
+    Copyright (c) 2006 Michael Hennerich <hennerich-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org>
+
+    derived from pcf8547.c
+
+    Copyright (c) 2000  Frodo Looijaard <frodol-B0qZmFHriGg@public.gmane.org>,
+                        Philip Edelbrock <phil-KXOFo5pg7o1l57MIdRCFDg@public.gmane.org>,
+                        Dan Eaton <dan.eaton-RtyX3GHucphnQYSypqKXDg@public.gmane.org>
+    Ported to Linux 2.6 by Aurelien Jarno <aurel32-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> with
+    the help of Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
+
+
+    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.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = { 0x2c, 0x2d, 0x2e, 0x2f, I2C_CLIENT_END };
+
+/* Insmod parameters */
+I2C_CLIENT_INSMOD_1(ad5252);
+
+/* Initial values */
+#define AD5252_INIT 128	/* Wiper in middle position */
+/* Each client has this additional data */
+struct ad5252_data {
+	struct i2c_client client;
+
+	u8 read, write;			/* Register values */
+};
+
+static int ad5252_attach_adapter(struct i2c_adapter *adapter);
+static int ad5252_detect(struct i2c_adapter *adapter, int address, int kind);
+static int ad5252_detach_client(struct i2c_client *client);
+static void ad5252_init_client(struct i2c_client *client);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver ad5252_driver = {
+	.driver = {
+	.name		= "ad5252",
+	},
+	.id		= I2C_DRIVERID_AD5252,
+	.attach_adapter	= ad5252_attach_adapter,
+	.detach_client	= ad5252_detach_client,
+};
+
+/* following are the sysfs callback functions */
+static ssize_t show_read_w1(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	return sprintf(buf, "%u\n", i2c_smbus_read_byte_data(client,0x1));
+}
+
+static DEVICE_ATTR(read_w1, S_IRUGO, show_read_w1, NULL);
+
+static ssize_t show_read_w3(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	return sprintf(buf, "%u\n", i2c_smbus_read_byte_data(client,0x3));
+}
+
+static DEVICE_ATTR(read_w3, S_IRUGO, show_read_w3, NULL);
+
+static ssize_t show_write(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct ad5252_data *data = i2c_get_clientdata(to_i2c_client(dev));
+	return sprintf(buf, "%u\n", data->write);
+}
+
+static ssize_t set_write_w1(struct device *dev, struct device_attribute *attr, const char *buf,
+			size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ad5252_data *data = i2c_get_clientdata(client);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
+
+	if (val > 0xff)
+		return -EINVAL;
+
+	data->write = val;
+
+	i2c_smbus_write_byte_data(client,0x1, data->write);
+	return count;
+}
+
+static DEVICE_ATTR(write_w1, S_IWUSR | S_IRUGO, show_write, set_write_w1);
+
+static ssize_t set_write_w3(struct device *dev, struct device_attribute *attr, const char *buf,
+			 size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ad5252_data *data = i2c_get_clientdata(client);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
+
+	if (val > 0xff)
+		return -EINVAL;
+
+	data->write = val;
+
+	i2c_smbus_write_byte_data(client,0x3, data->write);
+	return count;
+}
+
+static DEVICE_ATTR(write_w3, S_IWUSR | S_IRUGO, show_write, set_write_w3);
+
+/*
+ * Real code
+ */
+
+static int ad5252_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_probe(adapter, &addr_data, ad5252_detect);
+}
+
+/* This function is called by i2c_detect */
+int ad5252_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct ad5252_data *data;
+	int err = 0;
+	const char *client_name = "";
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
+		goto exit;
+
+	/* OK. For now, we presume we have a valid client. We now create the
+	   client structure, even though we cannot fill it completely yet. */
+	if (!(data = kzalloc(sizeof(struct ad5252_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &ad5252_driver;
+	new_client->flags = 0;
+
+	/* Determine the chip type */
+	if (kind <= 0) {
+		if (address >= 0x2c && address <= 0x2f)
+			kind = ad5252;
+
+	}
+
+	client_name = "ad5252";
+
+	/* Fill in the remaining client fields and put it into the global list */
+	strlcpy(new_client->name, client_name, I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+
+	/*FIXME: Don't know why there needs to be a delay !!!*/
+	udelay(100);
+
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_free;
+
+	/* Initialize the AD5252 chip */
+	ad5252_init_client(new_client);
+
+	/* Register sysfs hooks */
+	err = device_create_file(&new_client->dev, &dev_attr_read_w1);
+	err |= device_create_file(&new_client->dev, &dev_attr_read_w3);
+	err |= device_create_file(&new_client->dev, &dev_attr_write_w1);
+	err |= device_create_file(&new_client->dev, &dev_attr_write_w3);
+
+	if (err)
+		printk(KERN_WARNING "AD5252: failed to register sysfs hooks\n");
+
+	printk(KERN_INFO "AD5252 Attached\n");
+	return 0;
+
+/* OK, this is not exactly good programming practice, usually. But it is
+   very code-efficient in this case. */
+
+      exit_free:
+	kfree(data);
+      exit:
+	printk(KERN_INFO "AD5252 attaching failed\n");
+	return err;
+}
+
+static int ad5252_detach_client(struct i2c_client *client)
+{
+	int err;
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev,
+			"Client deregistration failed, client not detached.\n");
+		return err;
+	}
+
+	kfree(i2c_get_clientdata(client));
+	return 0;
+}
+
+/* Called when we have found a new AD5252. */
+static void ad5252_init_client(struct i2c_client *client)
+{
+	//struct ad5252_data *data = i2c_get_clientdata(client);
+	//data->write = AD5252_INIT;
+	//i2c_smbus_write_byte_data(client,0x1, data->write);
+	//i2c_smbus_write_byte_data(client,0x3, data->write);
+}
+
+static int __init ad5252_init(void)
+{
+	return i2c_add_driver(&ad5252_driver);
+}
+
+static void __exit ad5252_exit(void)
+{
+	i2c_del_driver(&ad5252_driver);
+}
+
+
+MODULE_AUTHOR
+    ("Michael Hennerich <hennerich-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org>, "
+     "Frodo Looijaard <frodol-B0qZmFHriGg@public.gmane.org>, "
+     "Philip Edelbrock <phil-KXOFo5pg7o1l57MIdRCFDg@public.gmane.org>, "
+     "Dan Eaton <dan.eaton-RtyX3GHucphnQYSypqKXDg@public.gmane.org> "
+     "and Aurelien Jarno <aurelien-rXXEIb44qovR7s880joybQ@public.gmane.org>");
+MODULE_DESCRIPTION("AD5252 digital potentiometer driver");
+MODULE_LICENSE("GPL");
+
+module_init(ad5252_init);
+module_exit(ad5252_exit);
diff --git a/include/linux/i2c-id.h b/include/linux/i2c-id.h
index 580acc9..adfcf71 100644
--- a/include/linux/i2c-id.h
+++ b/include/linux/i2c-id.h
@@ -94,6 +94,7 @@
 #define I2C_DRIVERID_I2CDEV	900
 
 #define I2C_DRIVERID_OV7670 1048	/* Omnivision 7670 camera */
+#define I2C_DRIVERID_AD5252 1049
 
 /*
  * ---- Adapter types ----------------------------------------------------
-- 
1.5.5


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

[PATCH 2/2] [I2C] add PCA9543 chip driver

[Bryan Wu]

From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Bryan Wu <cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/i2c/chips/Kconfig   |   13 +++
 drivers/i2c/chips/Makefile  |    1 +
 drivers/i2c/chips/pca9543.c |  212 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c-id.h      |    1 +
 4 files changed, 227 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/chips/pca9543.c

diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
index 1222e36..908fe9a 100644
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -60,6 +60,19 @@ config PCF8575
 	  This device is hard to detect and is rarely found on mainstream
 	  hardware.  If unsure, say N.
 
+config SENSORS_PCA9543
+	tristate "Philips PCA9543 bi-directional translating switch"
+	depends on I2C && EXPERIMENTAL
+	default n
+	help
+	  If you say yes here you get support for Philips PCA9543.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called pca9543.
+
+	  These devices are hard to detect and rarely found on mainstream
+	  hardware.  If unsure, say N.
+
 config SENSORS_PCA9539
 	tristate "Philips PCA9539 16-bit I/O port (DEPRECATED)"
 	depends on EXPERIMENTAL && GPIO_PCA9539 = "n"
diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
index 866def2..7b292e6 100644
--- a/drivers/i2c/chips/Makefile
+++ b/drivers/i2c/chips/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_DS1682)		+= ds1682.o
 obj-$(CONFIG_SENSORS_EEPROM)	+= eeprom.o
 obj-$(CONFIG_SENSORS_MAX6875)	+= max6875.o
 obj-$(CONFIG_SENSORS_PCA9539)	+= pca9539.o
+obj-$(CONFIG_SENSORS_PCA9543)	+= pca9543.o
 obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_PCF8575)		+= pcf8575.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
diff --git a/drivers/i2c/chips/pca9543.c b/drivers/i2c/chips/pca9543.c
new file mode 100644
index 0000000..dc19638
--- /dev/null
+++ b/drivers/i2c/chips/pca9543.c
@@ -0,0 +1,212 @@
+/*
+    pca9543.c - Part of lm_sensors, Linux kernel modules for hardware
+             monitoring
+    Copyright (c) 2000  Frodo Looijaard <frodol-B0qZmFHriGg@public.gmane.org>,
+                        Philip Edelbrock <phil-KXOFo5pg7o1l57MIdRCFDg@public.gmane.org>,
+                        Dan Eaton <dan.eaton-RtyX3GHucphnQYSypqKXDg@public.gmane.org>
+    Ported to Linux 2.6 by Aurelien Jarno <aurel32-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> with
+    the help of Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
+
+    Copyright (C) 2007 Michael Hennerich, Analog Devices Inc.
+    			<hennerich-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org>
+	based on the pcf8574.c
+
+    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.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+/* A few notes about the PCA9543:
+
+*   The PCA9543 is a bi-directional translating switch, controlled by the
+    I2C bus. The SCL/SDA upstream pair fans out to two downstream
+    pairs, or channels. Any individual SCx/SDx channels or combination
+    of channels can be selected, determined by the contents of the
+    programmable control register. Two interrupt inputs, INT0 to INT3,
+    one for each of the downstream pairs, are provided. One interrupt
+    output INT, which acts as an AND of the two interrupt inputs, is
+    provided.
+
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = { 0x70,0x71, 0x72, 0x73, I2C_CLIENT_END };
+
+/* Insmod parameters */
+I2C_CLIENT_INSMOD_1(pca9543);
+
+
+/* Each client has this additional data */
+struct pca9543_data {
+	struct i2c_client client;
+
+	u8 write;			/* Remember last written value */
+};
+
+static int pca9543_attach_adapter(struct i2c_adapter *adapter);
+static int pca9543_detect(struct i2c_adapter *adapter, int address, int kind);
+static int pca9543_detach_client(struct i2c_client *client);
+
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pca9543_driver = {
+	.driver = {
+		.name	= "pca9543",
+	},
+	.id		= I2C_DRIVERID_PCF8574,
+	.attach_adapter	= pca9543_attach_adapter,
+	.detach_client	= pca9543_detach_client,
+};
+
+/* following are the sysfs callback functions */
+static ssize_t show_read(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	return sprintf(buf, "%u\n", i2c_smbus_read_byte(client));
+}
+
+static DEVICE_ATTR(read, S_IRUGO, show_read, NULL);
+
+static ssize_t show_write(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct pca9543_data *data = i2c_get_clientdata(to_i2c_client(dev));
+	return sprintf(buf, "%u\n", data->write);
+}
+
+static ssize_t set_write(struct device *dev, struct device_attribute *attr, const char *buf,
+			 size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct pca9543_data *data = i2c_get_clientdata(client);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
+
+	if (val > 0xff)
+		return -EINVAL;
+
+	data->write = val;
+	i2c_smbus_write_byte(client, data->write);
+	return count;
+}
+
+static DEVICE_ATTR(write, S_IWUSR | S_IRUGO, show_write, set_write);
+
+static struct attribute *pca9543_attributes[] = {
+	&dev_attr_read.attr,
+	&dev_attr_write.attr,
+	NULL
+};
+
+static const struct attribute_group pca9543_attr_group = {
+	.attrs = pca9543_attributes,
+};
+
+/*
+ * Real code
+ */
+
+static int pca9543_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_probe(adapter, &addr_data, pca9543_detect);
+}
+
+/* This function is called by i2c_probe */
+static int pca9543_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct pca9543_data *data;
+	int err = 0;
+	const char *client_name = "pca9543";
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
+		goto exit;
+
+	/* OK. For now, we presume we have a valid client. We now create the
+	   client structure, even though we cannot fill it completely yet. */
+	if (!(data = kzalloc(sizeof(struct pca9543_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &pca9543_driver;
+	new_client->flags = 0;
+
+	/* Now, we would do the remaining detection. But the PCF8574 is plainly
+	   impossible to detect! Stupid chip. */
+
+	/* Fill in the remaining client fields and put it into the global list */
+	strlcpy(new_client->name, client_name, I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_free;
+
+
+	/* Register sysfs hooks */
+	err = sysfs_create_group(&new_client->dev.kobj, &pca9543_attr_group);
+	if (err)
+		goto exit_detach;
+	return 0;
+
+      exit_detach:
+	i2c_detach_client(new_client);
+      exit_free:
+	kfree(data);
+      exit:
+	return err;
+}
+
+static int pca9543_detach_client(struct i2c_client *client)
+{
+	int err;
+
+	sysfs_remove_group(&client->dev.kobj, &pca9543_attr_group);
+
+	if ((err = i2c_detach_client(client)))
+		return err;
+
+	kfree(i2c_get_clientdata(client));
+	return 0;
+}
+
+
+static int __init pca9543_init(void)
+{
+	return i2c_add_driver(&pca9543_driver);
+}
+
+static void __exit pca9543_exit(void)
+{
+	i2c_del_driver(&pca9543_driver);
+}
+
+
+MODULE_AUTHOR
+    ("Frodo Looijaard <frodol-B0qZmFHriGg@public.gmane.org>, "
+     "Philip Edelbrock <phil-KXOFo5pg7o1l57MIdRCFDg@public.gmane.org>, "
+     "Dan Eaton <dan.eaton-RtyX3GHucphnQYSypqKXDg@public.gmane.org> "
+     "and Aurelien Jarno <aurelien-rXXEIb44qovR7s880joybQ@public.gmane.org>"
+     "Michael Hennerich <hennerich-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org>");
+MODULE_DESCRIPTION("PCA9543 driver");
+MODULE_LICENSE("GPL");
+
+module_init(pca9543_init);
+module_exit(pca9543_exit);
diff --git a/include/linux/i2c-id.h b/include/linux/i2c-id.h
index adfcf71..72c246a 100644
--- a/include/linux/i2c-id.h
+++ b/include/linux/i2c-id.h
@@ -95,6 +95,7 @@
 
 #define I2C_DRIVERID_OV7670 1048	/* Omnivision 7670 camera */
 #define I2C_DRIVERID_AD5252 1049
+#define I2C_DRIVERID_PCF8575 1050
 
 /*
  * ---- Adapter types ----------------------------------------------------
-- 
1.5.5


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 0/2] Two I2C Chips drivers from Blackfin team

[Jean Delvare]

Hi Bryan,

On Mon, 12 May 2008 18:07:25 +0800, Bryan Wu wrote:
> Sorry for resending this 2 patches, because I delete that useless I2C ID
> patch from my previous patch set.

Resending patches if fine, no problem with this.

> Could you please review these patches?

I'm awfully busy these days, and I already have several patch reviews
pending, so it is unlikely that I will have the time to review these
patches in a near future, sorry.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 0/2] Two I2C Chips drivers from Blackfin team

[Bryan Wu]

On Mon, May 12, 2008 at 7:34 PM, Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> Hi Bryan,
>
>
>  On Mon, 12 May 2008 18:07:25 +0800, Bryan Wu wrote:
>  > Sorry for resending this 2 patches, because I delete that useless I2C ID
>  > patch from my previous patch set.
>
>  Resending patches if fine, no problem with this.
>
>
>  > Could you please review these patches?
>
>  I'm awfully busy these days, and I already have several patch reviews
>  pending, so it is unlikely that I will have the time to review these
>  patches in a near future, sorry.
>

Understand, would you please add these patches into you review pending queue.
When you got time, please help us to review these. IMO, these drive
may be helpful for
other developers.

Thanks
-Bryan Wu

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 2/2] [I2C] add PCA9543 chip driver

[Riku Voipio]

Bryan Wu wrote:
> From: Michael Hennerich 
> --- /dev/null
> +++ b/drivers/i2c/chips/pca9543.c
> @@ -0,0 +1,212 @@
> +/*
> +    pca9543.c - Part of lm_sensors, Linux kernel modules for hardware
> +             monitoring
>   
Not a sensor chip..
> +/* A few notes about the PCA9543:
> +
> +*   The PCA9543 is a bi-directional translating switch, controlled by the
> +    I2C bus. The SCL/SDA upstream pair fans out to two downstream
> +    pairs, or channels. Any individual SCx/SDx channels or combination
> +    of channels can be selected, determined by the contents of the
> +    programmable control register. Two interrupt inputs, INT0 to INT3,
> +    one for each of the downstream pairs, are provided. One interrupt
> +    output INT, which acts as an AND of the two interrupt inputs, is
> +    provided.
> +
> +*/
>   
You might want to add a url to the datasheet, rather than
a copypaste from it. Any description in the driver sould
> +
> +/* This is the driver that will be inserted */
> +static struct i2c_driver pca9543_driver = {
> +	.driver = {
> +		.name	= "pca9543",
> +	},
> +	.id		= I2C_DRIVERID_PCF8574,
> +	.attach_adapter	= pca9543_attach_adapter,
> +	.detach_client	= pca9543_detach_client,
> +};
>   
Since the chip is undetectable, and used on embedded platform, I think you
should consider a "new-style" driver with .probe/remove instead.
See i2c/writing-clients
> +	struct i2c_client *new_client;
>
> +	new_client = &data->client;
> +	i2c_set_clientdata(new_client, data);
> +	new_client->addr = address;
> +	new_client->adapter = adapter;
> +	new_client->driver = &pca9543_driver;
> +	new_client->flags = 0;
>
>   
s/new_client/client/ please.
> +	/* Now, we would do the remaining detection. But the PCF8574 is plainly
> +	   impossible to detect! Stupid chip. */
> +
>   
copypaste from PCF8574 driver? :) Again, consider if the new style drivers
would me more useful for your platform.



_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 2/2] [I2C] add PCA9543 chip driver

[Hennerich, Michael]

>-----Original Message-----
>From: Riku Voipio [mailto:riku.voipio-WgUW+8SLYMv1KXRcyAk9cg@public.gmane.org]
>Sent: Montag, 12. Mai 2008 14:31
>To: Bryan Wu
>Cc: i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org; khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org; Michael Hennerich
>Subject: Re: [i2c] [PATCH 2/2] [I2C] add PCA9543 chip driver
>
>Bryan Wu wrote:
>> From: Michael Hennerich
>> --- /dev/null
>> +++ b/drivers/i2c/chips/pca9543.c
>> @@ -0,0 +1,212 @@
>> +/*
>> +    pca9543.c - Part of lm_sensors, Linux kernel modules for
hardware
>> +             monitoring
>>
>Not a sensor chip..
>> +/* A few notes about the PCA9543:
>> +
>> +*   The PCA9543 is a bi-directional translating switch, controlled
by
>the
>> +    I2C bus. The SCL/SDA upstream pair fans out to two downstream
>> +    pairs, or channels. Any individual SCx/SDx channels or
combination
>> +    of channels can be selected, determined by the contents of the
>> +    programmable control register. Two interrupt inputs, INT0 to
INT3,
>> +    one for each of the downstream pairs, are provided. One
interrupt
>> +    output INT, which acts as an AND of the two interrupt inputs, is
>> +    provided.
>> +
>> +*/
>>
>You might want to add a url to the datasheet, rather than
>a copypaste from it. Any description in the driver sould
>> +
>> +/* This is the driver that will be inserted */
>> +static struct i2c_driver pca9543_driver = {
>> +	.driver = {
>> +		.name	= "pca9543",
>> +	},
>> +	.id		= I2C_DRIVERID_PCF8574,
>> +	.attach_adapter	= pca9543_attach_adapter,
>> +	.detach_client	= pca9543_detach_client,
>> +};
>>
>Since the chip is undetectable, and used on embedded platform, I think
you
>should consider a "new-style" driver with .probe/remove instead.
>See i2c/writing-clients
>> +	struct i2c_client *new_client;
>>
>> +	new_client = &data->client;
>> +	i2c_set_clientdata(new_client, data);
>> +	new_client->addr = address;
>> +	new_client->adapter = adapter;
>> +	new_client->driver = &pca9543_driver;
>> +	new_client->flags = 0;
>>
>>
>s/new_client/client/ please.
>> +	/* Now, we would do the remaining detection. But the PCF8574 is
>plainly
>> +	   impossible to detect! Stupid chip. */
>> +
>>
>copypaste from PCF8574 driver? :) Again, consider if the new style
drivers
>would me more useful for your platform.

Yes copypaste from PCF8574 driver. There is not much value in here, we
either send a new patch including your feedback or drop this. 

Best regards,
Michael

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 2/2] [I2C] add PCA9543 chip driver

[Bryan Wu]

On Tue, May 13, 2008 at 7:54 PM, Hennerich, Michael
<Michael.Hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org> wrote:
>
>
>
>  >-----Original Message-----
>  >From: Riku Voipio [mailto:riku.voipio-WgUW+8SLYMv1KXRcyAk9cg@public.gmane.org]
>  >Sent: Montag, 12. Mai 2008 14:31
>  >To: Bryan Wu
>  >Cc: i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org; khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org; Michael Hennerich
>  >Subject: Re: [i2c] [PATCH 2/2] [I2C] add PCA9543 chip driver
>  >
>  >Bryan Wu wrote:
>  >> From: Michael Hennerich
>  >> --- /dev/null
>  >> +++ b/drivers/i2c/chips/pca9543.c
>  >> @@ -0,0 +1,212 @@
>  >> +/*
>  >> +    pca9543.c - Part of lm_sensors, Linux kernel modules for
>  hardware
>  >> +             monitoring
>  >>
>  >Not a sensor chip..
>  >> +/* A few notes about the PCA9543:
>  >> +
>  >> +*   The PCA9543 is a bi-directional translating switch, controlled
>  by
>  >the
>  >> +    I2C bus. The SCL/SDA upstream pair fans out to two downstream
>  >> +    pairs, or channels. Any individual SCx/SDx channels or
>  combination
>  >> +    of channels can be selected, determined by the contents of the
>  >> +    programmable control register. Two interrupt inputs, INT0 to
>  INT3,
>  >> +    one for each of the downstream pairs, are provided. One
>  interrupt
>  >> +    output INT, which acts as an AND of the two interrupt inputs, is
>  >> +    provided.
>  >> +
>  >> +*/
>  >>
>  >You might want to add a url to the datasheet, rather than
>  >a copypaste from it. Any description in the driver sould
>  >> +
>  >> +/* This is the driver that will be inserted */
>  >> +static struct i2c_driver pca9543_driver = {
>  >> +    .driver = {
>  >> +            .name   = "pca9543",
>  >> +    },
>  >> +    .id             = I2C_DRIVERID_PCF8574,
>  >> +    .attach_adapter = pca9543_attach_adapter,
>  >> +    .detach_client  = pca9543_detach_client,
>  >> +};
>  >>
>  >Since the chip is undetectable, and used on embedded platform, I think
>  you
>  >should consider a "new-style" driver with .probe/remove instead.
>  >See i2c/writing-clients
>  >> +    struct i2c_client *new_client;
>  >>
>  >> +    new_client = &data->client;
>  >> +    i2c_set_clientdata(new_client, data);
>  >> +    new_client->addr = address;
>  >> +    new_client->adapter = adapter;
>  >> +    new_client->driver = &pca9543_driver;
>  >> +    new_client->flags = 0;
>  >>
>  >>
>  >s/new_client/client/ please.
>  >> +    /* Now, we would do the remaining detection. But the PCF8574 is
>  >plainly
>  >> +       impossible to detect! Stupid chip. */
>  >> +
>  >>
>  >copypaste from PCF8574 driver? :) Again, consider if the new style
>  drivers
>  >would me more useful for your platform.
>
>  Yes copypaste from PCF8574 driver. There is not much value in here, we
>  either send a new patch including your feedback or drop this.
>

Right, as Michael said, I dropped this patch from my guilt repo.

Thanks
-Bryan

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c