[patch 00/10] parport: ieee1284 fixes and cleanups

[patch 00/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

I played with a daisy chain device that is not ieee1284 compliant
and found buffer overflow and failure to open daisy chain devices.
While fixing it I found also a number of other problems also affecting
proper ieee1284 devices.

This is a collection of the changes I have made. They have been through
linux-parport mailing list already in January and they have been modified
according to comments.

--

[patch 01/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-fix-ieee1284-device-id-reading.patch --]
[-- Type: text/plain, Size: 7301 bytes --]

Fix potential buffer overflow in case the device ID did not end in
semicolon. Also might fail to negotiate back to IEEE1284_MODE_COMPAT
in case of failure. parport_device_id did not return what
Documentation/parport-lowlevel.txt said, so I changed it to match it.

Determining device ID length is overly complicated, but Tim Waugh
recalled on linux-parport seeing some buggy device that might need it.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

 drivers/parport/probe.c |  193 ++++++++++++++++++++++++++++++++----------------
 1 files changed, 130 insertions(+), 63 deletions(-)

Index: linux-dvb/drivers/parport/probe.c
===================================================================
--- linux-dvb.orig/drivers/parport/probe.c	2005-08-29 19:21:48.000000000 +0300
+++ linux-dvb/drivers/parport/probe.c	2005-08-29 20:11:07.000000000 +0300
@@ -129,8 +129,131 @@ static void parse_data(struct parport *p
 	kfree(txt);
 }
 
+/* Read up to count-1 bytes of device id. Terminate buffer with
+ * '\0'. Buffer begins with two Device ID length bytes as given by
+ * device. */
+static ssize_t parport_read_device_id (struct parport *port, char *buffer,
+				       size_t count)
+{
+	unsigned char length[2];
+	unsigned lelen, belen;
+	size_t idlens[4];
+	unsigned numidlens;
+	unsigned current_idlen;
+	ssize_t retval;
+	size_t len;
+
+	/* First two bytes are MSB,LSB of inclusive length. */
+	retval = parport_read (port, length, 2);
+
+	if (retval < 0)
+		return retval;
+	if (retval != 2)
+		return -EIO;
+
+	if (count < 2)
+		return 0;
+	memcpy(buffer, length, 2);
+	len = 2;
+
+	/* Some devices wrongly send LE length, and some send it two
+	 * bytes short. Construct a sorted array of lengths to try. */
+	belen = (length[0] << 8) + length[1];
+	lelen = (length[1] << 8) + length[0];
+	idlens[0] = min(belen, lelen);
+	idlens[1] = idlens[0]+2;
+	if (belen != lelen) {
+		int off = 2;
+		/* Don't try lenghts of 0x100 and 0x200 as 1 and 2 */
+		if (idlens[0] <= 2)
+			off = 0;
+		idlens[off] = max(belen, lelen);
+		idlens[off+1] = idlens[off]+2;
+		numidlens = off+2;
+	}
+	else {
+		/* Some devices don't truly implement Device ID, but
+		 * just return constant nibble forever. This catches
+		 * also those cases. */
+		if (idlens[0] == 0 || idlens[0] > 0xFFF) {
+			printk (KERN_DEBUG "%s: reported broken Device ID"
+				" length of %#zX bytes\n",
+				port->name, idlens[0]);
+			return -EIO;
+		}
+		numidlens = 2;
+	}
+
+	/* Try to respect the given ID length despite all the bugs in
+	 * the ID length. Read according to shortest possible ID
+	 * first. */
+	for (current_idlen = 0; current_idlen < numidlens; ++current_idlen) {
+		size_t idlen = idlens[current_idlen];
+		if (idlen+1 >= count)
+			break;
+
+		retval = parport_read (port, buffer+len, idlen-len);
+
+		if (retval < 0)
+			return retval;
+		len += retval;
+
+		if (port->physport->ieee1284.phase != IEEE1284_PH_HBUSY_DAVAIL) {
+			if (belen != len) {
+				printk (KERN_DEBUG "%s: Device ID was %d bytes"
+					" while device told it would be %d"
+					" bytes\n",
+					port->name, len, belen);
+			}
+			goto done;
+		}
+
+		/* This might end reading the Device ID too
+		 * soon. Hopefully the needed fields were already in
+		 * the first 256 bytes or so that we must have read so
+		 * far. */
+		if (buffer[len-1] == ';') {
+ 			printk (KERN_DEBUG "%s: Device ID reading stopped"
+				" before device told data not available. "
+				"Current idlen %d of %d, len bytes %02X %02X\n",
+				port->name, current_idlen, numidlens,
+				length[0], length[1]);
+			goto done;
+		}
+	}
+	if (current_idlen < numidlens) {
+		/* Buffer not large enough, read to end of buffer. */
+		size_t idlen, len2;
+		if (len+1 < count) {
+			retval = parport_read (port, buffer+len, count-len-1);
+			if (retval < 0)
+				return retval;
+			len += retval;
+		}
+		/* Read the whole ID since some devices would not
+		 * otherwise give back the Device ID from beginning
+		 * next time when asked. */
+		idlen = idlens[current_idlen];
+		len2 = len;
+		while(len2 < idlen && retval > 0) {
+			char tmp[4];
+			retval = parport_read (port, tmp,
+					       min(sizeof tmp, idlen-len2));
+			if (retval < 0)
+				return retval;
+			len2 += retval;
+		}
+	}
+	/* In addition, there are broken devices out there that don't
+	   even finish off with a semi-colon. We do not need to care
+	   about those at this time. */
+ done:
+	buffer[len] = '\0';
+	return len;
+}
+
 /* Get Std 1284 Device ID. */
-ssize_t parport_device_id (int devnum, char *buffer, size_t len)
+ssize_t parport_device_id (int devnum, char *buffer, size_t count)
 {
 	ssize_t retval = -ENXIO;
 	struct pardevice *dev = parport_open (devnum, "Device ID probe",
@@ -140,76 +263,20 @@ ssize_t parport_device_id (int devnum, c
 
 	parport_claim_or_block (dev);
 
-	/* Negotiate to compatibility mode, and then to device ID mode.
-	 * (This is in case we are already in device ID mode.) */
+	/* Negotiate to compatibility mode, and then to device ID
+	 * mode. (This so that we start form beginning of device ID if
+	 * already in device ID mode.) */
 	parport_negotiate (dev->port, IEEE1284_MODE_COMPAT);
 	retval = parport_negotiate (dev->port,
 				    IEEE1284_MODE_NIBBLE | IEEE1284_DEVICEID);
 
 	if (!retval) {
-		int idlen;
-		unsigned char length[2];
-
-		/* First two bytes are MSB,LSB of inclusive length. */
-		retval = parport_read (dev->port, length, 2);
-
-		if (retval != 2) goto end_id;
-
-		idlen = (length[0] << 8) + length[1] - 2;
-		/*
-		 * Check if the caller-allocated buffer is large enough
-		 * otherwise bail out or there will be an at least off by one.
-		 */
-		if (idlen + 1 < len)
-			len = idlen;
-		else {
-			retval = -EINVAL;
-			goto out;
-		}
-		retval = parport_read (dev->port, buffer, len);
-
-		if (retval != len)
-			printk (KERN_DEBUG "%s: only read %Zd of %Zd ID bytes\n",
-				dev->port->name, retval,
-				len);
-
-		/* Some printer manufacturers mistakenly believe that
-                   the length field is supposed to be _exclusive_.
-		   In addition, there are broken devices out there
-                   that don't even finish off with a semi-colon. */
-		if (buffer[len - 1] != ';') {
-			ssize_t diff;
-			diff = parport_read (dev->port, buffer + len, 2);
-			retval += diff;
-
-			if (diff)
-				printk (KERN_DEBUG
-					"%s: device reported incorrect "
-					"length field (%d, should be %Zd)\n",
-					dev->port->name, idlen, retval);
-			else {
-				/* One semi-colon short of a device ID. */
-				buffer[len++] = ';';
-				printk (KERN_DEBUG "%s: faking semi-colon\n",
-					dev->port->name);
-
-				/* If we get here, I don't think we
-                                   need to worry about the possible
-                                   standard violation of having read
-                                   more than we were told to.  The
-                                   device is non-compliant anyhow. */
-			}
-		}
-
-	end_id:
-		buffer[len] = '\0';
+		retval = parport_read_device_id (dev->port, buffer, count);
 		parport_negotiate (dev->port, IEEE1284_MODE_COMPAT);
+		if (retval > 2)
+			parse_data (dev->port, dev->daisy, buffer+2);
 	}
 
-	if (retval > 2)
-		parse_data (dev->port, dev->daisy, buffer);
-
-out:
 	parport_release (dev);
 	parport_close (dev);
 	return retval;

--

[patch 02/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-fix-read-nibble.patch --]
[-- Type: text/plain, Size: 6015 bytes --]

Did not move the parport interface properly into IEEE1284_PH_REV_IDLE
phase at end of data due to comparing bytes with nibbles. Internal
phase IEEE1284_PH_HBUSY_DNA became unused, so remove it.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

 drivers/media/video/cpia_pp.c  |   30 ++++++++-----------
 drivers/parport/ieee1284_ops.c |   62 +++++++++++++++++++----------------------
 include/linux/parport.h        |    1 
 3 files changed, 42 insertions(+), 51 deletions(-)

Index: linux-dvb/drivers/media/video/cpia_pp.c
===================================================================
--- linux-dvb.orig/drivers/media/video/cpia_pp.c	2005-05-30 19:44:27.000000000 +0300
+++ linux-dvb/drivers/media/video/cpia_pp.c	2005-05-30 19:48:30.000000000 +0300
@@ -170,16 +170,9 @@ static size_t cpia_read_nibble (struct p
 		/* Does the error line indicate end of data? */
 		if (((i /*& 1*/) == 0) &&
 		    (parport_read_status(port) & PARPORT_STATUS_ERROR)) {
-			port->physport->ieee1284.phase = IEEE1284_PH_HBUSY_DNA;
-				DBG("%s: No more nibble data (%d bytes)\n",
-				port->name, i/2);
-
-			/* Go to reverse idle phase. */
-			parport_frob_control (port,
-					      PARPORT_CONTROL_AUTOFD,
-					      PARPORT_CONTROL_AUTOFD);
-			port->physport->ieee1284.phase = IEEE1284_PH_REV_IDLE;
-			break;
+			DBG("%s: No more nibble data (%d bytes)\n",
+			    port->name, i/2);
+			goto end_of_data;
 		}
 
 		/* Event 7: Set nAutoFd low. */
@@ -227,18 +220,21 @@ static size_t cpia_read_nibble (struct p
 			byte = nibble;
 	}
 
-	i /= 2; /* i is now in bytes */
-
 	if (i == len) {
 		/* Read the last nibble without checking data avail. */
-		port = port->physport;
-		if (parport_read_status (port) & PARPORT_STATUS_ERROR)
-			port->ieee1284.phase = IEEE1284_PH_HBUSY_DNA;
+		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
+		end_of_data:
+			/* Go to reverse idle phase. */
+			parport_frob_control (port,
+					      PARPORT_CONTROL_AUTOFD,
+					      PARPORT_CONTROL_AUTOFD);
+			port->physport->ieee1284.phase = IEEE1284_PH_REV_IDLE;
+		}
 		else
-			port->ieee1284.phase = IEEE1284_PH_HBUSY_DAVAIL;
+			port->physport->ieee1284.phase = IEEE1284_PH_HBUSY_DAVAIL;
 	}
 
-	return i;
+	return i/2;
 }
 
 /* CPiA nonstandard "Nibble Stream" mode (2 nibbles per cycle, instead of 1)
Index: linux-dvb/drivers/parport/ieee1284_ops.c
===================================================================
--- linux-dvb.orig/drivers/parport/ieee1284_ops.c	2005-05-30 19:45:54.000000000 +0300
+++ linux-dvb/drivers/parport/ieee1284_ops.c	2005-05-30 19:48:30.000000000 +0300
@@ -166,17 +166,7 @@ size_t parport_ieee1284_read_nibble (str
 		/* Does the error line indicate end of data? */
 		if (((i & 1) == 0) &&
 		    (parport_read_status(port) & PARPORT_STATUS_ERROR)) {
-			port->physport->ieee1284.phase = IEEE1284_PH_HBUSY_DNA;
-			DPRINTK (KERN_DEBUG
-				"%s: No more nibble data (%d bytes)\n",
-				port->name, i/2);
-
-			/* Go to reverse idle phase. */
-			parport_frob_control (port,
-					      PARPORT_CONTROL_AUTOFD,
-					      PARPORT_CONTROL_AUTOFD);
-			port->physport->ieee1284.phase = IEEE1284_PH_REV_IDLE;
-			break;
+			goto end_of_data;
 		}
 
 		/* Event 7: Set nAutoFd low. */
@@ -226,18 +216,25 @@ size_t parport_ieee1284_read_nibble (str
 			byte = nibble;
 	}
 
-	i /= 2; /* i is now in bytes */
-
 	if (i == len) {
 		/* Read the last nibble without checking data avail. */
-		port = port->physport;
-		if (parport_read_status (port) & PARPORT_STATUS_ERROR)
-			port->ieee1284.phase = IEEE1284_PH_HBUSY_DNA;
+		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
+		end_of_data:
+			DPRINTK (KERN_DEBUG
+				"%s: No more nibble data (%d bytes)\n",
+				port->name, i/2);
+
+			/* Go to reverse idle phase. */
+			parport_frob_control (port,
+					      PARPORT_CONTROL_AUTOFD,
+					      PARPORT_CONTROL_AUTOFD);
+			port->physport->ieee1284.phase = IEEE1284_PH_REV_IDLE;
+		}
 		else
-			port->ieee1284.phase = IEEE1284_PH_HBUSY_DAVAIL;
+			port->physport->ieee1284.phase = IEEE1284_PH_HBUSY_DAVAIL;
 	}
 
-	return i;
+	return i/2;
 #endif /* IEEE1284 support */
 }
 
@@ -257,17 +254,7 @@ size_t parport_ieee1284_read_byte (struc
 
 		/* Data available? */
 		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
-			port->physport->ieee1284.phase = IEEE1284_PH_HBUSY_DNA;
-			DPRINTK (KERN_DEBUG
-				 "%s: No more byte data (%Zd bytes)\n",
-				 port->name, count);
-
-			/* Go to reverse idle phase. */
-			parport_frob_control (port,
-					      PARPORT_CONTROL_AUTOFD,
-					      PARPORT_CONTROL_AUTOFD);
-			port->physport->ieee1284.phase = IEEE1284_PH_REV_IDLE;
-			break;
+			goto end_of_data;
 		}
 
 		/* Event 14: Place data bus in high impedance state. */
@@ -319,11 +306,20 @@ size_t parport_ieee1284_read_byte (struc
 
 	if (count == len) {
 		/* Read the last byte without checking data avail. */
-		port = port->physport;
-		if (parport_read_status (port) & PARPORT_STATUS_ERROR)
-			port->ieee1284.phase = IEEE1284_PH_HBUSY_DNA;
+		if (parport_read_status (port) & PARPORT_STATUS_ERROR) {
+		end_of_data:
+			DPRINTK (KERN_DEBUG
+				 "%s: No more byte data (%Zd bytes)\n",
+				 port->name, count);
+
+			/* Go to reverse idle phase. */
+			parport_frob_control (port,
+					      PARPORT_CONTROL_AUTOFD,
+					      PARPORT_CONTROL_AUTOFD);
+			port->physport->ieee1284.phase = IEEE1284_PH_REV_IDLE;
+		}
 		else
-			port->ieee1284.phase = IEEE1284_PH_HBUSY_DAVAIL;
+			port->physport->ieee1284.phase = IEEE1284_PH_HBUSY_DAVAIL;
 	}
 
 	return count;
Index: linux-dvb/include/linux/parport.h
===================================================================
--- linux-dvb.orig/include/linux/parport.h	2005-05-30 19:44:27.000000000 +0300
+++ linux-dvb/include/linux/parport.h	2005-05-30 19:48:30.000000000 +0300
@@ -242,7 +242,6 @@ enum ieee1284_phase {
 	IEEE1284_PH_FWD_IDLE,
 	IEEE1284_PH_TERMINATE,
 	IEEE1284_PH_NEGOTIATION,
-	IEEE1284_PH_HBUSY_DNA,
 	IEEE1284_PH_REV_IDLE,
 	IEEE1284_PH_HBUSY_DAVAIL,
 	IEEE1284_PH_REV_DATA,

--

[patch 03/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-fix-ieee1284.3-daisy-chain-end-detection.patch --]
[-- Type: text/plain, Size: 2084 bytes --]

Daisy chain end detection failed at least with older daisy chain
devices that do not implement the last device signal.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

 drivers/parport/daisy.c |   27 +++++++++++++++++----------
 1 files changed, 17 insertions(+), 10 deletions(-)

Index: linux-dvb/drivers/parport/daisy.c
===================================================================
--- linux-dvb.orig/drivers/parport/daisy.c	2005-08-29 20:11:52.000000000 +0300
+++ linux-dvb/drivers/parport/daisy.c	2005-08-29 20:12:35.000000000 +0300
@@ -436,7 +436,7 @@ static int select_port (struct parport *
 
 static int assign_addrs (struct parport *port)
 {
-	unsigned char s, last_dev;
+	unsigned char s;
 	unsigned char daisy;
 	int thisdev = numdevs;
 	int detected;
@@ -472,10 +472,13 @@ static int assign_addrs (struct parport 
 	}
 
 	parport_write_data (port, 0x78); udelay (2);
-	last_dev = 0; /* We've just been speaking to a device, so we
-			 know there must be at least _one_ out there. */
+	s = parport_read_status (port);
 
-	for (daisy = 0; daisy < 4; daisy++) {
+	for (daisy = 0;
+	     (s & (PARPORT_STATUS_PAPEROUT|PARPORT_STATUS_SELECT))
+		     == (PARPORT_STATUS_PAPEROUT|PARPORT_STATUS_SELECT)
+		     && daisy < 4;
+	     ++daisy) {
 		parport_write_data (port, daisy);
 		udelay (2);
 		parport_frob_control (port,
@@ -485,14 +488,18 @@ static int assign_addrs (struct parport 
 		parport_frob_control (port, PARPORT_CONTROL_STROBE, 0);
 		udelay (1);
 
-		if (last_dev)
-			/* No more devices. */
-			break;
+		add_dev (numdevs++, port, daisy);
 
-		last_dev = !(parport_read_status (port)
-			     & PARPORT_STATUS_BUSY);
+		/* See if this device thought it was the last in the
+		 * chain. */
+		if (!(s & PARPORT_STATUS_BUSY))
+			break;
 
-		add_dev (numdevs++, port, daisy);
+		/* We are seeing pass through status now. We see
+		   last_dev from next device or if last_dev does not
+		   work status lines from some non-daisy chain
+		   device. */
+		s = parport_read_status (port);
 	}
 
 	parport_write_data (port, 0xff); udelay (2);

--

[patch 04/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-fix-ieee1284.3-daisy-device-opening-to-open-existing-devices.patch --]
[-- Type: text/plain, Size: 691 bytes --]

Device ID reading from daisy chain devices failed because the daisy
device could not be opened.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

Index: linux-dvb/drivers/parport/daisy.c
===================================================================
--- linux-dvb.orig/drivers/parport/daisy.c	2005-08-29 20:16:48.000000000 +0300
+++ linux-dvb/drivers/parport/daisy.c	2005-08-29 20:17:06.000000000 +0300
@@ -252,7 +252,7 @@ struct pardevice *parport_open (int devn
 		selected = port->daisy;
 		parport_release (dev);
 
-		if (selected != port->daisy) {
+		if (selected != daisy) {
 			/* No corresponding device. */
 			parport_unregister_device (dev);
 			return NULL;

--

[patch 05/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-slab-alloc-size.patch --]
[-- Type: text/plain, Size: 1296 bytes --]

Use the complete slab buffer that is allocated by kmalloc.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

 drivers/parport/daisy.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

Index: linux-dvb/drivers/parport/daisy.c
===================================================================
--- linux-dvb.orig/drivers/parport/daisy.c	2005-08-29 20:17:06.000000000 +0300
+++ linux-dvb/drivers/parport/daisy.c	2005-08-29 20:17:26.000000000 +0300
@@ -144,9 +144,9 @@ again:
 	add_dev (numdevs++, port, -1);
 
 	/* Find out the legacy device's IEEE 1284 device ID. */
-	deviceid = kmalloc (1000, GFP_KERNEL);
+	deviceid = kmalloc (1024, GFP_KERNEL);
 	if (deviceid) {
-		if (parport_device_id (numdevs - 1, deviceid, 1000) > 2)
+		if (parport_device_id (numdevs - 1, deviceid, 1024) > 2)
 			detected++;
 
 		kfree (deviceid);
@@ -508,11 +508,11 @@ static int assign_addrs (struct parport 
 		 detected);
 
 	/* Ask the new devices to introduce themselves. */
-	deviceid = kmalloc (1000, GFP_KERNEL);
+	deviceid = kmalloc (1024, GFP_KERNEL);
 	if (!deviceid) return 0;
 
 	for (daisy = 0; thisdev < numdevs; thisdev++, daisy++)
-		parport_device_id (thisdev, deviceid, 1000);
+		parport_device_id (thisdev, deviceid, 1024);
 
 	kfree (deviceid);
 	return detected;

--

[patch 06/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-add-some-missing-const-from-static-variables-in-parport-driver.patch --]
[-- Type: text/plain, Size: 5335 bytes --]

Trivial "const" additions to places in parport that truly are const.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

 drivers/parport/parport_pc.c |   38 ++++++++++++++++++++++----------------
 drivers/parport/probe.c      |    6 +++---
 2 files changed, 25 insertions(+), 19 deletions(-)

Index: linux-dvb/drivers/parport/parport_pc.c
===================================================================
--- linux-dvb.orig/drivers/parport/parport_pc.c	2005-06-24 10:41:40.000000000 +0300
+++ linux-dvb/drivers/parport/parport_pc.c	2005-06-24 13:03:46.000000000 +0300
@@ -1170,7 +1170,7 @@ dump_parport_state ("fwd idle", port);
 
 /* GCC is not inlining extern inline function later overwriten to non-inline,
    so we use outlined_ variants here.  */
-static struct parport_operations parport_pc_ops =
+static const struct parport_operations parport_pc_ops =
 {
 	.write_data	= parport_pc_write_data,
 	.read_data	= parport_pc_read_data,
@@ -1212,10 +1212,11 @@ static struct parport_operations parport
 static void __devinit show_parconfig_smsc37c669(int io, int key)
 {
 	int cr1,cr4,cra,cr23,cr26,cr27,i=0;
-	static const char *modes[]={ "SPP and Bidirectional (PS/2)",	
-				     "EPP and SPP",
-				     "ECP",
-				     "ECP and EPP" };
+	static const char *const modes[]={
+		"SPP and Bidirectional (PS/2)",	
+		"EPP and SPP",
+		"ECP",
+		"ECP and EPP" };
 
 	outb(key,io);
 	outb(key,io);
@@ -1289,7 +1290,7 @@ static void __devinit show_parconfig_sms
 static void __devinit show_parconfig_winbond(int io, int key)
 {
 	int cr30,cr60,cr61,cr70,cr74,crf0,i=0;
-	static const char *modes[] = {
+	static const char *const modes[] = {
 		"Standard (SPP) and Bidirectional(PS/2)", /* 0 */
 		"EPP-1.9 and SPP",
 		"ECP",
@@ -1298,7 +1299,9 @@ static void __devinit show_parconfig_win
 		"EPP-1.7 and SPP",		/* 5 */
 		"undefined!",
 		"ECP and EPP-1.7" };
-	static char *irqtypes[] = { "pulsed low, high-Z", "follows nACK" };
+	static char *const irqtypes[] = {
+		"pulsed low, high-Z",
+		"follows nACK" };
 		
 	/* The registers are called compatible-PnP because the
            register layout is modelled after ISA-PnP, the access
@@ -2397,7 +2400,8 @@ EXPORT_SYMBOL (parport_pc_unregister_por
 
 /* ITE support maintained by Rich Liu <richliu@poorman.org> */
 static int __devinit sio_ite_8872_probe (struct pci_dev *pdev, int autoirq,
-					 int autodma, struct parport_pc_via_data *via)
+					 int autodma,
+					 const struct parport_pc_via_data *via)
 {
 	short inta_addr[6] = { 0x2A0, 0x2C0, 0x220, 0x240, 0x1E0 };
 	struct resource *base_res;
@@ -2505,7 +2509,7 @@ static int __devinit sio_ite_8872_probe 
 static int __devinitdata parport_init_mode = 0;
 
 /* Data for two known VIA chips */
-static struct parport_pc_via_data via_686a_data __devinitdata = {
+static const struct parport_pc_via_data via_686a_data __devinitdata = {
 	0x51,
 	0x50,
 	0x85,
@@ -2514,7 +2518,7 @@ static struct parport_pc_via_data via_68
 	0xF0,
 	0xE6
 };
-static struct parport_pc_via_data via_8231_data __devinitdata = {
+static const struct parport_pc_via_data via_8231_data __devinitdata = {
 	0x45,
 	0x44,
 	0x50,
@@ -2525,7 +2529,8 @@ static struct parport_pc_via_data via_82
 };
 
 static int __devinit sio_via_probe (struct pci_dev *pdev, int autoirq,
-					 int autodma, struct parport_pc_via_data *via)
+				    int autodma,
+				    const struct parport_pc_via_data *via)
 {
 	u8 tmp, tmp2, siofunc;
 	u8 ppcontrol = 0;
@@ -2694,9 +2699,10 @@ enum parport_pc_sio_types {
 };
 
 /* each element directly indexed from enum list, above */
-static struct parport_pc_superio {
-	int (*probe) (struct pci_dev *pdev, int autoirq, int autodma, struct parport_pc_via_data *via);
-	struct parport_pc_via_data *via;
+static const struct parport_pc_superio {
+	int (*probe) (struct pci_dev *pdev, int autoirq, int autodma,
+		      const struct parport_pc_via_data *via);
+	const struct parport_pc_via_data *via;
 } parport_pc_superio_info[] __devinitdata = {
 	{ sio_via_probe, &via_686a_data, },
 	{ sio_via_probe, &via_8231_data, },
@@ -2756,7 +2762,7 @@ enum parport_pc_pci_cards {
 
 /* each element directly indexed from enum list, above 
  * (but offset by last_sio) */
-static struct parport_pc_pci {
+static const struct parport_pc_pci {
 	int numports;
 	struct { /* BAR (base address registers) numbers in the config
                     space header */
@@ -2827,7 +2833,7 @@ static struct parport_pc_pci {
 	/* netmos_9815 */               { 2, { { 0, -1 }, { 2, -1 }, } }, /* untested */
 };
 
-static struct pci_device_id parport_pc_pci_tbl[] = {
+static const struct pci_device_id parport_pc_pci_tbl[] = {
 	/* Super-IO onboard chips */
 	{ 0x1106, 0x0686, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sio_via_686a },
 	{ 0x1106, 0x8231, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sio_via_8231 },
Index: linux-dvb/drivers/parport/probe.c
===================================================================
--- linux-dvb.orig/drivers/parport/probe.c	2005-06-24 13:03:46.000000000 +0300
+++ linux-dvb/drivers/parport/probe.c	2005-06-24 13:03:46.000000000 +0300
@@ -11,9 +11,9 @@
 #include <linux/string.h>
 #include <asm/uaccess.h>
 
-static struct {
-	char *token;
-	char *descr;
+static const struct {
+	const char *token;
+	const char *descr;
 } classes[] = {
 	{ "",            "Legacy device" },
 	{ "PRINTER",     "Printer" },

--

[patch 07/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-fix-parport-driver-to-compile-with-debugging-defined.patch --]
[-- Type: text/plain, Size: 1197 bytes --]

Add missing "struct" keyword preventing compilation with DEBUG_PARPORT
defined. Also add some "const".

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

 include/linux/parport_pc.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-dvb/include/linux/parport_pc.h
===================================================================
--- linux-dvb.orig/include/linux/parport_pc.h	2005-06-24 15:42:28.000000000 +0300
+++ linux-dvb/include/linux/parport_pc.h	2005-06-24 15:42:52.000000000 +0300
@@ -85,8 +85,8 @@ extern __inline__ void dump_parport_stat
 	unsigned char ecr = inb (ECONTROL (p));
 	unsigned char dcr = inb (CONTROL (p));
 	unsigned char dsr = inb (STATUS (p));
-	static char *ecr_modes[] = {"SPP", "PS2", "PPFIFO", "ECP", "xXx", "yYy", "TST", "CFG"};
-	const struct parport_pc_private *priv = (parport_pc_private *)p->physport->private_data;
+	static const char *const ecr_modes[] = {"SPP", "PS2", "PPFIFO", "ECP", "xXx", "yYy", "TST", "CFG"};
+	const struct parport_pc_private *priv = (struct parport_pc_private *)p->physport->private_data;
 	int i;
 
 	printk (KERN_DEBUG "*** parport state (%s): ecr=[%s", str, ecr_modes[(ecr & 0xe0) >> 5]);

--

[patch 08/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-correct-dependency-on-parport-pc-rather-than-just-parport.patch --]
[-- Type: text/plain, Size: 3266 bytes --]

Make drivers that use directly PC parport HW depend on PARPORT_PC
rather than HW independent PARPORT.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

 drivers/block/Kconfig        |    2 +-
 drivers/block/paride/Kconfig |    5 +++--
 drivers/scsi/Kconfig         |    8 ++++----
 3 files changed, 8 insertions(+), 7 deletions(-)

Index: linux-dvb/drivers/block/Kconfig
===================================================================
--- linux-dvb.orig/drivers/block/Kconfig	2005-06-23 22:12:45.000000000 +0300
+++ linux-dvb/drivers/block/Kconfig	2005-06-24 13:03:46.000000000 +0300
@@ -117,7 +117,7 @@ config BLK_DEV_XD
 
 config PARIDE
 	tristate "Parallel port IDE device support"
-	depends on PARPORT
+	depends on PARPORT_PC
 	---help---
 	  There are many external CD-ROM and disk devices that connect through
 	  your computer's parallel port. Most of them are actually IDE devices
Index: linux-dvb/drivers/block/paride/Kconfig
===================================================================
--- linux-dvb.orig/drivers/block/paride/Kconfig	2005-06-23 22:12:45.000000000 +0300
+++ linux-dvb/drivers/block/paride/Kconfig	2005-06-24 13:03:46.000000000 +0300
@@ -4,11 +4,12 @@
 # PARIDE doesn't need PARPORT, but if PARPORT is configured as a module,
 # PARIDE must also be a module.  The bogus CONFIG_PARIDE_PARPORT option
 # controls the choices given to the user ...
+# PARIDE only supports PC style parports. Tough for USB or other parports...
 config PARIDE_PARPORT
 	tristate
 	depends on PARIDE!=n
-	default m if PARPORT=m
-	default y if PARPORT!=m
+	default m if PARPORT_PC=m
+	default y if PARPORT_PC!=m
 
 comment "Parallel IDE high-level drivers"
 	depends on PARIDE
Index: linux-dvb/drivers/scsi/Kconfig
===================================================================
--- linux-dvb.orig/drivers/scsi/Kconfig	2005-06-24 10:41:40.000000000 +0300
+++ linux-dvb/drivers/scsi/Kconfig	2005-06-24 13:03:46.000000000 +0300
@@ -855,7 +855,7 @@ config SCSI_INIA100
 
 config SCSI_PPA
 	tristate "IOMEGA parallel port (ppa - older drives)"
-	depends on SCSI && PARPORT
+	depends on SCSI && PARPORT_PC
 	---help---
 	  This driver supports older versions of IOMEGA's parallel port ZIP
 	  drive (a 100 MB removable media device).
@@ -882,7 +882,7 @@ config SCSI_PPA
 
 config SCSI_IMM
 	tristate "IOMEGA parallel port (imm - newer drives)"
-	depends on SCSI && PARPORT
+	depends on SCSI && PARPORT_PC
 	---help---
 	  This driver supports newer versions of IOMEGA's parallel port ZIP
 	  drive (a 100 MB removable media device).
@@ -909,7 +909,7 @@ config SCSI_IMM
 
 config SCSI_IZIP_EPP16
 	bool "ppa/imm option - Use slow (but safe) EPP-16"
-	depends on PARPORT && (SCSI_PPA || SCSI_IMM)
+	depends on SCSI_PPA || SCSI_IMM
 	---help---
 	  EPP (Enhanced Parallel Port) is a standard for parallel ports which
 	  allows them to act as expansion buses that can handle up to 64
@@ -924,7 +924,7 @@ config SCSI_IZIP_EPP16
 
 config SCSI_IZIP_SLOW_CTR
 	bool "ppa/imm option - Assume slow parport control register"
-	depends on PARPORT && (SCSI_PPA || SCSI_IMM)
+	depends on SCSI_PPA || SCSI_IMM
 	help
 	  Some parallel ports are known to have excessive delays between
 	  changing the parallel port control register and good data being

--

[patch 09/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-remove-unused-includes.patch --]
[-- Type: text/plain, Size: 699 bytes --]

Small cleanup of includes meant for older implementation.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

diff -Nru a/drivers/net/plip.c b/drivers/net/plip.c
--- a/drivers/net/plip.c	2005-01-08 07:44:25 +02:00
+++ b/drivers/net/plip.c	2005-01-16 12:20:44 +02:00
@@ -98,7 +98,6 @@
 #include <linux/in.h>
 #include <linux/errno.h>
 #include <linux/delay.h>
-#include <linux/lp.h>
 #include <linux/init.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
@@ -106,7 +105,6 @@
 #include <linux/skbuff.h>
 #include <linux/if_plip.h>
 #include <linux/workqueue.h>
-#include <linux/ioport.h>
 #include <linux/spinlock.h>
 #include <linux/parport.h>
 #include <linux/bitops.h>

--

[patch 10/10] parport: ieee1284 fixes and cleanups

[marko.kohtala]

[-- Attachment #1: parport-export-parport-get-port-for-ppscsi.patch --]
[-- Type: text/plain, Size: 620 bytes --]

Help external ppSCSI driver by exporting parport_get_port to match the
parport_put_port.

Signed-off-by: Marko Kohtala <marko.kohtala@gmail.com>

---

diff -Nru a/drivers/parport/share.c b/drivers/parport/share.c
--- a/drivers/parport/share.c	2004-10-28 10:39:58 +03:00
+++ b/drivers/parport/share.c	2004-12-15 22:50:57 +02:00
@@ -1007,6 +1007,7 @@
 EXPORT_SYMBOL(parport_unregister_driver);
 EXPORT_SYMBOL(parport_register_device);
 EXPORT_SYMBOL(parport_unregister_device);
+EXPORT_SYMBOL(parport_get_port);
 EXPORT_SYMBOL(parport_put_port);
 EXPORT_SYMBOL(parport_find_number);
 EXPORT_SYMBOL(parport_find_base);

--

Re: [patch 00/10] parport: ieee1284 fixes and cleanups

[Andrew Morton]

marko.kohtala@gmail.com wrote:
>
> I played with a daisy chain device that is not ieee1284 compliant
> and found buffer overflow and failure to open daisy chain devices.
> While fixing it I found also a number of other problems also affecting
> proper ieee1284 devices.
> 
> This is a collection of the changes I have made. They have been through
> linux-parport mailing list already in January and they have been modified
> according to comments.

umm, OK.  parport patches worry me because nobody seems to understand the
code any more.  We'll see.

You just sent ten patches, all with the same name.  This causes me grief
(See http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, section 2a).

I now need to invent names for your patches, and if I later refer to one of
them you won't know which one I'm talking about.  Oh well.

Re: [patch 00/10] parport: ieee1284 fixes and cleanups

[Marko Kohtala]

On 9/7/05, Andrew Morton <akpm@osdl.org> wrote:
> marko.kohtala@gmail.com wrote:
> > This is a collection of the changes I have made. They have been through
> > linux-parport mailing list already in January and they have been modified
> > according to comments.
> 
> umm, OK.  parport patches worry me because nobody seems to understand the
> code any more.  We'll see.

Thanks. I'm worried too.

>From the responses in linux-parport I gathered some of the bugs are
there because people have not used the code. Therefore I also
considered removing it would be good, but was afraid to do so.

Also there are drivers that would have use for it, but they just are
currently implemented either by just reserving the whole parport and
then doing their daisy chain magic.

> You just sent ten patches, all with the same name.  This causes me grief
> (See http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, section 2a).

I used "quilt mail" to send those patches and it seems it requires
some additional trick I did not notice to make the patches have
different subjects. I had read the document you referred to but missed
that part. I also had picked somewhere the idea that having same
subject helped group the patches.

> I now need to invent names for your patches, and if I later refer to one of
> them you won't know which one I'm talking about.  Oh well.

I can also resend the patches to you. I understand you have a lot to
do already and I want to be of help, not burden.

Re: [patch 00/10] parport: ieee1284 fixes and cleanups

[Andrew Morton]

Marko Kohtala <marko.kohtala@gmail.com> wrote:
>
> > You just sent ten patches, all with the same name.  This causes me grief
>  > (See http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, section 2a).
> 
>  I used "quilt mail" to send those patches and it seems it requires
>  some additional trick I did not notice to make the patches have
>  different subjects.

I complained to the quilt guys about that and they did make a move to fix
it, but I recall not being very happy with the proposal.  Anyway, make sure
you have the latest version and check the documentation - it's in there
somewhere.

As a last resort, put the title into the first line of the changelog and
I'll cut-n-paste it.

>  that part. I also had picked somewhere the idea that having same
>  subject helped group the patches.
> 
>  > I now need to invent names for your patches, and if I later refer to one of
>  > them you won't know which one I'm talking about.  Oh well.
> 
>  I can also resend the patches to you. I understand you have a lot to
>  do already and I want to be of help, not burden.

Is OK, I'll work something out.

Re: [patch 00/10] parport: ieee1284 fixes and cleanups

[Dmitry Torokhov]

On Wednesday 07 September 2005 06:50, Andrew Morton wrote:
> Marko Kohtala <marko.kohtala@gmail.com> wrote:
> >
> > > You just sent ten patches, all with the same name.  This causes me grief
> >  > (See http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, section 2a).
> > 
> >  I used "quilt mail" to send those patches and it seems it requires
> >  some additional trick I did not notice to make the patches have
> >  different subjects.
> 
> I complained to the quilt guys about that and they did make a move to fix
> it, but I recall not being very happy with the proposal.  Anyway, make sure
> you have the latest version and check the documentation - it's in there
> somewhere.
> 
> As a last resort, put the title into the first line of the changelog and
> I'll cut-n-paste it.
>

I have the following in my .quiltrc

quilt_mail_patch_filter() {
        local x=$(cat)
        echo "$x" \
        | sed -n \
               -e 's/^\(To\|Cc\):/Recipient-\1:/ip' \
               -e 's/^Subject:/Replace-Subject:/p' \
               -e '/^\*\*\*\|---/q'
        echo
        # Discard the patch header, and pass on the rest
        echo "$x" | awk '
        !seen_from && (/^From: /) { print $0 "\n" ; seen_from = 1 }
        !in_body && (/^[-A-Za-z]+:/) { next }
        !in_body && (/^$/) { in_body = 1 ; next }
        { print }
        '
}

And I have my patches in the following form:

Subject: mail subject
From: <someone if not I, git uses it>

<area (Usually Input)>: Short patch description

Decsription

Signed-off-by: X XX
---
<patch>

Then quilt mail command seems to do the right thing.

-- 
Dmitry