[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[BGardner]

This is a driver for the PCA9539, a 16 bit digital I/O chip.
It uses the new i2c-sysfs interfaces.

Signed-off-by: Ben Gardner <bgardner@wabtec.com>

--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c
1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c
2005-06-03 15:15:02.000000000 -0500
@@ -0,0 +1,188 @@
+/*
+    pca9539.c - 16 port digital I/O with interrupt and reset
+    
+    Copyright (C) 2005 Ben Gardner <bgardner@wabtec.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; version 2 of the License.
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sysfs.h>
+#include <linux/i2c-sensor.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = { 0x74, 0x75, 0x76, 0x77,
I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+/* Insmod parameters */
+SENSORS_INSMOD_1(pca9539);
+
+enum pca9539_cmd
+{
+	PCA9539_INPUT_0 = 0,
+	PCA9539_INPUT_1 = 1,
+	PCA9539_OUTPUT_0 = 2,
+	PCA9539_OUTPUT_1 = 3,
+	PCA9539_INVERT_0 = 4,
+	PCA9539_INVERT_1 = 5,
+	PCA9539_CONFIG_0 = 6,
+	PCA9539_CONFIG_1 = 7
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter);
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int
kind);
+static int pca9539_detach_client(struct i2c_client *client);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pca9539_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "pca9539",
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= pca9539_attach_adapter,
+	.detach_client	= pca9539_detach_client,
+};
+
+struct pca9539_data {
+	struct i2c_client client;
+};
+
+/* following are the sysfs callback functions */
+static ssize_t pca9539_show(struct device *dev, struct device_attribute
*attr,
+			    char *buf)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct pca9539_data *data i2c_get_clientdata(to_i2c_client(dev));
+	return sprintf(buf, "%d\n",
i2c_smbus_read_byte_data(&data->client, (u8)psa->index));
+}
+
+static ssize_t pca9539_store(struct device *dev, struct
device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct pca9539_data *data i2c_get_clientdata(to_i2c_client(dev));
+	u8 val = simple_strtoul(buf, NULL, 0);
+	i2c_smbus_write_byte_data(&data->client, (u8)psa->index, val);
+	return count;
+}
+
+/* Define the device attributes */
+
+#define PCA9539_ENTRY_RO(name,enum_name)	\
+	static
SENSOR_DEVICE_ATTR(name,S_IRUGO,pca9539_show,NULL,enum_name)
+
+#define PCA9539_ENTRY_RW(name,enum_name)	\
+	static
SENSOR_DEVICE_ATTR(name,S_IRUGO|S_IWUSR,pca9539_show,pca9539_store,enum_
name)
+
+PCA9539_ENTRY_RO(input0,  PCA9539_INPUT_0);
+PCA9539_ENTRY_RO(input1,  PCA9539_INPUT_1);
+PCA9539_ENTRY_RW(output0, PCA9539_OUTPUT_0);
+PCA9539_ENTRY_RW(output1, PCA9539_OUTPUT_1);
+PCA9539_ENTRY_RW(invert0, PCA9539_INVERT_0);
+PCA9539_ENTRY_RW(invert1, PCA9539_INVERT_1);
+PCA9539_ENTRY_RW(config0, PCA9539_CONFIG_0);
+PCA9539_ENTRY_RW(config1, PCA9539_CONFIG_1);
+
+static struct attribute *pca9539_attributes[] = {
+	&sensor_dev_attr_input0.dev_attr.attr,
+	&sensor_dev_attr_input1.dev_attr.attr,
+	&sensor_dev_attr_output0.dev_attr.attr,
+	&sensor_dev_attr_output1.dev_attr.attr,
+	&sensor_dev_attr_invert0.dev_attr.attr,
+	&sensor_dev_attr_invert1.dev_attr.attr,
+	&sensor_dev_attr_config0.dev_attr.attr,
+	&sensor_dev_attr_config1.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group pca9539_defattr_group = {
+	.attrs = pca9539_attributes,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_detect(adapter, &addr_data, pca9539_detect);
+}
+
+/* This function is called by i2c_detect */
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int
kind)
+{
+	struct i2c_client *new_client;
+	struct pca9539_data *data;
+	int err = 0;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE
+				     | I2C_FUNC_SMBUS_WRITE_BYTE_DATA
+				     | I2C_FUNC_SMBUS_READ_BYTE_DATA))
+		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 = kmalloc(sizeof(struct pca9539_data), GFP_KERNEL)))
{
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct pca9539_data));
+	
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &pca9539_driver;
+	new_client->flags = 0;
+
+	/* Detection: not sure how to do that... 
+	 * possibility: toggle the value read from invert0 and see if
input0
+	 * changes polarity. 
+	 * However, if the inputs are changing, this won't work.
+	 */
+	strlcpy(new_client->name, "pca9539", I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_kfree;
+
+	/* Register sysfs hooks */
+	return sysfs_create_group(&new_client->dev.kobj,
&pca9539_defattr_group);
+
+exit_kfree:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int pca9539_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;
+}
+
+static int __init pca9539_init(void)
+{
+	return i2c_add_driver(&pca9539_driver);
+}
+
+static void __exit pca9539_exit(void)
+{
+	i2c_del_driver(&pca9539_driver);
+}
+
+MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
+MODULE_DESCRIPTION("PCA9539 driver");
+MODULE_LICENSE("GPL");
+
+module_init(pca9539_init);
+module_exit(pca9539_exit);
+

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier (retry)

[BGardner]

Please ignore the previous patch -- I had left out the Makefile and
Kconfig changes.

This is a driver for the PCA9539, a 16 bit digital I/O chip.
It uses the new i2c-sysfs interfaces.

Signed-off-by: Ben Gardner <bgardner@wabtec.com>

--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c
1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c
2005-06-03 15:15:02.000000000 -0500
@@ -0,0 +1,188 @@
+/*
+    pca9539.c - 16 port digital I/O with interrupt and reset
+    
+    Copyright (C) 2005 Ben Gardner <bgardner@wabtec.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; version 2 of the License.
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sysfs.h>
+#include <linux/i2c-sensor.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = { 0x74, 0x75, 0x76, 0x77,
I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+/* Insmod parameters */
+SENSORS_INSMOD_1(pca9539);
+
+enum pca9539_cmd
+{
+	PCA9539_INPUT_0 = 0,
+	PCA9539_INPUT_1 = 1,
+	PCA9539_OUTPUT_0 = 2,
+	PCA9539_OUTPUT_1 = 3,
+	PCA9539_INVERT_0 = 4,
+	PCA9539_INVERT_1 = 5,
+	PCA9539_CONFIG_0 = 6,
+	PCA9539_CONFIG_1 = 7
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter);
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int
kind);
+static int pca9539_detach_client(struct i2c_client *client);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pca9539_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "pca9539",
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= pca9539_attach_adapter,
+	.detach_client	= pca9539_detach_client,
+};
+
+struct pca9539_data {
+	struct i2c_client client;
+};
+
+/* following are the sysfs callback functions */
+static ssize_t pca9539_show(struct device *dev, struct device_attribute
*attr,
+			    char *buf)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct pca9539_data *data i2c_get_clientdata(to_i2c_client(dev));
+	return sprintf(buf, "%d\n",
i2c_smbus_read_byte_data(&data->client, (u8)psa->index));
+}
+
+static ssize_t pca9539_store(struct device *dev, struct
device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct pca9539_data *data i2c_get_clientdata(to_i2c_client(dev));
+	u8 val = simple_strtoul(buf, NULL, 0);
+	i2c_smbus_write_byte_data(&data->client, (u8)psa->index, val);
+	return count;
+}
+
+/* Define the device attributes */
+
+#define PCA9539_ENTRY_RO(name,enum_name)	\
+	static
SENSOR_DEVICE_ATTR(name,S_IRUGO,pca9539_show,NULL,enum_name)
+
+#define PCA9539_ENTRY_RW(name,enum_name)	\
+	static
SENSOR_DEVICE_ATTR(name,S_IRUGO|S_IWUSR,pca9539_show,pca9539_store,enum_
name)
+
+PCA9539_ENTRY_RO(input0,  PCA9539_INPUT_0);
+PCA9539_ENTRY_RO(input1,  PCA9539_INPUT_1);
+PCA9539_ENTRY_RW(output0, PCA9539_OUTPUT_0);
+PCA9539_ENTRY_RW(output1, PCA9539_OUTPUT_1);
+PCA9539_ENTRY_RW(invert0, PCA9539_INVERT_0);
+PCA9539_ENTRY_RW(invert1, PCA9539_INVERT_1);
+PCA9539_ENTRY_RW(config0, PCA9539_CONFIG_0);
+PCA9539_ENTRY_RW(config1, PCA9539_CONFIG_1);
+
+static struct attribute *pca9539_attributes[] = {
+	&sensor_dev_attr_input0.dev_attr.attr,
+	&sensor_dev_attr_input1.dev_attr.attr,
+	&sensor_dev_attr_output0.dev_attr.attr,
+	&sensor_dev_attr_output1.dev_attr.attr,
+	&sensor_dev_attr_invert0.dev_attr.attr,
+	&sensor_dev_attr_invert1.dev_attr.attr,
+	&sensor_dev_attr_config0.dev_attr.attr,
+	&sensor_dev_attr_config1.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group pca9539_defattr_group = {
+	.attrs = pca9539_attributes,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_detect(adapter, &addr_data, pca9539_detect);
+}
+
+/* This function is called by i2c_detect */
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int
kind)
+{
+	struct i2c_client *new_client;
+	struct pca9539_data *data;
+	int err = 0;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE
+				     | I2C_FUNC_SMBUS_WRITE_BYTE_DATA
+				     | I2C_FUNC_SMBUS_READ_BYTE_DATA))
+		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 = kmalloc(sizeof(struct pca9539_data), GFP_KERNEL)))
{
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct pca9539_data));
+	
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &pca9539_driver;
+	new_client->flags = 0;
+
+	/* Detection: not sure how to do that... 
+	 * possibility: toggle the value read from invert0 and see if
input0
+	 * changes polarity. 
+	 * However, if the inputs are changing, this won't work.
+	 */
+	strlcpy(new_client->name, "pca9539", I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_kfree;
+
+	/* Register sysfs hooks */
+	return sysfs_create_group(&new_client->dev.kobj,
&pca9539_defattr_group);
+
+exit_kfree:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int pca9539_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;
+}
+
+static int __init pca9539_init(void)
+{
+	return i2c_add_driver(&pca9539_driver);
+}
+
+static void __exit pca9539_exit(void)
+{
+	i2c_del_driver(&pca9539_driver);
+}
+
+MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
+MODULE_DESCRIPTION("PCA9539 driver");
+MODULE_LICENSE("GPL");
+
+module_init(pca9539_init);
+module_exit(pca9539_exit);
+
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Makefile
2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Makefile
2005-06-03 12:09:41.000000000 -0500
@@ -34,6 +34,7 @@
 obj-$(CONFIG_SENSORS_MAX1619)	+= max1619.o
 obj-$(CONFIG_SENSORS_M41T00)	+= m41t00.o
 obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
