Printing to off-line printer in 2.4.0-prerelease

Hello,

When trying to print to an off-line printer with 2.4 kernels, the "write" system call to /dev/lp0 stalls for 10 seconds and then returns EIO. This has the unfortunate effect that the printout will be lost, because the redhat print filters (in rh7) use "cat" to send data to the printer device, and cat gives up when EIO is returned.

In 2.2 kernels, the write call waited until the printer went on-line again.

I think the problem is that the lp_write() function in lp.c calls lp_check_status() before starting to write to the parport, and if the printer is not ready, it simply gives up. (The LP_ABORT flag is not used in that case.)

The following patch makes things work for me again. The kernel keeps generating the following debug messages until the printer becomes ready, but that doesn't seem to cause any problems.

  Jan  3 18:54:27 ppro kernel: DMA write timed out
  Jan  3 18:54:37 ppro kernel: parport0: FIFO is stuck
  Jan  3 18:54:37 ppro kernel: parport0: BUSY timeout (1) in compat_write_block_pio
  Jan  3 18:54:37 ppro kernel: lp0 off-line

Is there a better way to fix this problem?

@@ -259,7 +259,6 @@
 	parport_set_timeout (lp_table[minor].dev,
 			     lp_table[minor].timeout);
 
-	if ((retv = lp_check_status (minor)) == 0)
 	do {
 		/* Write the data. */
 		written = parport_write (port, kbuf, copy_size);

@@ -524,7 +524,8 @@
 					     PARPORT_STATUS_PAPEROUT,
 					     PARPORT_STATUS_PAPEROUT);
 		if (r)
-			DPRINTK (KERN_INFO "%s: Timeout at event 31\n");
+			DPRINTK (KERN_INFO "%s: Timeout at event 31\n",
+				 port->name);
 
 		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 		DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",

Peter �sterlund             [email protected]
Sk�ndalsv�gen 35            http://home1.swipnet.se/~w-15919
S-128 66 Sk�ndal            +46 8 942647
Sweden

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

It looks the simpler fix to me (main loop needs someway to handle errors anyways) but ask Tim too.

Another way to fix it is to loop in interruptible mode inside lp_error waiting the error to go away.

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I didn't give my previous patch enough testing. It still doesn't work if I queue a print job when my printer (HP LaserJet 4MP) is powered off. Apparently the printer tells the computer it is OK to send data to it when it is off.

Anyway, this new patch works for all tests I could think of. I tested with the printer initially powered off, initially powered on but in offline mode and initially powered on and in online mode. It also works if I put the printer in offline mode during sending of a print job. (tunelp -a and -o also work as expected.)

I also only get one DMA write timeout when putting the printer in offline mode during sending, instead of repeated timeouts as I got with the previous patch.

Here is the new patch:

@@ -231,6 +231,7 @@
 	ssize_t retv = 0;
 	ssize_t written;
 	size_t copy_size = count;
+	int check_status;
 
 #ifdef LP_STATS
 	if (jiffies-lp_table[minor].lastcall > LP_TIME(minor))

@@ -259,10 +260,26 @@
 	parport_set_timeout (lp_table[minor].dev,
 			     lp_table[minor].timeout);
 
-	if ((retv = lp_check_status (minor)) == 0)
+	check_status = 1;
 	do {
-		/* Write the data. */
-		written = parport_write (port, kbuf, copy_size);
+		int error = 0;
+		if (check_status) {
+			error = lp_check_status (minor);
+			if (error) {
+				if (LP_F(minor) & LP_ABORT) {
+					if (retv == 0)
+						retv = error;
+					break;
+				}
+				parport_yield_blocking (lp_table[minor].dev);
+			}
+			check_status = 0;
+		}
+		if (error == 0)
+			/* Write the data. */
+			written = parport_write (port, kbuf, copy_size);
+		else
+			written = 0;
 		if (written >= 0) {
 			copy_size -= written;
 			count -= written;

@@ -279,15 +296,7 @@
 
 		if (copy_size > 0) {
 			/* incomplete write -> check error ! */
-			int error = lp_check_status (minor);
-
-			if (LP_F(minor) & LP_ABORT) {
-				if (retv == 0)
-					retv = error;
-				break;
-			}
-
-			parport_yield_blocking (lp_table[minor].dev);
+			check_status = 1;
 		} else if (current->need_resched)
 			schedule ();
 

@@ -524,7 +524,8 @@
 					     PARPORT_STATUS_PAPEROUT,
 					     PARPORT_STATUS_PAPEROUT);
 		if (r)
-			DPRINTK (KERN_INFO "%s: Timeout at event 31\n");
+			DPRINTK (KERN_INFO "%s: Timeout at event 31\n",
+				 port->name);
 
 		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 		DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",

Peter �sterlund             [email protected]
Sk�ndalsv�gen 35            http://home1.swipnet.se/~w-15919
S-128 66 Sk�ndal            +46 8 942647
Sweden

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

So then parport_write is probably buggy because it's losing data silenty while the printer is off. So the below is probably a band aid. Really some printer acts in a different way (see the LP_CAREFUL hack in 2.2.x) so it maybe that parport_write is ok on some printer and it would need something like a LP_CAREFUL option to work also on some other printer. Or maybe some parport handshake is badly designed in hardware and it cannot report errors (or maybe there's the hardware compatibility mode that cannot know about LP_CAREFUL to workaround some printer behaviour). In such case your patch is probably the only way to go (but almost certainly for the software compatibility mode it should be possible to report errors via parport_write as we do in 2.2.x).

