Cleanup I2C handling

* Move OS specific code to new files i2c.h and i2c.c
* As side effect a likely bug (linux only code) in the existing driver
  has been fixed.

Compile tested on Debian/i386 and OpenWRT/arm.

Signed-off-by: Harald Geyer <harald@ccbib.org>

acinclude.m4

@@ -231,6 +231,9 @@ dnl			else
 			if test "$x_ac_have_i2c" = yes; then
 				HD44780_DRIVERS="$HD44780_DRIVERS hd44780-hd44780-i2c.o"
 				HD44780_DRIVERS="$HD44780_DRIVERS hd44780-hd44780-piplate.o"
+				HD44780_I2C="i2c.o"
+			else
+				HD44780_I2C=""
 			fi
 			if test "$x_ac_have_spi" = yes; then
 				HD44780_DRIVERS="$HD44780_DRIVERS hd44780-hd44780-spi.o hd44780-hd44780-pifacecad.o"
@@ -563,6 +566,7 @@ AC_SUBST(LIBLIRC_CLIENT)
 AC_SUBST(LIBSVGA)
 AC_SUBST(DRIVERS)
 AC_SUBST(HD44780_DRIVERS)
+AC_SUBST(HD44780_I2C)
 AC_SUBST(GLCD_DRIVERS)
 AC_SUBST(LIBG15)
 AC_SUBST(LIBGLCD)

server/drivers/Makefile.am

@@ -48,8 +48,8 @@ glcd_LDADD =         libLCD.a @GLCD_DRIVERS@ @FT2_LIBS@ @LIBPNG_LIBS@ @LIBSERDIS
 glcd_DEPENDENCIES =  @GLCD_DRIVERS@ glcd-glcd-render.o
 glcdlib_LDADD =      @LIBGLCD@
 glk_LDADD =          libbignum.a
-hd44780_LDADD =      libLCD.a @HD44780_DRIVERS@ @LIBUSB_LIBS@ @LIBFTDI_LIBS@ libbignum.a
+hd44780_LDADD =      libLCD.a @HD44780_DRIVERS@ @HD44780_I2C@ @LIBUSB_LIBS@ @LIBFTDI_LIBS@ libbignum.a
-hd44780_DEPENDENCIES = @HD44780_DRIVERS@
+hd44780_DEPENDENCIES = @HD44780_DRIVERS@ @HD44780_I2C@
 i2500vfd_LDADD =     @LIBFTDI_LIBS@
 imon_LDADD =         libLCD.a libbignum.a
 imonlcd_LDADD =      libLCD.a
@@ -94,8 +94,8 @@ glcd_SOURCES =       lcd.h report.h glcd_drv.c glcd_drv.h glcd-low.h glcd-driver
 EXTRA_glcd_SOURCES = glcd-t6963.c t6963_low.c t6963_low.h glcd-png.c glcd-serdisp.c glcd-glcd2usb.c glcd-glcd2usb.h glcd-x11.c glcd-picolcdgfx.c
 glcdlib_SOURCES =    lcd.h lcd_lib.h glcdlib.h glcdlib.c report.h
 glk_SOURCES =        lcd.h glk.c glk.h glkproto.c glkproto.h report.h