+obj-$(CONFIG_SENSORS_PCA9539)	+= pca9539.o
 obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_RTC8564)	+= rtc8564.o
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig
2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig
2005-06-03 12:09:22.000000000 -0500
@@ -498,4 +498,14 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called m41t00.
 
+config SENSORS_PCA9539
+	tristate "PCA9539 16-bit digital IO"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the PCA9539
+	  low power digital I/O with 16 bit.
+          
+	  This driver can also be built as a module.  If so, the module
+	  will be called pca9539.
+
 endmenu

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[Greg KH]

On Fri, Jun 03, 2005 at 04:29:28PM -0400, BGardner@Wabtec.com wrote:
> 
> Please ignore the previous patch -- I had left out the Makefile and
> Kconfig changes.
> 
> This is a driver for the PCA9539, a 16 bit digital I/O chip.
> It uses the new i2c-sysfs interfaces.
> 
> Signed-off-by: Ben Gardner <bgardner@wabtec.com>
> 
> --- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c
> 1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c
> 2005-06-03 15:15:02.000000000 -0500

Hm, the patch is line-wrapped.  Care to try it again?

thanks,

greg k-h

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[BGardner]

Sorry about that. I suspect that my MS Exchange server is wrapping lines
when it processes mail heading out to the internet.  You know, when it
attaches that oh-so-useful disclaimer.
Anyway, here is attempt 3, as an attachment.
=
This is a driver for the PCA9539, a 16 bit digital I/O chip.
It uses the new i2c-sysfs interfaces.

Signed-off-by: Ben Gardner <bgardner@wabtec.com>
-------------- next part --------------
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c	2005-06-03 15:15:02.000000000 -0500
@@ -0,0 +1,188 @@
+/*
+    pca9539.c - 16 port digital I/O with interrupt and reset
+    
+    Copyright (C) 2005 Ben Gardner <bgardner@wabtec.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; version 2 of the License.
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sysfs.h>
+#include <linux/i2c-sensor.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = { 0x74, 0x75, 0x76, 0x77, I2C_CLIENT_END };
+static unsigned int normal_isa[] = { I2C_CLIENT_ISA_END };
+
+/* Insmod parameters */
+SENSORS_INSMOD_1(pca9539);
+
+enum pca9539_cmd
+{
+	PCA9539_INPUT_0 = 0,
+	PCA9539_INPUT_1 = 1,
+	PCA9539_OUTPUT_0 = 2,
+	PCA9539_OUTPUT_1 = 3,
+	PCA9539_INVERT_0 = 4,
+	PCA9539_INVERT_1 = 5,
+	PCA9539_CONFIG_0 = 6,
+	PCA9539_CONFIG_1 = 7
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter);
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind);
+static int pca9539_detach_client(struct i2c_client *client);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pca9539_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "pca9539",
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= pca9539_attach_adapter,
+	.detach_client	= pca9539_detach_client,
+};
+
+struct pca9539_data {
+	struct i2c_client client;
+};
+
+/* following are the sysfs callback functions */
+static ssize_t pca9539_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct pca9539_data *data = i2c_get_clientdata(to_i2c_client(dev));
+	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(&data->client, (u8)psa->index));
+}
+
+static ssize_t pca9539_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct pca9539_data *data = i2c_get_clientdata(to_i2c_client(dev));
+	u8 val = simple_strtoul(buf, NULL, 0);
+	i2c_smbus_write_byte_data(&data->client, (u8)psa->index, val);
+	return count;
+}
+
+/* Define the device attributes */
+
+#define PCA9539_ENTRY_RO(name,enum_name)	\
+	static SENSOR_DEVICE_ATTR(name,S_IRUGO,pca9539_show,NULL,enum_name)
+
+#define PCA9539_ENTRY_RW(name,enum_name)	\
+	static SENSOR_DEVICE_ATTR(name,S_IRUGO|S_IWUSR,pca9539_show,pca9539_store,enum_name)
+
+PCA9539_ENTRY_RO(input0,  PCA9539_INPUT_0);
+PCA9539_ENTRY_RO(input1,  PCA9539_INPUT_1);
+PCA9539_ENTRY_RW(output0, PCA9539_OUTPUT_0);
+PCA9539_ENTRY_RW(output1, PCA9539_OUTPUT_1);
+PCA9539_ENTRY_RW(invert0, PCA9539_INVERT_0);
+PCA9539_ENTRY_RW(invert1, PCA9539_INVERT_1);
+PCA9539_ENTRY_RW(config0, PCA9539_CONFIG_0);
+PCA9539_ENTRY_RW(config1, PCA9539_CONFIG_1);
+
+static struct attribute *pca9539_attributes[] = {
+	&sensor_dev_attr_input0.dev_attr.attr,
+	&sensor_dev_attr_input1.dev_attr.attr,
+	&sensor_dev_attr_output0.dev_attr.attr,
+	&sensor_dev_attr_output1.dev_attr.attr,
+	&sensor_dev_attr_invert0.dev_attr.attr,
+	&sensor_dev_attr_invert1.dev_attr.attr,
+	&sensor_dev_attr_config0.dev_attr.attr,
+	&sensor_dev_attr_config1.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group pca9539_defattr_group = {
+	.attrs = pca9539_attributes,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_detect(adapter, &addr_data, pca9539_detect);
+}
+
+/* This function is called by i2c_detect */
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct pca9539_data *data;
+	int err = 0;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE
+				     | I2C_FUNC_SMBUS_WRITE_BYTE_DATA
+				     | I2C_FUNC_SMBUS_READ_BYTE_DATA))
+		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 = kmalloc(sizeof(struct pca9539_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct pca9539_data));
+	
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &pca9539_driver;
+	new_client->flags = 0;
+
+	/* Detection: not sure how to do that... 
+	 * possibility: toggle the value read from invert0 and see if input0
+	 * changes polarity. 
+	 * However, if the inputs are changing, this won't work.
+	 */
+	strlcpy(new_client->name, "pca9539", I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_kfree;
+
+	/* Register sysfs hooks */
+	return sysfs_create_group(&new_client->dev.kobj, &pca9539_defattr_group);
+
+exit_kfree:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int pca9539_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;
+}
+
+static int __init pca9539_init(void)
+{
+	return i2c_add_driver(&pca9539_driver);
+}
+
+static void __exit pca9539_exit(void)
+{
+	i2c_del_driver(&pca9539_driver);
+}
+
+MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
+MODULE_DESCRIPTION("PCA9539 driver");
+MODULE_LICENSE("GPL");
+
+module_init(pca9539_init);
+module_exit(pca9539_exit);
+
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Makefile	2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Makefile	2005-06-03 12:09:41.000000000 -0500
@@ -34,6 +34,7 @@
 obj-$(CONFIG_SENSORS_MAX1619)	+= max1619.o
 obj-$(CONFIG_SENSORS_M41T00)	+= m41t00.o
 obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
+obj-$(CONFIG_SENSORS_PCA9539)	+= pca9539.o
 obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_RTC8564)	+= rtc8564.o
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig	2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig	2005-06-03 12:09:22.000000000 -0500
@@ -498,4 +498,14 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called m41t00.
 
+config SENSORS_PCA9539
+	tristate "PCA9539 16-bit digital IO"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the PCA9539
+	  low power digital I/O with 16 bit.
+          
+	  This driver can also be built as a module.  If so, the module
+	  will be called pca9539.
+
 endmenu

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[Jean Delvare]