I see, it makes sense to try to parport_write only when errors goes away, but I think it's nicer to have lp_error or lp_check_status that loops internally in interruptible mode if LP_ABORT isn't set via lptune. probably the code should be restructured a bit.

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

The tunelp man-page seems to think there are printers that need the LP_CAREFUL handling. I also noted that if I disconnect my printer from the computer, the data will no longer be lost. Apparently the printer confuses the parallel port when it is powered off.

What do you think about the following patch? It also works for all the tests mentioned in my previous message.

@@ -188,10 +188,7 @@
 	int error = 0;
 	unsigned int last = lp_table[minor].last_error;
 	unsigned char status = r_str(minor);
-	if ((status & LP_PERRORP) && !(LP_F(minor) & LP_CAREFUL))
-		/* No error. */
-		last = 0;
-	else if ((status & LP_POUTPA)) {
+	if ((status & LP_POUTPA)) {
 		if (last != LP_POUTPA) {
 			last = LP_POUTPA;
 			printk(KERN_INFO "lp%d out of paper\n", minor);

@@ -210,8 +207,7 @@
 		}
 		error = -EIO;
 	} else {
-		last = 0; /* Come here if LP_CAREFUL is set and no
-                             errors are reported. */
+		last = 0; /* Come here if no errors are reported. */
 	}
 
 	lp_table[minor].last_error = last;

@@ -222,6 +218,21 @@
 	return error;
 }
 
