Skip to contentSkip to conversation

New chip driver: pcf8591

PATCH New Aurelien Jarno 3 participants 5 messages May 19, 2005

lore
Share & tools
lore

Aurelien Jarno

author
#0 lore raw

PATCH 1/1 New chip driver: pcf8591

Hi Greg,

Please find below a patch against kernel 2.6.5-rc2-mm4 to add the pcf8591 driver (a 8-bit A/D and D/A converter). I have ported it from the 2.4 version, and it includes some fixes, improvements and simplifications.

It has been reviewed by Jean Delvare on IRC.

Please also note that the patch also fixes a missing space in drivers/i2c/chips/Kconfig, introduced by the previous patch I sent you concerning the pcf8574.

Thanks,
Aurelien

drivers/i2c/chips/Kconfig +11 -1
diff -urN linux-2.6.5-rc2-mm4.orig/drivers/i2c/chips/Kconfig linux-2.6.5-rc2-mm4/drivers/i2c/chips/Kconfig
@@ -217,7 +217,17 @@
 	  If you say yes here you get support for Philips PCF8574 and 
 	  PCF8574A chips.
 
-	  This driver can also be built as a module. If so, the module
+	  This driver can also be built as a module.  If so, the module
 	  will be called pcf8574.
 
+config SENSORS_PCF8591
+	tristate "Philips PCF8591"
+	depends on I2C && EXPERIMENTAL
+	select I2C_SENSOR
+	help
+	  If you say yes here you get support for Philips PCF8591 chips.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called pcf8591.
+
 endmenu
drivers/i2c/chips/Makefile +1 -0
diff -urN linux-2.6.5-rc2-mm4.orig/drivers/i2c/chips/Makefile linux-2.6.5-rc2-mm4/drivers/i2c/chips/Makefile
@@ -20,6 +20,7 @@
 obj-$(CONFIG_SENSORS_LM85)	+= lm85.o
 obj-$(CONFIG_SENSORS_LM90)	+= lm90.o
 obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
+obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_VIA686A)	+= via686a.o
 obj-$(CONFIG_SENSORS_W83L785TS)	+= w83l785ts.o
drivers/i2c/chips/pcf8591.c +321 -0

321 lines not on this page · raw message

#1 lore raw

PATCH 1/1 New chip driver: pcf8591

Quoted 11 lines · Aurelien Jarno

Hi Greg,

Please find below a patch against kernel 2.6.5-rc2-mm4 to add the pcf8591 driver (a 8-bit A/D and D/A converter). I have ported it from the 2.4 version, and it includes some fixes, improvements and simplifications.

It has been reviewed by Jean Delvare on IRC.

Please also note that the patch also fixes a missing space in drivers/i2c/chips/Kconfig, introduced by the previous patch I sent you concerning the pcf8574.

Sorry for the delay, this has now been applied, along with a few formatting changes, and one bugfix:

drivers/i2c/chips/pcf8591.c line 134

Quoted 12 lines
drivers/i2c/chips/pcf8591.c +12 -0
+static ssize_t set_out0_output(struct device *dev, const char *buf, size_t count)
+{
+	unsigned int value;
+	struct i2c_client *client = to_i2c_client(dev);
+	struct pcf8591_data *data = i2c_get_clientdata(client);
+	if ((value = (simple_strtoul(buf, NULL, 10) + 5) / 10) <= 255);
+	{
+		data->aout = value;
+		i2c_smbus_write_byte_data(client, data->control, data->aout);
+	}
+	return count;
+}

I fixed that if statement above (you really didn't want that ';' there, right?

thanks,

greg k-h

#2 lore raw

PATCH 1/1 New chip driver: pcf8591

In reply to Aurelien Jarno
Quoted 3 lines · Jean Delvare

BTW, wouldn't a "else return -1" make sense here? Silently ignoring errors doesn't sound good to me.

Well make it a real error, like -EINVAL or something, so userspace has a chance to report it properly. Not just -1.

thanks,

greg k-h

#3 lore raw

PATCH 1/1 New chip driver: pcf8591

In reply to Aurelien Jarno
Quoted 18 lines

Sorry for the delay, this has now been applied, along with a few formatting changes, and one bugfix:

drivers/i2c/chips/pcf8591.c line 134

Quoted 12 lines

+static ssize_t set_out0_output(struct device *dev, const char *buf, size_t count)+{

drivers/i2c/chips/pcf8591.c +10 -0
+	unsigned int value;
+	struct i2c_client *client = to_i2c_client(dev);
+	struct pcf8591_data *data = i2c_get_clientdata(client);
+	if ((value = (simple_strtoul(buf, NULL, 10) + 5) / 10) <= 255);
+	{
+		data->aout = value;
+		i2c_smbus_write_byte_data(client, data->control, data->aout);
+	}
+	return count;
+}

I fixed that if statement above (you really didn't want that ';' there, right?

OMG, we missed that...

Nice catching Greg! :)

BTW, wouldn't a "else return -1" make sense here? Silently ignoring errors doesn't sound good to me.

Sorry for not noticing this before.

Signature · 2 lines
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/
#4 lore raw

PATCH 1/1 New chip driver: pcf8591

In reply to Aurelien Jarno
Quoted 5 lines
Quoted 2 lines

BTW, wouldn't a "else return -1" make sense here? Silently ignoring errors doesn't sound good to me.

Well make it a real error, like -EINVAL or something, so userspace has a chance to report it properly. Not just -1.

Oh, I guess we want to correct it87.c and fscher.c in that way too then.

I had this to my todo list.

Signature · 2 lines
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/