Hi Ben,

> Anyway, here is attempt 3, as an attachment.
> => 
> This is a driver for the PCA9539, a 16 bit digital I/O chip.
> It uses the new i2c-sysfs interfaces.

And here are my comments about your code (which overall looks very
good).

> +/* following are the sysfs callback functions */
> +static ssize_t pca9539_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
> +	struct pca9539_data *data = i2c_get_clientdata(to_i2c_client(dev));
> +	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(&data->client, (u8)psa->index));
> +}

This can be made a lot more simple. You currently go from dev to client
to data back to client. What about:

static ssize_t pca9539_show(struct device *dev, struct device_attribute *attr,
			    char *buf)
{
	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
	struct i2c_client *client = to_i2c_client(dev);
	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(client, (u8)psa->index));
}

Same holds for pca9539_store(), obviously.

BTW, please split lines which are longer than 80 characters, as
mentioned in Documentation/CodingStyle.

> +static ssize_t pca9539_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t count)
> +{
> +	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
> +	struct pca9539_data *data = i2c_get_clientdata(to_i2c_client(dev));
> +	u8 val = simple_strtoul(buf, NULL, 0);
> +	i2c_smbus_write_byte_data(&data->client, (u8)psa->index, val);
> +	return count;
> +}

You fail to check the validity of the new value. Wrapping it into an u8
is no good. It would be far better to actually check the value, and
reject it if incorrect. Something in the lines of:

	unsigned long val = simple_strtoul(buf, NULL, 0);
	if (val > 0xff)
		return -EINVAL;

This is what the similar pcf8574 driver does. You may even have it warn
in the logs if you want.

> +	static SENSOR_DEVICE_ATTR(name,S_IRUGO,pca9539_show,NULL,enum_name)
> (...)
> +	static SENSOR_DEVICE_ATTR(name,S_IRUGO|S_IWUSR,pca9539_show,pca9539_store,enum_name)

Add one space after each comma please. Spaces are also welcome around
"|". As a side note, "enum_name" is a bad name IMHO, I would go for
"index" or "command" or whatever you like which expresses what this
parameter is holding.

> +PCA9539_ENTRY_RW(config0, PCA9539_CONFIG_0);
> +PCA9539_ENTRY_RW(config1, PCA9539_CONFIG_1);

"config" isn't a very relevant name. It could mean about anything
depending on the chip. What about "direction" instead?

> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE
> +				     | I2C_FUNC_SMBUS_WRITE_BYTE_DATA
> +				     | I2C_FUNC_SMBUS_READ_BYTE_DATA))
> +		goto exit;

Not correct. You test for I2C_FUNC_SMBUS_BYTE but never actually use
this SMBus command in your code. Additionally,
I2C_FUNC_SMBUS_WRITE_BYTE_DATA | I2C_FUNC_SMBUS_READ_BYTE_DATA can be
shorten into I2C_FUNC_SMBUS_BYTE_DATA.

> +	/* Detection: not sure how to do that... 
> +	 * possibility: toggle the value read from invert0 and see if input0
> +	 * changes polarity. 
> +	 * However, if the inputs are changing, this won't work.
> +	 */

No, detection code should not write anything to the chips.

One possible detection method is to check that input and output match
(modulo the polarity inversion) for all pins configured as outputs. I
have made something similar in sensors-detect for the PCA9556, which is
basically the same chip as the PCA9539 with half the GPIO pins. The code
there reads like:

	return unless ($input & ~$config) = (($output ^ $invert) & ~$config);

Another possible check would be on the number of registers (or
commands). The PCA9539 has 8 commands, so reading "register" 0x07 will
return a value, while reading register "0x08" will return an error.

If you could add detection for your chip to sensors-detect once you're
done with the driver, this would be appreciated.

> +	/* Register sysfs hooks */
> +	return sysfs_create_group(&new_client->dev.kobj, &pca9539_defattr_group);

Negative, an error from sysfs_create_group() should not be returned by
the detection function due to the current (broken) design of
i2c_detect().

> --- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig	2005-06-01 14:29:17.000000000 -0500
> +++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig	2005-06-03 12:09:22.000000000 -0500
> @@ -498,4 +498,14 @@
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called m41t00.
>  
> +config SENSORS_PCA9539
> +	tristate "PCA9539 16-bit digital IO"
> +	depends on I2C && EXPERIMENTAL
> +	help
> +	  If you say yes here you get support for the PCA9539
> +	  low power digital I/O with 16 bit.
> +          
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called pca9539.
> +
>  endmenu

I'd rather see this entry next to the PCF8574 one, as both chips are
similar in their function.

Thanks,
-- 
Jean Delvare

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[Yani Ioannou]

Hi Ben,

With respect to the sysfs attributes/callbacks much better :-), it
really cleans up the code using the new callbacks and initializing the
sensor device attributes as an attribute group. It also saves me or
others having to go through and try to clean things up later.

Thanks,
Yani

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[BGardner]

Hi Jean, 

Thanks for your feedback.
I've incorporated your suggestions, so the next pca9539 patch should be
'final'. ;)

Ben

-----Original Message-----
From: Jean Delvare [mailto:khali@linux-fr.org] 
Sent: Saturday, June 04, 2005 2:26 AM
To: Gardner, Ben
Cc: LM Sensors; Greg KH
Subject: Re: [lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier
(retry 3 - attachment)

Hi Ben,

> Anyway, here is attempt 3, as an attachment.
> => 
> This is a driver for the PCA9539, a 16 bit digital I/O chip.
> It uses the new i2c-sysfs interfaces.

And here are my comments about your code (which overall looks very
good).

> +/* following are the sysfs callback functions */ static ssize_t 
> +pca9539_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
> +	struct pca9539_data *data i2c_get_clientdata(to_i2c_client(dev));
> +	return sprintf(buf, "%d\n",
i2c_smbus_read_byte_data(&data->client, 
> +(u8)psa->index)); }

This can be made a lot more simple. You currently go from dev to client
to data back to client. What about:

static ssize_t pca9539_show(struct device *dev, struct device_attribute
*attr,
			    char *buf)
{
	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
	struct i2c_client *client = to_i2c_client(dev);
	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(client,
(u8)psa->index)); }

Same holds for pca9539_store(), obviously.

BTW, please split lines which are longer than 80 characters, as
mentioned in Documentation/CodingStyle.

> +static ssize_t pca9539_store(struct device *dev, struct
device_attribute *attr,
> +			     const char *buf, size_t count) {
> +	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
> +	struct pca9539_data *data i2c_get_clientdata(to_i2c_client(dev));
> +	u8 val = simple_strtoul(buf, NULL, 0);
> +	i2c_smbus_write_byte_data(&data->client, (u8)psa->index, val);
> +	return count;
> +}

You fail to check the validity of the new value. Wrapping it into an u8
is no good. It would be far better to actually check the value, and
reject it if incorrect. Something in the lines of:

	unsigned long val = simple_strtoul(buf, NULL, 0);
	if (val > 0xff)
		return -EINVAL;

This is what the similar pcf8574 driver does. You may even have it warn
in the logs if you want.

> +	static
SENSOR_DEVICE_ATTR(name,S_IRUGO,pca9539_show,NULL,enum_name)
> (...)
> +	static 
> +SENSOR_DEVICE_ATTR(name,S_IRUGO|S_IWUSR,pca9539_show,pca9539_store,en
> +um_name)

Add one space after each comma please. Spaces are also welcome around
"|". As a side note, "enum_name" is a bad name IMHO, I would go for
"index" or "command" or whatever you like which expresses what this
parameter is holding.

> +PCA9539_ENTRY_RW(config0, PCA9539_CONFIG_0); 
> +PCA9539_ENTRY_RW(config1, PCA9539_CONFIG_1);

"config" isn't a very relevant name. It could mean about anything
depending on the chip. What about "direction" instead?

> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE
> +				     | I2C_FUNC_SMBUS_WRITE_BYTE_DATA
> +				     | I2C_FUNC_SMBUS_READ_BYTE_DATA))
> +		goto exit;

Not correct. You test for I2C_FUNC_SMBUS_BYTE but never actually use
this SMBus command in your code. Additionally,
I2C_FUNC_SMBUS_WRITE_BYTE_DATA | I2C_FUNC_SMBUS_READ_BYTE_DATA can be
shorten into I2C_FUNC_SMBUS_BYTE_DATA.

> +	/* Detection: not sure how to do that... 
> +	 * possibility: toggle the value read from invert0 and see if
input0
> +	 * changes polarity. 
> +	 * However, if the inputs are changing, this won't work.
> +	 */

No, detection code should not write anything to the chips.

One possible detection method is to check that input and output match
(modulo the polarity inversion) for all pins configured as outputs. I
have made something similar in sensors-detect for the PCA9556, which is
basically the same chip as the PCA9539 with half the GPIO pins. The code
there reads like:

	return unless ($input & ~$config) = (($output ^ $invert) &
~$config);

Another possible check would be on the number of registers (or
commands). The PCA9539 has 8 commands, so reading "register" 0x07 will
return a value, while reading register "0x08" will return an error.

If you could add detection for your chip to sensors-detect once you're
done with the driver, this would be appreciated.