+static int lp_wait_ready(int minor)
+{
+	int error = 0;
+	do {
+		error = lp_check_status (minor);
+		if (error && (LP_F(minor) & LP_ABORT))
+			break;
+		if (signal_pending (current)) {
+			error = -EINTR;
+			break;
+		}
+	} while (error);
+	return error;
+}
+
 static ssize_t lp_write(struct file * file, const char * buf,
 		        size_t count, loff_t *ppos)
 {

@@ -259,7 +270,7 @@
 	parport_set_timeout (lp_table[minor].dev,
 			     lp_table[minor].timeout);
 
-	if ((retv = lp_check_status (minor)) == 0)
+	if ((retv = lp_wait_ready (minor)) == 0)
 	do {
 		/* Write the data. */
 		written = parport_write (port, kbuf, copy_size);

@@ -279,9 +290,9 @@
 
 		if (copy_size > 0) {
 			/* incomplete write -> check error ! */
-			int error = lp_check_status (minor);
+			int error = lp_wait_ready (minor);
 
-			if (LP_F(minor) & LP_ABORT) {
+			if (error) {
 				if (retv == 0)
 					retv = error;
 				break;

@@ -453,10 +464,7 @@
 				LP_F(minor) &= ~LP_ABORTOPEN;
 			break;
 		case LPCAREFUL:
-			if (arg)
-				LP_F(minor) |= LP_CAREFUL;
-			else
-				LP_F(minor) &= ~LP_CAREFUL;
+			/* Obsolete */
 			break;
 		case LPWAIT:
 			LP_WAIT(minor) = arg;

@@ -524,7 +524,8 @@
 					     PARPORT_STATUS_PAPEROUT,
 					     PARPORT_STATUS_PAPEROUT);
 		if (r)
-			DPRINTK (KERN_INFO "%s: Timeout at event 31\n");
+			DPRINTK (KERN_INFO "%s: Timeout at event 31\n",
+				 port->name);
 
 		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 		DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",

@@ -20,7 +20,6 @@
 #define LP_NOPA  0x0010
 #define LP_ERR   0x0020
 #define LP_ABORT 0x0040
-#define LP_CAREFUL 0x0080 /* obsoleted -arca */
 #define LP_ABORTOPEN 0x0100
 
 #define LP_TRUST_IRQ_  0x0200 /* obsolete */

Peter �sterlund             [email protected]
Sk�ndalsv�gen 35            http://home1.swipnet.se/~w-15919
S-128 66 Sk�ndal            +46 8 942647
Sweden

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I'm worried somebody needed to disable LP_CAREFUL to print, probably it's not a big deal to keep it. About the lp_wait_ready that's what I had in mind with the "rework" thing and it looks fine. However parport_write can still could silenty discard data, but maybe it can't notice errors with some handshake. I didn't checked the details of the DMA based handshake so Tim needs to comment if this can be considered a final/right fix (I hope it's not ;).

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

On Thu, 4 Jan 2001, Andrea Arcangeli wrote:

: I'm worried somebody needed to disable LP_CAREFUL to print, probably : it's not a big deal to keep it.

I removed it because otherwise I would have had to do twice as many tests to convince myself that all combinations of flags and printer states worked correctly. I can reinsert it if you think that's necessary. But then I don't think the tunelp man page and the comments in the kernel should say that it is obsolete. I think obsolete means "you should never ever have to use this stuff".

: However parport_write can still could silenty discard data, but maybe : it can't notice errors with some handshake.

At least with my printer, it only discards data when it is powered off. If you power off the printer between the times when lp_wait_ready returns and parport_write is called, I think it is ok to lose data. After all, if the power off had occurred a few ms later, the data would already have been in the printer when it was powered off, and then you would have lost data anyway.

: I didn't checked the details of the DMA based handshake so Tim needs : to comment if this can be considered a final/right fix (I hope it's : not ;).

I don't understand the lowlevel parport details, so I can not comment on this.

Peter Österlund             [email protected]
Sköndalsvägen 35            http://home1.swipnet.se/~w-15919
S-128 66 Sköndal            +46 8 942647
Sweden


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Agreed.

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I think that LP_CAREFUL is still needed. There are printers that think that nFault overrides the other error lines and there are printers that drag nFault to 'okay to print' when off. To keep everyone happy (those that want to print to error-line-wreckless printers, and those that want jobs being sent to a printer that's off to wait until the printer comes online) I think we need LP_CAREFUL to remain.

Tim.
*/

I'm afraid that with some (most) printers there's just nothing that can be done about this, to my knowledge.

I wonder where the EIO is coming from though. Grep only shows up ieee1284.c (in parport_read) and daisy.c (in cpp_mux, called at parport init time). Neither of those should be getting triggered.

Tim.
*/

I remeber that too, that's why we still have LP_CAREFUL around.

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I think lp_check_status.

	} else if (!(status & LP_PSELECD)) {
		if (last != LP_PSELECD) {
			last = LP_PSELECD;
			printk(KERN_INFO "lp%d off-line\n", minor);
		}
		error = -EIO;
	} else if (!(status & LP_PERRORP)) {
		if (last != LP_PERRORP) {
			last = LP_PERRORP;
			printk(KERN_INFO "lp%d on fire\n", minor);
		}
		error = -EIO;

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Okay. So what about this patch instead? If the printer is off-line to start with, fall into parport_write anyway (it will just time out and return 0). If LP_ABORT is set, we return -EAGAIN.

Tim.
*/

@@ -213,7 +213,7 @@
 			last = LP_PSELECD;
 			printk(KERN_INFO "lp%d off-line\n", minor);
 		}
-		error = -EIO;
+		error = -EAGAIN;
 	} else if (!(status & LP_PERRORP)) {
 		if (last != LP_PERRORP) {
 			last = LP_PERRORP;

@@ -270,8 +270,11 @@
 	parport_set_timeout (lp_table[minor].dev,
 			     lp_table[minor].timeout);
 
-	if ((retv = lp_check_status (minor)) == 0)
-	do {
+	retv = lp_check_status (minor);
+	if (retv == -EAGAIN && (LP_F(minor) & LP_ABORT) == 0)
+		retv = 0;
+
+	if (retv == 0) do {
 		/* Write the data. */
 		written = parport_write (port, kbuf, copy_size);
 		if (written >= 0) {

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

As noted yesterday falling into parport_write will silenty lose data when the printer is off.

If it's not feasible to make parport_write reliable against power-off printer, then I recommend to loop in interruptible mode before entering the main loop (waiting the printer to power-on) like in latest patch from Peter.

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

(Actually it depends; I think FIFO/DMA paths are fine, but yes, the software implementation can lose data.)

Have I missed a patch? How do you know whether or not the printer is on yet?

As I understand it, you can't guarantee anything about any of the signals when the printer is off, so all you can do is look for 'suspicous' things (like 'no error' and 'paper out'). But some printers do this during normal operation, and hence the LP_CAREFUL switch.

Return -EIO when the printer is on and off-line is a bug, sure enough. That's what the -EAGAIN patch was for, and Peter's patch fixes this too.

But if you want to avoid losing data when your printer is off you need to use LP_CAREFUL, and hope printing still works at all (depends on your printer).

If this goes away:

        if ((status & LP_PERRORP) && !(LP_F(minor) & LP_CAREFUL))
                /* No error. */
                last = 0;

then some people might not be able to print at all.

Tim.
*/

On my printer (HP LJ4MP) if parport_write is called when the printer is off, it will lose data even in DMA mode. I added printk's to verify that when I'm sending a 176 byte file to /dev/lp0, it will be transferred with one call to parport_pc_fifo_write_block_dma and /proc/interrupts says that one new parport interrupt has arrived.

[...]

OK, how about this patch then?

@@ -222,6 +222,21 @@
 	return error;
 }
 
+static int lp_wait_ready(int minor)
+{
+	int error = 0;
+	do {
+		error = lp_check_status (minor);
+		if (error && (LP_F(minor) & LP_ABORT))
+			break;
+		if (signal_pending (current)) {
+			error = -EINTR;
+			break;
+		}
+	} while (error);
+	return error;
+}
+
 static ssize_t lp_write(struct file * file, const char * buf,
 		        size_t count, loff_t *ppos)
 {

@@ -259,7 +274,7 @@
 	parport_set_timeout (lp_table[minor].dev,
 			     lp_table[minor].timeout);
 
-	if ((retv = lp_check_status (minor)) == 0)
+	if ((retv = lp_wait_ready (minor)) == 0)
 	do {
 		/* Write the data. */
 		written = parport_write (port, kbuf, copy_size);

@@ -279,9 +294,9 @@
 
 		if (copy_size > 0) {
 			/* incomplete write -> check error ! */
-			int error = lp_check_status (minor);
+			int error = lp_wait_ready (minor);
 
-			if (LP_F(minor) & LP_ABORT) {
+			if (error) {
 				if (retv == 0)
 					retv = error;
 				break;

@@ -524,7 +524,8 @@
 					     PARPORT_STATUS_PAPEROUT,
 					     PARPORT_STATUS_PAPEROUT);
 		if (r)
-			DPRINTK (KERN_INFO "%s: Timeout at event 31\n");
+			DPRINTK (KERN_INFO "%s: Timeout at event 31\n",
+				 port->name);
 
 		port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
 		DPRINTK (KERN_DEBUG "%s: ECP direction: forward\n",

@@ -20,7 +20,7 @@
 #define LP_NOPA  0x0010
 #define LP_ERR   0x0020
 #define LP_ABORT 0x0040
-#define LP_CAREFUL 0x0080 /* obsoleted -arca */
+#define LP_CAREFUL 0x0080
 #define LP_ABORTOPEN 0x0100
 
 #define LP_TRUST_IRQ_  0x0200 /* obsolete */

Peter �sterlund             [email protected]
Sk�ndalsv�gen 35            http://home1.swipnet.se/~w-15919
S-128 66 Sk�ndal            +46 8 942647
Sweden

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

If you do this, you should probably also return -EAGAIN if the printer is out of paper, otherwise I would still lose data when the printer goes out of paper. Currently it returns -ENOSPC in this situation. I suppose the different return codes were meant as a way for user space to be able to know why printing failed, so that it could take appropriate actions, but maybe this is not used by any programs.

Peter �sterlund             [email protected]
Sk�ndalsv�gen 35            http://home1.swipnet.se/~w-15919
S-128 66 Sk�ndal            +46 8 942647
Sweden

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

They were intended for that, yes, but it's probably better to stick with the 2.2 return codes. Here's a patch to do that. Look okay?

Tim.
*/

2001-01-04 Tim Waugh <[email protected]>

* drivers/char/lp.c: Follow 2.2 behaviour more closely.

@@ -207,7 +207,7 @@
 			last = LP_POUTPA;
 			printk(KERN_INFO "lp%d out of paper\n", minor);
 		}
-		error = -ENOSPC;
+		error = -EIO;
 	} else if (!(status & LP_PSELECD)) {
 		if (last != LP_PSELECD) {
 			last = LP_PSELECD;

@@ -230,7 +230,10 @@
 	if (last != 0)
 		lp_error(minor);
 
-	return error;
+	if (LP_F (minor) & LP_ABORT)
+		return error;
+
+	return 0;
 }
 
 static ssize_t lp_write(struct file * file, const char * buf,

@@ -292,7 +295,7 @@
 			/* incomplete write -> check error ! */
 			int error = lp_check_status (minor);
 
-			if (LP_F(minor) & LP_ABORT) {
+			if (error) {
 				if (retv == 0)
 					retv = error;
 				break;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I assume you meant to return -EAGAIN, not -EIO. However, it doesn't work if the printer is powered off and LP_ABORT is false. In that case lp_write calls lp_check_status, which detects the error, waits 10 seconds, but then returns 0. lp_write then calls parport_write which will happily send the data to the powered off printer, then return success to user space and the data is lost.

Peter �sterlund             [email protected]
Sk�ndalsv�gen 35            http://home1.swipnet.se/~w-15919
S-128 66 Sk�ndal            +46 8 942647
Sweden

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/