-hd44780_SOURCES =    lcd.h lcd_lib.h hd44780.h hd44780.c hd44780-drivers.h hd44780-low.h hd44780-charmap.h report.h adv_bignum.h
+hd44780_SOURCES =    lcd.h lcd_lib.h hd44780.h hd44780.c hd44780-drivers.h hd44780-low.h hd44780-charmap.h report.h adv_bignum.h i2c.h
-EXTRA_hd44780_SOURCES = port.h lpt-port.h timing.h lcd_sem.c lcd_sem.h hd44780-4bit.c hd44780-4bit.h hd44780-bwct-usb.c hd44780-bwct-usb.h hd44780-ethlcd.c hd44780-ethlcd.h hd44780-ext8bit.c hd44780-ext8bit.h hd44780-ftdi.c hd44780-ftdi.h hd44780-i2c.c hd44780-i2c.h hd44780-lcd2usb.c hd44780-lcd2usb.h hd44780-lis2.c hd44780-lis2.h hd44780-pifacecad.c hd44780-pifacecad.h hd44780-piplate.c hd44780-piplate.h hd44780-rpi.c hd44780-rpi.h hd44780-serial.c hd44780-serial.h hd44780-serialLpt.c hd44780-serialLpt.h hd44780-spi.c hd44780-spi.h hd44780-usb4all.c hd44780-usb4all.h hd44780-usblcd.c hd44780-usblcd.h hd44780-usbtiny.c hd44780-usbtiny.h hd44780-uss720.c hd44780-uss720.h hd44780-winamp.c hd44780-winamp.h  hd44780-lcm162.c hd44780-lcm162.h
+EXTRA_hd44780_SOURCES = port.h lpt-port.h timing.h i2c.c lcd_sem.c lcd_sem.h hd44780-4bit.c hd44780-4bit.h hd44780-bwct-usb.c hd44780-bwct-usb.h hd44780-ethlcd.c hd44780-ethlcd.h hd44780-ext8bit.c hd44780-ext8bit.h hd44780-ftdi.c hd44780-ftdi.h hd44780-i2c.c hd44780-i2c.h hd44780-lcd2usb.c hd44780-lcd2usb.h hd44780-lis2.c hd44780-lis2.h hd44780-pifacecad.c hd44780-pifacecad.h hd44780-piplate.c hd44780-piplate.h hd44780-rpi.c hd44780-rpi.h hd44780-serial.c hd44780-serial.h hd44780-serialLpt.c hd44780-serialLpt.h hd44780-spi.c hd44780-spi.h hd44780-usb4all.c hd44780-usb4all.h hd44780-usblcd.c hd44780-usblcd.h hd44780-usbtiny.c hd44780-usbtiny.h hd44780-uss720.c hd44780-uss720.h hd44780-winamp.c hd44780-winamp.h  hd44780-lcm162.c hd44780-lcm162.h
 i2500vfd_SOURCES =   lcd.h i2500vfd.c i2500vfd.h glcd_font5x8.h report.h
 icp_a106_SOURCES =   lcd.h lcd_lib.h icp_a106.c icp_a106.h report.h
 imon_SOURCES =       lcd.h lcd_lib.h hd44780-charmap.h imon.h imon.c report.h adv_bignum.h

server/drivers/hd44780-i2c.c

@@ -82,19 +82,8 @@
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
-#include <fcntl.h>
-#include <sys/ioctl.h>
-
-#ifdef HAVE_DEV_IICBUS_IIC_H
-#include <dev/iicbus/iic.h>
-#else /* HAVE_LINUX_I2C_DEV_H */
-#include <linux/i2c-dev.h>
-/* I2C_SLAVE is missing in linux/i2c-dev.h from kernel headers of 2.4.x kernels */
-#ifndef I2C_SLAVE
-#define I2C_SLAVE 0x0703  /* Change slave address */
-#endif
-#endif
 
+#include "i2c.h"
 
 // Generally, any function that accesses the LCD control lines needs to be
 // implemented separately for each HW design. This is typically (but not