> +	/* Register sysfs hooks */
> +	return sysfs_create_group(&new_client->dev.kobj, 
> +&pca9539_defattr_group);

Negative, an error from sysfs_create_group() should not be returned by
the detection function due to the current (broken) design of
i2c_detect().

> --- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig
2005-06-01 14:29:17.000000000 -0500
> +++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig
2005-06-03 12:09:22.000000000 -0500
> @@ -498,4 +498,14 @@
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called m41t00.
>  
> +config SENSORS_PCA9539
> +	tristate "PCA9539 16-bit digital IO"
> +	depends on I2C && EXPERIMENTAL
> +	help
> +	  If you say yes here you get support for the PCA9539
> +	  low power digital I/O with 16 bit.
> +          
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called pca9539.
> +
>  endmenu

I'd rather see this entry next to the PCF8574 one, as both chips are
similar in their function.

Thanks,
--
Jean Delvare

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier (retry 4

[BGardner]

Greg,

Attached (due to email line-wrapping problems) is an updated patch for
the PCA9539.
I think this one is now ready for inclusion.

Thanks,
Ben

=
This is a driver for the PCA9539, a 16 bit digital I/O chip.
It uses the new i2c-sysfs interfaces.
Includes documentation.

Signed-off-by: Ben Gardner <bgardner@wabtec.com>
-------------- next part --------------
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig	2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig	2005-06-06 09:22:05.904153486 -0500
@@ -440,6 +440,16 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called pcf8574.
 
+config SENSORS_PCA9539
+	tristate "PCA9539 16-bit digital IO"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the PCA9539
+	  low power digital I/O with 16 bit.
+          
+	  This driver can also be built as a module.  If so, the module
+	  will be called pca9539.
+
 config SENSORS_PCF8591
 	tristate "Philips PCF8591"
 	depends on I2C && EXPERIMENTAL
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Makefile	2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Makefile	2005-06-03 12:09:41.000000000 -0500
@@ -34,6 +34,7 @@
 obj-$(CONFIG_SENSORS_MAX1619)	+= max1619.o
 obj-$(CONFIG_SENSORS_M41T00)	+= m41t00.o
 obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
+obj-$(CONFIG_SENSORS_PCA9539)	+= pca9539.o
 obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_RTC8564)	+= rtc8564.o
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c	2005-06-06 09:21:15.000000000 -0500
@@ -0,0 +1,192 @@
+/*
+    pca9539.c - 16 port digital I/O with interrupt and reset
+    
+    Copyright (C) 2005 Ben Gardner <bgardner@wabtec.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; version 2 of the License.
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-sysfs.h>
+#include <linux/i2c-sensor.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = {0x74, 0x75, 0x76, 0x77, I2C_CLIENT_END};
+static unsigned int normal_isa[] = {I2C_CLIENT_ISA_END};
+
+/* Insmod parameters */
+SENSORS_INSMOD_1(pca9539);
+
+enum pca9539_cmd
+{
+	PCA9539_INPUT_0 = 0,
+	PCA9539_INPUT_1 = 1,
+	PCA9539_OUTPUT_0 = 2,
+	PCA9539_OUTPUT_1 = 3,
+	PCA9539_INVERT_0 = 4,
+	PCA9539_INVERT_1 = 5,
+	PCA9539_DIRECTION_0 = 6,
+	PCA9539_DIRECTION_1 = 7
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter);
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind);
+static int pca9539_detach_client(struct i2c_client *client);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pca9539_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "pca9539",
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= pca9539_attach_adapter,
+	.detach_client	= pca9539_detach_client,
+};
+
+struct pca9539_data {
+	struct i2c_client client;
+};
+
+/* following are the sysfs callback functions */
+static ssize_t pca9539_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(client, 
+							     (u8)psa->index));
+}
+
+static ssize_t pca9539_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	unsigned long val = simple_strtoul(buf, NULL, 0);
+	if (val > 0xff)
+		return -EINVAL;
+	i2c_smbus_write_byte_data(client, (u8)psa->index, (u8)val);
+	return count;
+}
+
+/* Define the device attributes */
+
+#define PCA9539_ENTRY_RO(name, cmd_idx) \
+	static SENSOR_DEVICE_ATTR(name, S_IRUGO, pca9539_show, NULL, cmd_idx)
+
+#define PCA9539_ENTRY_RW(name, cmd_idx) \
+	static SENSOR_DEVICE_ATTR(name, S_IRUGO | S_IWUSR, pca9539_show, \
+				  pca9539_store, cmd_idx)
+
+PCA9539_ENTRY_RO(input0, PCA9539_INPUT_0);
+PCA9539_ENTRY_RO(input1, PCA9539_INPUT_1);
+PCA9539_ENTRY_RW(output0, PCA9539_OUTPUT_0);
+PCA9539_ENTRY_RW(output1, PCA9539_OUTPUT_1);
+PCA9539_ENTRY_RW(invert0, PCA9539_INVERT_0);
+PCA9539_ENTRY_RW(invert1, PCA9539_INVERT_1);
+PCA9539_ENTRY_RW(direction0, PCA9539_DIRECTION_0);
+PCA9539_ENTRY_RW(direction1, PCA9539_DIRECTION_1);
+
+static struct attribute *pca9539_attributes[] = {
+	&sensor_dev_attr_input0.dev_attr.attr,
+	&sensor_dev_attr_input1.dev_attr.attr,
+	&sensor_dev_attr_output0.dev_attr.attr,
+	&sensor_dev_attr_output1.dev_attr.attr,
+	&sensor_dev_attr_invert0.dev_attr.attr,
+	&sensor_dev_attr_invert1.dev_attr.attr,
+	&sensor_dev_attr_direction0.dev_attr.attr,
+	&sensor_dev_attr_direction1.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group pca9539_defattr_group = {
+	.attrs = pca9539_attributes,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_detect(adapter, &addr_data, pca9539_detect);
+}
+
+/* This function is called by i2c_detect */
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct pca9539_data *data;
+	int err = 0;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		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 = kmalloc(sizeof(struct pca9539_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct pca9539_data));
+	
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &pca9539_driver;
+	new_client->flags = 0;
+
+	/* Detection: the pcs9539 only has 8 registers (0-7).
+	   A read of 7 should succeed, but a read of 8 should fail. */
+	if ((i2c_smbus_read_byte_data(new_client, 7) < 0) &&
+	    (i2c_smbus_read_byte_data(new_client, 8) >= 0))
+		goto exit_kfree;
+
+	strlcpy(new_client->name, "pca9539", I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_kfree;
+
+	/* Register sysfs hooks (don't care about failure) */
+	sysfs_create_group(&new_client->dev.kobj, &pca9539_defattr_group);
+
+	return 0;
+
+exit_kfree:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int pca9539_detach_client(struct i2c_client *client)
+{
+	int err;
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev, "Client deregistration failed.\n");
+		return err;
+	}
+
+	kfree(i2c_get_clientdata(client));
+	return 0;
+}
+
+static int __init pca9539_init(void)
+{
+	return i2c_add_driver(&pca9539_driver);
+}
+
+static void __exit pca9539_exit(void)
+{
+	i2c_del_driver(&pca9539_driver);
+}
+
+MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
+MODULE_DESCRIPTION("PCA9539 driver");
+MODULE_LICENSE("GPL");
+
+module_init(pca9539_init);
+module_exit(pca9539_exit);
+
--- linux-2.6.12-rc5-mm2-clean/Documentation/i2c/chips/pca9539	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2/Documentation/i2c/chips/pca9539	2005-06-06 09:36:49.167804115 -0500
@@ -0,0 +1,54 @@
+Kernel driver pca9539
+==========+
+Supported chips:
+  * Maxim pca9539
+    Prefixes: 'pca9539'
+    Addresses scanned: 0x74 - 0x77
+    Datasheets:
+        http://www.semiconductors.philips.com/acrobat/datasheets/PCA9539_2.pdf
+
+Author: Ben Gardner <bgardner@wabtec.com>
+
+
+Module Parameters
+-----------------
+
+None
+
+
+Description
+-----------
+
+The Phillips PCA9539 is a 16 bit low power I/O device.
+All 16 lines can be individually configured as an input or output.
+The input sense can also be inverted.
+The 16 lines are split between two bytes.
+
+
+Sysfs entries
+-------------
+
+Each is a byte that maps to the 8 I/O bits.
+A '0' suffix is for bits 0-7, while '1' is for bits 8-15.
+
+name          - reads 'pca9539'
+input[01]     - read the current value
+output[01]    - sets the output value
+direction[01] - direction of each bit: 1=input, 0=output
+invert[01]    - toggle the input bit sense
+
+input reads the actual state of the line and is always available.
+The direction defaults to input only for all channels.
+
+
+General Remarks
+---------------
+
+Note that each output, direction, and invert entry controls 8 lines.
+You should use the read, modify, write sequence.
+For example. to set output bit 0 of 1.
+  val=$(cat output0)
+  val=$(( $val | 1 ))
+  echo $val > output0
+

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[Jean Delvare]

Hi Ben,

> Attached (due to email line-wrapping problems) is an updated patch for
> the PCA9539.
> I think this one is now ready for inclusion.

I'm very sorry but it looks like I still have a few objections ;)

> --- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig	2005-06-01 14:29:17.000000000 -0500
> +++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig	2005-06-06 09:22:05.904153486 -0500
> @@ -440,6 +440,16 @@
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called pcf8574.
>  
> +config SENSORS_PCA9539
> +	tristate "PCA9539 16-bit digital IO"

"Philips PCA9539 16-bit digital I/O"

(BTW, is there something like analog bits?)

> +	depends on I2C && EXPERIMENTAL
> +	help
> +	  If you say yes here you get support for the PCA9539

Philips PCA9539

> +	  low power digital I/O with 16 bit.

"with 16 bit" sounds really weird. "16-bit digital I/O" here again
maybe?

> --- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c	1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c	2005-06-06 09:21:15.000000000 -0500
> @@ -0,0 +1,192 @@
> +/*
> +    pca9539.c - 16 port digital I/O with interrupt and reset

s/ port/-bit/ for consistency.

> +#include <linux/i2c-sysfs.h>

It has become linux/hwmon-sysfs.h a few hours ago ;)

http://lkml.org/lkml/2005/6/6/108

> +enum pca9539_cmd
> +{
> +	PCA9539_INPUT_0 = 0,
> +	PCA9539_INPUT_1 = 1,
> +	PCA9539_OUTPUT_0 = 2,
> +	PCA9539_OUTPUT_1 = 3,
> +	PCA9539_INVERT_0 = 4,
> +	PCA9539_INVERT_1 = 5,
> +	PCA9539_DIRECTION_0 = 6,
> +	PCA9539_DIRECTION_1 = 7
> +};

I think tab-aligned equal signs would improve readability. Also, it's
considered good practice to leave a comma on the last line, so as to
minimize possible further patches.

> +	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(client, 
> +							     (u8)psa->index));
> (...)
> +	i2c_smbus_write_byte_data(client, (u8)psa->index, (u8)val);

These (u8) casts do not seem to be needed.

> +	/* Detection: the pcs9539 only has 8 registers (0-7).
> +	   A read of 7 should succeed, but a read of 8 should fail. */

s/pcs/pca/

> +	if ((i2c_smbus_read_byte_data(new_client, 7) < 0) &&
> +	    (i2c_smbus_read_byte_data(new_client, 8) >= 0))
> +		goto exit_kfree;

Shouldn't this be || rather than &&? Either condition is sufficient to
discard the chip.

> --- linux-2.6.12-rc5-mm2-clean/Documentation/i2c/chips/pca9539	1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.12-rc5-mm2/Documentation/i2c/chips/pca9539	2005-06-06 09:36:49.167804115 -0500
> @@ -0,0 +1,54 @@
> +Kernel driver pca9539
> +==========> +
> +Supported chips:
> +  * Maxim pca9539

Isn't it really Philips? Also, we tend to use caps when designating the
chips themselves.

> +    Prefixes: 'pca9539'

No s, there's a single prefix.

> +    Addresses scanned: 0x74 - 0x77
> +    Datasheets:

No s, there's a single datasheet.

> +Module Parameters
> +-----------------
> +
> +None
> +
> +

You can skip that section altogether.

> +Description
> +-----------
> +
> +The Phillips PCA9539 is a 16 bit low power I/O device.

Single 'l' to Philips.

> +name          - reads 'pca9539'

This is a standard entry, do not bother documenting it here.

> +The direction defaults to input only for all channels.

s/only//?

OK, I think that's all, and it should be alright next time :)

Thanks for your collaboration, I know how much work it is to go through
these review & update cycles, but this is how we manage to get the best
code and documentation into the kernel tree.

-- 
Jean Delvare

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[BGardner]

Jean,

Thanks for taking the time to review.
I made the changes you suggested and will repost as retry 5.
For the description, I went with the wording in the datasheet: "16-bit
I/O port" 

Ben