@@ -123,13 +112,9 @@ void i2c_HD44780_close(PrivateData *p);
 static void
 i2c_out(PrivateData *p, unsigned char val)
 {
-	char data[2];
+	unsigned char data[2];
 	int datalen;
 	static int no_more_errormsgs=0;
-#ifdef HAVE_DEV_IICBUS_IIC_H
-	struct iiccmd cmd;
-	bzero(&cmd, sizeof(cmd));
-#endif
 
 	if (p->port & I2C_PCAX_MASK) { // we have a PCA9554 or similar, that needs a 2-byte command
 		data[0]=1; // command: read/write output port register
@@ -140,24 +125,13 @@ i2c_out(PrivateData *p, unsigned char val)
 		datalen=1;
 	}
 
-#ifdef HAVE_DEV_IICBUS_IIC_H
+	if (i2c_write(p->i2c, data, datalen) < 0) {
-	cmd.slave = (p->port & I2C_ADDR_MASK) << 1;
+		p->hd44780_functions->drv_report(no_more_errormsgs?RPT_DEBUG:RPT_ERR, "HD44780: I2C: i2c write data %u failed: %s",
-	cmd.last = 1;
+			val, strerror(errno));
-	cmd.count = datalen;
-	cmd.buf = data;
-
-	if (ioctl(p->fd, I2CWRITE, &cmd) < 0) {
-#else /* HAVE_LINUX_I2C_DEV_H */
-	if (write(p->fd,data,datalen) != datalen) {
-#endif
-		p->hd44780_functions->drv_report(no_more_errormsgs?RPT_DEBUG:RPT_ERR, "HD44780: I2C: i2c write data %u to address 0x%02X failed: %s",
-			val, p->port & I2C_ADDR_MASK, strerror(errno));
 		no_more_errormsgs=1;
 	}
 }
 
-#define DEFAULT_DEVICE		"/dev/i2c-0"
-
 
 /**
  * Initialize the driver.
@@ -170,6 +144,7 @@ hd_init_i2c(Driver *drvthis)
 {
 	PrivateData *p = (PrivateData*) drvthis->private_data;
 	HD44780_functions *hd44780_functions = p->hd44780_functions;
+	char device[256] = I2C_DEFAULT_DEVICE;
 
 	p->i2c_backlight_invert = drvthis->config_get_bool(drvthis->name, "BacklightInvert", 0, BL_INVERT);
 	p->i2c_line_RS = drvthis->config_get_int(drvthis->name, "i2c_line_RS", 0, RS);
@@ -192,7 +167,6 @@ hd_init_i2c(Driver *drvthis)
 	report(RPT_INFO, "HD44780: I2C: Pin D7 mapped to 0x%02X", p->i2c_line_D7);
 	report(RPT_INFO, "HD44780: I2C: Invert Backlight %d", p->i2c_backlight_invert);
 	
-    char device[256] = DEFAULT_DEVICE;
 #ifdef HAVE_DEV_IICBUS_IIC_H
 	struct iiccmd cmd;
 	bzero(&cmd, sizeof(cmd));
@@ -203,48 +177,27 @@ hd_init_i2c(Driver *drvthis)
 	/* READ CONFIG FILE */
 
 	/* Get serial device to use */
-	strncpy(device, drvthis->config_get_string(drvthis->name, "Device", 0, DEFAULT_DEVICE), sizeof(device));
+	strncpy(device, drvthis->config_get_string(drvthis->name, "Device", 0, I2C_DEFAULT_DEVICE), sizeof(device));
 	device[sizeof(device)-1] = '\0';
 	report(RPT_INFO,"HD44780: I2C: Using device '%s' and address 0x%02X for a %s",
 		device, p->port & I2C_ADDR_MASK, (p->port & I2C_PCAX_MASK) ? "PCA9554(A)" : "PCF8574(A)");
 
-	// Open the I2C device
+	p->i2c = i2c_open(device, p->port & I2C_ADDR_MASK);
-	p->fd = open(device, O_RDWR);
+	if (!p->i2c) {
-	if (p->fd < 0) {
+		report(RPT_ERR, "HD44780: I2C: connecting to device '%s' slave 0x%02X failed:", device, p->port & I2C_ADDR_MASK, strerror(errno));
-		report(RPT_ERR, "HD44780: I2C: open i2c device '%s' failed: %s", device, strerror(errno));
 		return(-1);
 	}
 
-	// Set I2C address
-#ifdef HAVE_DEV_IICBUS_IIC_H
-	cmd.slave = (p->port & I2C_ADDR_MASK) << 1;
-	cmd.last = 0;
-	cmd.count = 0;
-	if (ioctl(p->fd, I2CRSTCARD, &cmd) < 0) {
-		report(RPT_ERR, "HD44780: I2C: reset bus failed: %s", strerror(errno));
-		return -1;
-	}
-	if (ioctl(p->fd, I2CSTART, &cmd) < 0) {
-		report(RPT_ERR, "HD44780: I2C: set address to 0x%02X: %s", p->port & I2C_ADDR_MASK, strerror(errno));
-		return -1;
-	}
-#else /* HAVE_LINUX_I2C_DEV_H */
-	if (ioctl(p->fd,I2C_SLAVE, p->port & I2C_ADDR_MASK) < 0) {
-		report(RPT_ERR, "HD44780: I2C: set address to '%i': %s", p->port & I2C_ADDR_MASK, strerror(errno));
-		return(-1);
-	}
-#endif
-
 	if (p->port & I2C_PCAX_MASK) { // we have a PCA9554 or similar, that needs special config
-		char data[2];
+		unsigned char data[2];
 		data[0] = 2; // command: set polarity inversion
 		data[1] = 0; // -> no polarity inversion
-		if (write(p->fd,data,2) != 2) {
+		if (i2c_write(p->i2c, data, 2) < 0) {
 			report(RPT_ERR, "HD44780: I2C: i2c set polarity inversion failed: %s", strerror(errno));
 		}
 		data[0] = 3; // command: set output direction
 		data[1] = 0; // -> all pins are outputs
-		if (write(p->fd,data,2) != 2) {
+		if (i2c_write(p->i2c, data, 2) <0) {
 			report(RPT_ERR, "HD44780: I2C: i2c set output direction failed: %s", strerror(errno));
 		}
 	}
@@ -311,12 +264,8 @@ hd_init_i2c(Driver *drvthis)
 
 void
 i2c_HD44780_close(PrivateData *p) {
-	if (p->fd >= 0) {
+	if (p->i2c >= 0)
-#ifdef HAVE_DEV_IICBUS_IIC_H
+		i2c_close(p->i2c);
-		ioctl(p->fd, I2CSTOP);
-#endif
-		close(p->fd);
-	}
 }
 
 

server/drivers/hd44780-low.h

@@ -29,6 +29,8 @@
 # include <ftdi.h>
 #endif
 
+#include "i2c.h"
+
 /** \name Symbolic names for connection types
  *@{*/
 #define HD44780_CT_UNKNOWN		0
@@ -157,7 +159,7 @@ typedef struct hd44780_private_data {
 #endif
 
 #ifdef HAVE_I2C
-	/* libi2c-based connection types */
+	/* i2c based connection types */
 	
 	int i2c_backlight_invert;
 	int i2c_line_RS;
@@ -169,6 +171,7 @@ typedef struct hd44780_private_data {
 	int i2c_line_D6;
 	int i2c_line_D7;
 
+	I2CHandle *i2c;
 #endif
 
 #ifdef WITH_ETHLCD

server/drivers/i2c.c

@@ -0,0 +1,120 @@
+/** \file server/drivers/i2c.c
+ * OS agnostic functions to access i2c devices.
+ */
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <strings.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+
+#ifdef HAVE_DEV_IICBUS_IIC_H
+#include <dev/iicbus/iic.h>
+#else /* HAVE_LINUX_I2C_DEV_H */
+#include <linux/i2c-dev.h>
+/* I2C_SLAVE is missing in linux/i2c-dev.h from kernel headers of 2.4.x kernels
+*/
+#ifndef I2C_SLAVE
+#define I2C_SLAVE 0x0703  /* ioctl to change slave address */
+#endif
+#endif
+
+#define I2C_DEFAULT_DEVICE "/dev/i2c-0"
+
+/** data to access an i2c slave */
+typedef struct {
+	int fd;			/**< file descriptor of the i2c device */
+#ifdef HAVE_DEV_IICBUS_IIC_H
+	unsigned int slave;	/**< slave address */
+#endif
+} I2CHandle;
+
+
+/**
+ * setup connection to i2c slave
+ * \param device   device file of the i2c bus
+ * \param addr     use this slave address
+ * \retval NULL    Error
+ * \retval	 Pointer to handle of the connection
+ */
+I2CHandle *i2c_open(const char *device, unsigned int addr)
+{
+	I2CHandle *h;
+#ifdef HAVE_DEV_IICBUS_IIC_H
+	struct iiccmd cmd;
+	bzero(&cmd, sizeof(cmd));
+#endif
+
+
+	h = malloc(sizeof(*h));
+	if(!h)
+		return NULL;
+
+	h->fd = open(device, O_RDWR);
+	if(h->fd < 0)
+		goto free;
+
+#ifdef HAVE_DEV_IICBUS_IIC_H
+	cmd.slave = h->slave = addr << 1;
+	cmd.last = 0;
+	cmd.count = 0;
+	if (ioctl(h->fd, I2CRSTCARD, &cmd) < 0)
+		goto close;
+	if (ioctl(h->fd, I2CSTART, &cmd) < 0)
+		goto close;
+#else /* HAVE_LINUX_I2C_DEV_H */
+	if (ioctl(h->fd, I2C_SLAVE, addr) < 0) {
+		goto close;
+	}
+#endif
+
+	return h;
+
+close:
+	close(h->fd);
+free:
+	free(h);
+	return NULL;
+}
+
+/**
+ * close connection to i2c slave
+ */
+void i2c_close(I2CHandle *h)
+{
+#ifdef HAVE_DEV_IICBUS_IIC_H
+	ioctl(h->fd, I2CSTOP);
+#endif
+	close(h->fd);
+	free(h);
+}
+
+/**
+ * write data to an i2c slave
+ * \param h	Handle of i2c slave
+ * \param buf   Buffer to send to the device
+ * \param count Number of bytes to write
+ * \retval >=0     Success.
+ * \retval <0      Error.
+ */
+int i2c_write(I2CHandle *h, void *buf, unsigned int count)
+{
+#ifdef HAVE_DEV_IICBUS_IIC_H
+	struct iiccmd cmd;
+	bzero(&cmd, sizeof(cmd));
+	cmd.slave = h->slave;
+	cmd.last = 1;
+	cmd.count = count;
+	cmd.buf = buf;
+
+	return ioctl(h->fd, I2CWRITE, &cmd);
+#else /* HAVE_LINUX_I2C_DEV_H */
+	if (write(h->fd, buf, count) != count)
+		return -1;
+	return 0;
+#endif
+}

server/drivers/i2c.h

@@ -0,0 +1,14 @@
+#ifndef I2C_H
+#define I2C_H
+
+struct i2c_data;
+
+typedef struct i2c_data I2CHandle;
+
+#define I2C_DEFAULT_DEVICE "/dev/i2c-0"
+
+extern I2CHandle *i2c_open(const char *device, unsigned int addr);
+extern void i2c_close(I2CHandle *h);
+extern int i2c_write(I2CHandle *h, void *buf, unsigned int count);
+
+#endif /* I2C_H */