-----Original Message-----
From: Jean Delvare [mailto:khali@linux-fr.org] 
Sent: Monday, June 06, 2005 2:37 PM
To: Gardner, Ben
Cc: Greg KH; LM Sensors
Subject: Re: [lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier
(retry 4 - attachment)

Hi Ben,

> Attached (due to email line-wrapping problems) is an updated patch for

> the PCA9539.
> I think this one is now ready for inclusion.

I'm very sorry but it looks like I still have a few objections ;)

> --- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig
2005-06-01 14:29:17.000000000 -0500
> +++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig
2005-06-06 09:22:05.904153486 -0500
> @@ -440,6 +440,16 @@
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called pcf8574.
>  
> +config SENSORS_PCA9539
> +	tristate "PCA9539 16-bit digital IO"

"Philips PCA9539 16-bit digital I/O"

(BTW, is there something like analog bits?)

> +	depends on I2C && EXPERIMENTAL
> +	help
> +	  If you say yes here you get support for the PCA9539

Philips PCA9539

> +	  low power digital I/O with 16 bit.

"with 16 bit" sounds really weird. "16-bit digital I/O" here again
maybe?

> --- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c
1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c
2005-06-06 09:21:15.000000000 -0500
> @@ -0,0 +1,192 @@
> +/*
> +    pca9539.c - 16 port digital I/O with interrupt and reset

s/ port/-bit/ for consistency.

> +#include <linux/i2c-sysfs.h>

It has become linux/hwmon-sysfs.h a few hours ago ;)

http://lkml.org/lkml/2005/6/6/108

> +enum pca9539_cmd
> +{
> +	PCA9539_INPUT_0 = 0,
> +	PCA9539_INPUT_1 = 1,
> +	PCA9539_OUTPUT_0 = 2,
> +	PCA9539_OUTPUT_1 = 3,
> +	PCA9539_INVERT_0 = 4,
> +	PCA9539_INVERT_1 = 5,
> +	PCA9539_DIRECTION_0 = 6,
> +	PCA9539_DIRECTION_1 = 7
> +};

I think tab-aligned equal signs would improve readability. Also, it's
considered good practice to leave a comma on the last line, so as to
minimize possible further patches.

> +	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(client, 
> +
(u8)psa->index));
> (...)
> +	i2c_smbus_write_byte_data(client, (u8)psa->index, (u8)val);

These (u8) casts do not seem to be needed.

> +	/* Detection: the pcs9539 only has 8 registers (0-7).
> +	   A read of 7 should succeed, but a read of 8 should fail. */

s/pcs/pca/

> +	if ((i2c_smbus_read_byte_data(new_client, 7) < 0) &&
> +	    (i2c_smbus_read_byte_data(new_client, 8) >= 0))
> +		goto exit_kfree;

Shouldn't this be || rather than &&? Either condition is sufficient to
discard the chip.

> --- linux-2.6.12-rc5-mm2-clean/Documentation/i2c/chips/pca9539
1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.12-rc5-mm2/Documentation/i2c/chips/pca9539
2005-06-06 09:36:49.167804115 -0500
> @@ -0,0 +1,54 @@
> +Kernel driver pca9539
> +==========> +
> +Supported chips:
> +  * Maxim pca9539

Isn't it really Philips? Also, we tend to use caps when designating the
chips themselves.

> +    Prefixes: 'pca9539'

No s, there's a single prefix.

> +    Addresses scanned: 0x74 - 0x77
> +    Datasheets:

No s, there's a single datasheet.

> +Module Parameters
> +-----------------
> +
> +None
> +
> +

You can skip that section altogether.

> +Description
> +-----------
> +
> +The Phillips PCA9539 is a 16 bit low power I/O device.

Single 'l' to Philips.

> +name          - reads 'pca9539'

This is a standard entry, do not bother documenting it here.

> +The direction defaults to input only for all channels.

s/only//?

OK, I think that's all, and it should be alright next time :)

Thanks for your collaboration, I know how much work it is to go through
these review & update cycles, but this is how we manage to get the best
code and documentation into the kernel tree.

--
Jean Delvare

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier (retry 5

[BGardner]

Greg,

Attached (due to email line-wrapping problems) is an updated patch for
the Philips PCA9539.
I think this one is now ready for inclusion.

Thanks,
Ben

=
This is a driver for the PCA9539, a 16 bit digital I/O chip.
It uses the new i2c-sysfs interfaces.
The patch includes documentation.
It depends on the patch that renames "i2c-sysfs.h" to "hwmon-sysfs.h"

Signed-off-by: Ben Gardner <bgardner@wabtec.com>

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[Greg KH]

On Mon, Jun 06, 2005 at 04:32:52PM -0400, BGardner@Wabtec.com wrote:
> Greg,
> 
> Attached (due to email line-wrapping problems) is an updated patch for
> the Philips PCA9539.
> I think this one is now ready for inclusion.

-ENOPATCH :(

And please don't attach base-64 patches, they are a pain.

If only everyone would just use a sane email client...

thanks,

greg k-h

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c driver (retry 7)

[bgardner]

Here's a retry using KMail as the email client.
=
This is a driver for the PCA9539, a 16 bit digital I/O chip.
It uses the new i2c-sysfs interfaces.
The patch includes documentation.
It depends on the patch that renames "i2c-sysfs.h" to "hwmon-sysfs.h"

Signed-off-by: Ben Gardner <bgardner@wabtec.com>

--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig 2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig 2005-06-06 15:24:28.961183516 -0500
@@ -440,6 +440,16 @@
? ? This driver can also be built as a module. ?If so, the module
? ? will be called pcf8574.
?
+config SENSORS_PCA9539
+ tristate "Philips PCA9539 16-bit I/O port"
+ depends on I2C && EXPERIMENTAL
+ help
+ ? If you say yes here you get support for the Philips PCA9539
+ ? 16-bit I/O port.
+ ? ? ? ? ?
+ ? This driver can also be built as a module. ?If so, the module
+ ? will be called pca9539.
+
?config SENSORS_PCF8591
? tristate "Philips PCF8591"
? depends on I2C && EXPERIMENTAL
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Makefile 2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Makefile 2005-06-03 12:09:41.000000000 -0500
@@ -34,6 +34,7 @@
?obj-$(CONFIG_SENSORS_MAX1619) += max1619.o
?obj-$(CONFIG_SENSORS_M41T00) += m41t00.o
?obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
+obj-$(CONFIG_SENSORS_PCA9539) += pca9539.o
?obj-$(CONFIG_SENSORS_PCF8574) += pcf8574.o
?obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
?obj-$(CONFIG_SENSORS_RTC8564) += rtc8564.o
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c 1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c 2005-06-06 15:15:30.000000000 -0500
@@ -0,0 +1,192 @@
+/*
+ ? ?pca9539.c - 16-bit I/O port with interrupt and reset
+ ? ?
+ ? ?Copyright (C) 2005 Ben Gardner <bgardner@wabtec.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; version 2 of the License.
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c-sensor.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = {0x74, 0x75, 0x76, 0x77, I2C_CLIENT_END};
+static unsigned int normal_isa[] = {I2C_CLIENT_ISA_END};
+
+/* Insmod parameters */
+SENSORS_INSMOD_1(pca9539);
+
+enum pca9539_cmd
+{
+ PCA9539_INPUT_0 ?= 0,
+ PCA9539_INPUT_1 ?= 1,
+ PCA9539_OUTPUT_0 = 2,
+ PCA9539_OUTPUT_1 = 3,
+ PCA9539_INVERT_0 = 4,
+ PCA9539_INVERT_1 = 5,
+ PCA9539_DIRECTION_0 = 6,
+ PCA9539_DIRECTION_1 = 7,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter);
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind);
+static int pca9539_detach_client(struct i2c_client *client);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pca9539_driver = {
+ .owner ?= THIS_MODULE,
+ .name ?= "pca9539",
+ .flags ?= I2C_DF_NOTIFY,
+ .attach_adapter = pca9539_attach_adapter,
+ .detach_client = pca9539_detach_client,
+};
+
+struct pca9539_data {
+ struct i2c_client client;
+};
+
+/* following are the sysfs callback functions */
+static ssize_t pca9539_show(struct device *dev, struct device_attribute *attr,
+ ? ? ? char *buf)
+{
+ struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+ struct i2c_client *client = to_i2c_client(dev);
+ return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(client, 
+ ? ? ? ? ? ?psa->index));
+}
+
+static ssize_t pca9539_store(struct device *dev, struct device_attribute *attr,
+ ? ? ? ?const char *buf, size_t count)
+{
+ struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned long val = simple_strtoul(buf, NULL, 0);
+ if (val > 0xff)
+ ?return -EINVAL;
+ i2c_smbus_write_byte_data(client, psa->index, val);
+ return count;
+}
+
+/* Define the device attributes */
+
+#define PCA9539_ENTRY_RO(name, cmd_idx) \
+ static SENSOR_DEVICE_ATTR(name, S_IRUGO, pca9539_show, NULL, cmd_idx)
+
+#define PCA9539_ENTRY_RW(name, cmd_idx) \
+ static SENSOR_DEVICE_ATTR(name, S_IRUGO | S_IWUSR, pca9539_show, \
+ ? ? ?pca9539_store, cmd_idx)
+
+PCA9539_ENTRY_RO(input0, PCA9539_INPUT_0);
+PCA9539_ENTRY_RO(input1, PCA9539_INPUT_1);
+PCA9539_ENTRY_RW(output0, PCA9539_OUTPUT_0);
+PCA9539_ENTRY_RW(output1, PCA9539_OUTPUT_1);
+PCA9539_ENTRY_RW(invert0, PCA9539_INVERT_0);
+PCA9539_ENTRY_RW(invert1, PCA9539_INVERT_1);
+PCA9539_ENTRY_RW(direction0, PCA9539_DIRECTION_0);
+PCA9539_ENTRY_RW(direction1, PCA9539_DIRECTION_1);
+
+static struct attribute *pca9539_attributes[] = {
+ &sensor_dev_attr_input0.dev_attr.attr,
+ &sensor_dev_attr_input1.dev_attr.attr,
+ &sensor_dev_attr_output0.dev_attr.attr,
+ &sensor_dev_attr_output1.dev_attr.attr,
+ &sensor_dev_attr_invert0.dev_attr.attr,
+ &sensor_dev_attr_invert1.dev_attr.attr,
+ &sensor_dev_attr_direction0.dev_attr.attr,
+ &sensor_dev_attr_direction1.dev_attr.attr,
+ NULL
+};
+
+static struct attribute_group pca9539_defattr_group = {
+ .attrs = pca9539_attributes,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter)
+{
+ return i2c_detect(adapter, &addr_data, pca9539_detect);
+}
+
+/* This function is called by i2c_detect */
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+ struct i2c_client *new_client;
+ struct pca9539_data *data;
+ int err = 0;
+
+ if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+ ?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 = kmalloc(sizeof(struct pca9539_data), GFP_KERNEL))) {
+ ?err = -ENOMEM;
+ ?goto exit;
+ }
+ memset(data, 0, sizeof(struct pca9539_data));
+ 
+ new_client = &data->client;
+ i2c_set_clientdata(new_client, data);
+ new_client->addr = address;
+ new_client->adapter = adapter;
+ new_client->driver = &pca9539_driver;
+ new_client->flags = 0;
+
+ /* Detection: the pca9539 only has 8 registers (0-7).
+ ? ?A read of 7 should succeed, but a read of 8 should fail. */
+ if ((i2c_smbus_read_byte_data(new_client, 7) < 0) ||
+ ? ? (i2c_smbus_read_byte_data(new_client, 8) >= 0))
+ ?goto exit_kfree;
+
+ strlcpy(new_client->name, "pca9539", I2C_NAME_SIZE);
+
+ /* Tell the I2C layer a new client has arrived */
+ if ((err = i2c_attach_client(new_client)))
+ ?goto exit_kfree;
+
+ /* Register sysfs hooks (don't care about failure) */
+ sysfs_create_group(&new_client->dev.kobj, &pca9539_defattr_group);
+
+ return 0;
+
+exit_kfree:
+ kfree(data);
+exit:
+ return err;
+}
+
+static int pca9539_detach_client(struct i2c_client *client)
+{
+ int err;
+
+ if ((err = i2c_detach_client(client))) {
+ ?dev_err(&client->dev, "Client deregistration failed.\n");
+ ?return err;
+ }
+
+ kfree(i2c_get_clientdata(client));
+ return 0;
+}
+
+static int __init pca9539_init(void)
+{
+ return i2c_add_driver(&pca9539_driver);
+}
+
+static void __exit pca9539_exit(void)
+{
+ i2c_del_driver(&pca9539_driver);
+}
+
+MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
+MODULE_DESCRIPTION("PCA9539 driver");
+MODULE_LICENSE("GPL");
+
+module_init(pca9539_init);
+module_exit(pca9539_exit);
+
--- linux-2.6.12-rc5-mm2-clean/Documentation/i2c/chips/pca9539 1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2/Documentation/i2c/chips/pca9539 2005-06-06 15:09:25.161264937 -0500
@@ -0,0 +1,47 @@
+Kernel driver pca9539
+==========+
+Supported chips:
+ ?* Philips PCA9539
+ ? ?Prefix: 'pca9539'
+ ? ?Addresses scanned: 0x74 - 0x77
+ ? ?Datasheet:
+ ? ? ? ?http://www.semiconductors.philips.com/acrobat/datasheets/PCA9539_2.pdf
+
+Author: Ben Gardner <bgardner@wabtec.com>
+
+
+Description
+-----------
+
+The Philips PCA9539 is a 16 bit low power I/O device.
+All 16 lines can be individually configured as an input or output.
+The input sense can also be inverted.
+The 16 lines are split between two bytes.
+
+
+Sysfs entries
+-------------
+
+Each is a byte that maps to the 8 I/O bits.
+A '0' suffix is for bits 0-7, while '1' is for bits 8-15.
+
+input[01] ? ? - read the current value
+output[01] ? ?- sets the output value
+direction[01] - direction of each bit: 1=input, 0=output
+invert[01] ? ?- toggle the input bit sense
+
+input reads the actual state of the line and is always available.
+The direction defaults to input for all channels.
+
+
+General Remarks
+---------------
+
+Note that each output, direction, and invert entry controls 8 lines.
+You should use the read, modify, write sequence.
+For example. to set output bit 0 of 1.
+ ?val=$(cat output0)
+ ?val=$(( $val | 1 ))
+ ?echo $val > output0
+

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[BGardner]

Lets try that again with the attachement, since I've had 0 luck sending
inline.

=
Greg,

Attached (due to email line-wrapping problems) is an updated patch for
the Philips PCA9539.
I think this one is now ready for inclusion.

Thanks,
Ben

=
This is a driver for the PCA9539, a 16 bit digital I/O chip.
It uses the new i2c-sysfs interfaces.
The patch includes documentation.
It depends on the patch that renames "i2c-sysfs.h" to "hwmon-sysfs.h"

Signed-off-by: Ben Gardner <bgardner@wabtec.com>
-------------- next part --------------
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig	2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig	2005-06-06 15:24:28.961183516 -0500
@@ -440,6 +440,16 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called pcf8574.
 
+config SENSORS_PCA9539
+	tristate "Philips PCA9539 16-bit I/O port"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the Philips PCA9539
+	  16-bit I/O port.
+          
+	  This driver can also be built as a module.  If so, the module
+	  will be called pca9539.
+
 config SENSORS_PCF8591
 	tristate "Philips PCF8591"
 	depends on I2C && EXPERIMENTAL
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Makefile	2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Makefile	2005-06-03 12:09:41.000000000 -0500
@@ -34,6 +34,7 @@
 obj-$(CONFIG_SENSORS_MAX1619)	+= max1619.o
 obj-$(CONFIG_SENSORS_M41T00)	+= m41t00.o
 obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
+obj-$(CONFIG_SENSORS_PCA9539)	+= pca9539.o
 obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_RTC8564)	+= rtc8564.o
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c	2005-06-06 15:15:30.000000000 -0500
@@ -0,0 +1,192 @@
+/*
+    pca9539.c - 16-bit I/O port with interrupt and reset
+    
+    Copyright (C) 2005 Ben Gardner <bgardner@wabtec.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; version 2 of the License.
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c-sensor.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = {0x74, 0x75, 0x76, 0x77, I2C_CLIENT_END};
+static unsigned int normal_isa[] = {I2C_CLIENT_ISA_END};
+
+/* Insmod parameters */
+SENSORS_INSMOD_1(pca9539);
+
+enum pca9539_cmd
+{
+	PCA9539_INPUT_0		= 0,
+	PCA9539_INPUT_1		= 1,
+	PCA9539_OUTPUT_0	= 2,
+	PCA9539_OUTPUT_1	= 3,
+	PCA9539_INVERT_0	= 4,
+	PCA9539_INVERT_1	= 5,
+	PCA9539_DIRECTION_0	= 6,
+	PCA9539_DIRECTION_1	= 7,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter);
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind);
+static int pca9539_detach_client(struct i2c_client *client);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pca9539_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "pca9539",
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= pca9539_attach_adapter,
+	.detach_client	= pca9539_detach_client,
+};
+
+struct pca9539_data {
+	struct i2c_client client;
+};
+
+/* following are the sysfs callback functions */
+static ssize_t pca9539_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(client, 
+							     psa->index));
+}
+
+static ssize_t pca9539_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	unsigned long val = simple_strtoul(buf, NULL, 0);
+	if (val > 0xff)
+		return -EINVAL;
+	i2c_smbus_write_byte_data(client, psa->index, val);
+	return count;
+}
+
+/* Define the device attributes */
+
+#define PCA9539_ENTRY_RO(name, cmd_idx) \
+	static SENSOR_DEVICE_ATTR(name, S_IRUGO, pca9539_show, NULL, cmd_idx)
+
+#define PCA9539_ENTRY_RW(name, cmd_idx) \
+	static SENSOR_DEVICE_ATTR(name, S_IRUGO | S_IWUSR, pca9539_show, \
+				  pca9539_store, cmd_idx)
+
+PCA9539_ENTRY_RO(input0, PCA9539_INPUT_0);
+PCA9539_ENTRY_RO(input1, PCA9539_INPUT_1);
+PCA9539_ENTRY_RW(output0, PCA9539_OUTPUT_0);
+PCA9539_ENTRY_RW(output1, PCA9539_OUTPUT_1);
+PCA9539_ENTRY_RW(invert0, PCA9539_INVERT_0);
+PCA9539_ENTRY_RW(invert1, PCA9539_INVERT_1);
+PCA9539_ENTRY_RW(direction0, PCA9539_DIRECTION_0);
+PCA9539_ENTRY_RW(direction1, PCA9539_DIRECTION_1);
+
+static struct attribute *pca9539_attributes[] = {
+	&sensor_dev_attr_input0.dev_attr.attr,
+	&sensor_dev_attr_input1.dev_attr.attr,
+	&sensor_dev_attr_output0.dev_attr.attr,
+	&sensor_dev_attr_output1.dev_attr.attr,
+	&sensor_dev_attr_invert0.dev_attr.attr,
+	&sensor_dev_attr_invert1.dev_attr.attr,
+	&sensor_dev_attr_direction0.dev_attr.attr,
+	&sensor_dev_attr_direction1.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group pca9539_defattr_group = {
+	.attrs = pca9539_attributes,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_detect(adapter, &addr_data, pca9539_detect);
+}
+
+/* This function is called by i2c_detect */
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct pca9539_data *data;
+	int err = 0;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		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 = kmalloc(sizeof(struct pca9539_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct pca9539_data));
+	
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &pca9539_driver;
+	new_client->flags = 0;
+
+	/* Detection: the pca9539 only has 8 registers (0-7).
+	   A read of 7 should succeed, but a read of 8 should fail. */
+	if ((i2c_smbus_read_byte_data(new_client, 7) < 0) ||
+	    (i2c_smbus_read_byte_data(new_client, 8) >= 0))
+		goto exit_kfree;
+
+	strlcpy(new_client->name, "pca9539", I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_kfree;
+
+	/* Register sysfs hooks (don't care about failure) */
+	sysfs_create_group(&new_client->dev.kobj, &pca9539_defattr_group);
+
+	return 0;
+
+exit_kfree:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int pca9539_detach_client(struct i2c_client *client)
+{
+	int err;
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev, "Client deregistration failed.\n");
+		return err;
+	}
+
+	kfree(i2c_get_clientdata(client));
+	return 0;
+}
+
+static int __init pca9539_init(void)
+{
+	return i2c_add_driver(&pca9539_driver);
+}
+
+static void __exit pca9539_exit(void)
+{
+	i2c_del_driver(&pca9539_driver);
+}
+
+MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
+MODULE_DESCRIPTION("PCA9539 driver");
+MODULE_LICENSE("GPL");
+
+module_init(pca9539_init);
+module_exit(pca9539_exit);
+
--- linux-2.6.12-rc5-mm2-clean/Documentation/i2c/chips/pca9539	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2/Documentation/i2c/chips/pca9539	2005-06-06 15:09:25.161264937 -0500
@@ -0,0 +1,47 @@
+Kernel driver pca9539
+==========+
+Supported chips:
+  * Philips PCA9539
+    Prefix: 'pca9539'
+    Addresses scanned: 0x74 - 0x77
+    Datasheet:
+        http://www.semiconductors.philips.com/acrobat/datasheets/PCA9539_2.pdf
+
+Author: Ben Gardner <bgardner@wabtec.com>
+
+
+Description
+-----------
+
+The Philips PCA9539 is a 16 bit low power I/O device.
+All 16 lines can be individually configured as an input or output.
+The input sense can also be inverted.
+The 16 lines are split between two bytes.
+
+
+Sysfs entries
+-------------
+
+Each is a byte that maps to the 8 I/O bits.
+A '0' suffix is for bits 0-7, while '1' is for bits 8-15.
+
+input[01]     - read the current value
+output[01]    - sets the output value
+direction[01] - direction of each bit: 1=input, 0=output
+invert[01]    - toggle the input bit sense
+
+input reads the actual state of the line and is always available.
+The direction defaults to input for all channels.
+
+
+General Remarks
+---------------
+
+Note that each output, direction, and invert entry controls 8 lines.
+You should use the read, modify, write sequence.
+For example. to set output bit 0 of 1.
+  val=$(cat output0)
+  val=$(( $val | 1 ))
+  echo $val > output0
+

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c drvier

[Greg KH]

On Mon, Jun 06, 2005 at 07:15:31PM -0400, BGardner@Wabtec.com wrote:
> 
> Lets try that again with the attachement, since I've had 0 luck sending
> inline.

Content-Type: text/plain;
        name="pca9539.diff.txt"
Content-Transfer-Encoding: base64
Content-Description: pca9539.diff.txt
Content-Disposition: attachment;
        filename="pca9539.diff.txt"
		
base64 is nasty.  I can't just pipe that email through patch that way...

thanks,

greg k-h

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c driver (retry 9

[bgardner]

Greg,

I (finally) figured out how to send an attachment that is NOT Base64 encoded.
Patch should be happy with this email. 
Thanks for your patience. =)

Ben

=
This is an i2c driver for the Philips PCA9539 (16 bit I/O port).
It uses the new i2c-sysfs interfaces.
The patch includes documentation.
It depends on the patch that renames "i2c-sysfs.h" to "hwmon-sysfs.h"

Signed-off-by: Ben Gardner <bgardner@wabtec.com>
-------------- next part --------------
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Kconfig	2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Kconfig	2005-06-06 15:24:28.961183516 -0500
@@ -440,6 +440,16 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called pcf8574.
 
+config SENSORS_PCA9539
+	tristate "Philips PCA9539 16-bit I/O port"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the Philips PCA9539
+	  16-bit I/O port.
+          
+	  This driver can also be built as a module.  If so, the module
+	  will be called pca9539.
+
 config SENSORS_PCF8591
 	tristate "Philips PCF8591"
 	depends on I2C && EXPERIMENTAL
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/Makefile	2005-06-01 14:29:17.000000000 -0500
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/Makefile	2005-06-03 12:09:41.000000000 -0500
@@ -34,6 +34,7 @@
 obj-$(CONFIG_SENSORS_MAX1619)	+= max1619.o
 obj-$(CONFIG_SENSORS_M41T00)	+= m41t00.o
 obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
+obj-$(CONFIG_SENSORS_PCA9539)	+= pca9539.o
 obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_RTC8564)	+= rtc8564.o
--- linux-2.6.12-rc5-mm2-clean/drivers/i2c/chips/pca9539.c	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2-pca9539/drivers/i2c/chips/pca9539.c	2005-06-06 15:15:30.000000000 -0500
@@ -0,0 +1,192 @@
+/*
+    pca9539.c - 16-bit I/O port with interrupt and reset
+    
+    Copyright (C) 2005 Ben Gardner <bgardner@wabtec.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; version 2 of the License.
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/i2c-sensor.h>
+
+/* Addresses to scan */
+static unsigned short normal_i2c[] = {0x74, 0x75, 0x76, 0x77, I2C_CLIENT_END};
+static unsigned int normal_isa[] = {I2C_CLIENT_ISA_END};
+
+/* Insmod parameters */
+SENSORS_INSMOD_1(pca9539);
+
+enum pca9539_cmd
+{
+	PCA9539_INPUT_0		= 0,
+	PCA9539_INPUT_1		= 1,
+	PCA9539_OUTPUT_0	= 2,
+	PCA9539_OUTPUT_1	= 3,
+	PCA9539_INVERT_0	= 4,
+	PCA9539_INVERT_1	= 5,
+	PCA9539_DIRECTION_0	= 6,
+	PCA9539_DIRECTION_1	= 7,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter);
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind);
+static int pca9539_detach_client(struct i2c_client *client);
+
+/* This is the driver that will be inserted */
+static struct i2c_driver pca9539_driver = {
+	.owner		= THIS_MODULE,
+	.name		= "pca9539",
+	.flags		= I2C_DF_NOTIFY,
+	.attach_adapter	= pca9539_attach_adapter,
+	.detach_client	= pca9539_detach_client,
+};
+
+struct pca9539_data {
+	struct i2c_client client;
+};
+
+/* following are the sysfs callback functions */
+static ssize_t pca9539_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	return sprintf(buf, "%d\n", i2c_smbus_read_byte_data(client, 
+							     psa->index));
+}
+
+static ssize_t pca9539_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	struct sensor_device_attribute *psa = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	unsigned long val = simple_strtoul(buf, NULL, 0);
+	if (val > 0xff)
+		return -EINVAL;
+	i2c_smbus_write_byte_data(client, psa->index, val);
+	return count;
+}
+
+/* Define the device attributes */
+
+#define PCA9539_ENTRY_RO(name, cmd_idx) \
+	static SENSOR_DEVICE_ATTR(name, S_IRUGO, pca9539_show, NULL, cmd_idx)
+
+#define PCA9539_ENTRY_RW(name, cmd_idx) \
+	static SENSOR_DEVICE_ATTR(name, S_IRUGO | S_IWUSR, pca9539_show, \
+				  pca9539_store, cmd_idx)
+
+PCA9539_ENTRY_RO(input0, PCA9539_INPUT_0);
+PCA9539_ENTRY_RO(input1, PCA9539_INPUT_1);
+PCA9539_ENTRY_RW(output0, PCA9539_OUTPUT_0);
+PCA9539_ENTRY_RW(output1, PCA9539_OUTPUT_1);
+PCA9539_ENTRY_RW(invert0, PCA9539_INVERT_0);
+PCA9539_ENTRY_RW(invert1, PCA9539_INVERT_1);
+PCA9539_ENTRY_RW(direction0, PCA9539_DIRECTION_0);
+PCA9539_ENTRY_RW(direction1, PCA9539_DIRECTION_1);
+
+static struct attribute *pca9539_attributes[] = {
+	&sensor_dev_attr_input0.dev_attr.attr,
+	&sensor_dev_attr_input1.dev_attr.attr,
+	&sensor_dev_attr_output0.dev_attr.attr,
+	&sensor_dev_attr_output1.dev_attr.attr,
+	&sensor_dev_attr_invert0.dev_attr.attr,
+	&sensor_dev_attr_invert1.dev_attr.attr,
+	&sensor_dev_attr_direction0.dev_attr.attr,
+	&sensor_dev_attr_direction1.dev_attr.attr,
+	NULL
+};
+
+static struct attribute_group pca9539_defattr_group = {
+	.attrs = pca9539_attributes,
+};
+
+static int pca9539_attach_adapter(struct i2c_adapter *adapter)
+{
+	return i2c_detect(adapter, &addr_data, pca9539_detect);
+}
+
+/* This function is called by i2c_detect */
+static int pca9539_detect(struct i2c_adapter *adapter, int address, int kind)
+{
+	struct i2c_client *new_client;
+	struct pca9539_data *data;
+	int err = 0;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		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 = kmalloc(sizeof(struct pca9539_data), GFP_KERNEL))) {
+		err = -ENOMEM;
+		goto exit;
+	}
+	memset(data, 0, sizeof(struct pca9539_data));
+	
+	new_client = &data->client;
+	i2c_set_clientdata(new_client, data);
+	new_client->addr = address;
+	new_client->adapter = adapter;
+	new_client->driver = &pca9539_driver;
+	new_client->flags = 0;
+
+	/* Detection: the pca9539 only has 8 registers (0-7).
+	   A read of 7 should succeed, but a read of 8 should fail. */
+	if ((i2c_smbus_read_byte_data(new_client, 7) < 0) ||
+	    (i2c_smbus_read_byte_data(new_client, 8) >= 0))
+		goto exit_kfree;
+
+	strlcpy(new_client->name, "pca9539", I2C_NAME_SIZE);
+
+	/* Tell the I2C layer a new client has arrived */
+	if ((err = i2c_attach_client(new_client)))
+		goto exit_kfree;
+
+	/* Register sysfs hooks (don't care about failure) */
+	sysfs_create_group(&new_client->dev.kobj, &pca9539_defattr_group);
+
+	return 0;
+
+exit_kfree:
+	kfree(data);
+exit:
+	return err;
+}
+
+static int pca9539_detach_client(struct i2c_client *client)
+{
+	int err;
+
+	if ((err = i2c_detach_client(client))) {
+		dev_err(&client->dev, "Client deregistration failed.\n");
+		return err;
+	}
+
+	kfree(i2c_get_clientdata(client));
+	return 0;
+}
+
+static int __init pca9539_init(void)
+{
+	return i2c_add_driver(&pca9539_driver);
+}
+
+static void __exit pca9539_exit(void)
+{
+	i2c_del_driver(&pca9539_driver);
+}
+
+MODULE_AUTHOR("Ben Gardner <bgardner@wabtec.com>");
+MODULE_DESCRIPTION("PCA9539 driver");
+MODULE_LICENSE("GPL");
+
+module_init(pca9539_init);
+module_exit(pca9539_exit);
+
--- linux-2.6.12-rc5-mm2-clean/Documentation/i2c/chips/pca9539	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.12-rc5-mm2/Documentation/i2c/chips/pca9539	2005-06-06 15:09:25.161264937 -0500
@@ -0,0 +1,47 @@
+Kernel driver pca9539
+==========+
+Supported chips:
+  * Philips PCA9539
+    Prefix: 'pca9539'
+    Addresses scanned: 0x74 - 0x77
+    Datasheet:
+        http://www.semiconductors.philips.com/acrobat/datasheets/PCA9539_2.pdf
+
+Author: Ben Gardner <bgardner@wabtec.com>
+
+
+Description
+-----------
+
+The Philips PCA9539 is a 16 bit low power I/O device.
+All 16 lines can be individually configured as an input or output.
+The input sense can also be inverted.
+The 16 lines are split between two bytes.
+
+
+Sysfs entries
+-------------
+
+Each is a byte that maps to the 8 I/O bits.
+A '0' suffix is for bits 0-7, while '1' is for bits 8-15.
+
+input[01]     - read the current value
+output[01]    - sets the output value
+direction[01] - direction of each bit: 1=input, 0=output
+invert[01]    - toggle the input bit sense
+
+input reads the actual state of the line and is always available.
+The direction defaults to input for all channels.
+
+
+General Remarks
+---------------
+
+Note that each output, direction, and invert entry controls 8 lines.
+You should use the read, modify, write sequence.
+For example. to set output bit 0 of 1.
+  val=$(cat output0)
+  val=$(( $val | 1 ))
+  echo $val > output0
+

[lm-sensors] [PATCH 2.6.12-rc5-mm2] pca9539: new i2c driver

[Jean Delvare]

> I (finally) figured out how to send an attachment that is NOT Base64
> encoded. Patch should be happy with this email. 
> Thanks for your patience. =)

This one is fine with me, and applies just fine on top of my tree as
well :)

-- 
Jean Delvare