Andrea Arcangeli (andrea@e-mind.com)
Mon, 10 May 1999 01:29:24 +0200 (CEST)
On Sun, 9 May 1999 Andries.Brouwer@cwi.nl wrote:
>So, now we all have made our points of view clear, let us
>return to kernel and tunelp itself.
Yowoowww finally a productive email ;). I hope we'll continue this way.
>2.0.36 has
> #define LPSTRICT 0x060f
>2.1.131 (and 2.2.*) has
> #define LPTRUSTIRQ 0x060f
>
>
>This is a bug. It makes it a bit awkward to release an improved
>tunelp, entirely regardless of who does the releasing, since
>tunelp would have to check for the version of the running kernel,
>and the man page would need a lengthy explanation.
An uname(2) would workaround the problem but I agree that the thing should
be fixed in the kernel.
>Both ioctls are not precisely what seems to be needed,
The trustirq bit allow lp to go ahead sending the next char even if the
printer still tell to be BUSY. Otherwise lp may have to fast-polling after
every irq with most recent printers. This feature should break the
printing while instead with my printer (Epson Stylus Color) it decreases
the CPU load generated by the printing task.
The handshake of the new printers is far from the the original-centronics
one. Now the ieee1284 compat-handshake is a bit different from the
original centronics handshake. I suppose they thought that the changes are
minor and so no-software would notice them.
Incidentally the 2.0.36 and the early 2.1.x lp.c noticed that the printer
wasn't talking the original centronics handshake and that's why the lp of
2.0.x and early 2.1.x doesn't work correctly on all printer if it's used
with irqs.
When I started playing with lp.c I only known that lp.c wasn't working on
all printers when using irqs. So I did reverse engeneering of the new
handshake and I discovered the exact bit that was broking the irq printing
on some printer. Then I changed lp to handle the new handshake.
What I called "buggy handshake of some HP and Epson printer" is really the
ieee1284 compat handshake. But at the time I did reverse engeneering I
couldn't know that.
Here it is the new compat (""backwards compatible"") handshake
cut-and-paste from lp.c:
* ___
* ACK _______________ ___________
* |__|
* ____
* BUSY _________ _______
* |____________|
When the ack line goes low or high (it depends on the type of parallel
port) we get the irq. But when the printing task gets the wakeup from the
irq, then it sees the printer still busy because BUSY is still low (see
the graph). So lp must start polling even if we are in the irq-driven
printing (for the record the 2.0.x lp instead goes watiting for an irq
again and that's why the old lp stalls forever).
It make _nonono_ sense that the irq happens before the printer become
ready. It's simply a bogus design of the irq handshake.
The right thing the hardware should do, would be to generate the irq when
then lp can go ahead without having to poll, this would give far more
sense to the irq printing.
Since I thought that people who designs the specs is not so stupid as it
seems by looking the handshake timings, then I thought they was trying
fool any non-proprietary driver. If they tell the hardware to looks like
to be still busy for some time after the wakeup, then any standard driver
will waste time in a polling-loop so wasting CPU of the host machine.
So I tried to force lp to go ahead after the irq even if the printer told
to be busy, and it worked fine. This is what the TRUSTIRQ bit does. I
don't know if you'll like it or not but it's a backdoor to get
performances. It's not the default obviously, since in theory it should
break the printing.
Since it was possible to detect any hardware that can be improved (or that
may break as it should) using the trustirq feature, then I also added a
printk(KERN_WARNING), (hmmm really it should be a KERN_INFO and not a
KENR_WARNING... little mistake... ;) the first time lp is going to start a
polling cycle after an irq.
>In 2.0.35 some code was inserted to replace a waiting loop by
>some code waiting for NBSY=0. People complained that the Epson 800
>printer became terribly slow and in 2.0.36 the patch was reverted,
>with an ioctl LPSTRICT to select the 2.0.35 behaviour.
Here it is the interesting part of 2.0.36:
--- v2.0.35/linux/drivers/char/lp.c Sun Nov 15 10:49:34 1998
+++ linux/drivers/char/lp.c Sun Nov 15 10:32:54 1998
@@ -89,14 +89,18 @@
while(wait != LP_WAIT(minor)) wait++;
/* control port takes strobe high */
outb_p(( LP_PSELECP | LP_PINITP | LP_PSTROBE ), ( LP_C( minor )));
- /* Wait until NBUSY line goes high */
- count = 0;
- do {
- status = LP_S(minor);
- count++;
- if (need_resched)
- schedule();
- } while (LP_READY(minor, status) && (count<LP_CHAR(minor)));
+
+ if(LP_F(minor)&LP_STRICT)
+ {
+ /* Wait until NBUSY line goes high */
+ count = 0;
+ do {
+ status = LP_S(minor);
+ count++;
+ } while (LP_READY(minor, status) && (count<LP_CHAR(minor)));
+ }
+ else while(wait) wait--;
+
/* take strobe low */
outb_p(( LP_PSELECP | LP_PINITP ), ( LP_C( minor )));
/* update waittime statistics */
The 2.0.35 behaviour was bogus because it may issue a schedule() in a
fast-path that should take only 0.5usec to run.
>The first question is: this 2.0.35 behaviour - is it strictly according
>to the specs? Not according to my notes, which say:
Agreed whole code with LPSTRICT has to be replaced with udelay(1).
>In the 2.0.36 code I do not see any waiting for a definite period
>of time - just strange loops like
> wait = 1000;
> while(wait) wait--;
>with an execution time very much dependent on processor and alignment.
Yes: udelay(1) is the right things to do (see lp_wait in 2.2.x lp.c).
The whole point is that even the strict bit is not enough since the
printer may not have the time to put busy low after we set the strobe
high.
>Andrea's code in 2.2.* looks much better.
Thanks ;). I worked together with Tim and Philip (just to be more precise ;).
I think the LP_STRICT should be killed and we should replace it with the
proper udelay as 2.2.x does.
This is the right thing to do according to me. Patch against 2.0.36 but
it's completly untested (nor I tried to compile it).
--- /home/andrea/devel/kernel-tree/linux-2.0.36/drivers/char/lp.c Mon Dec 7 18:58:46 1998
+++ lp.c Mon May 10 01:18:28 1999
@@ -64,6 +64,8 @@
return LP_S(minor);
}
+#define lp_wait(minor) udelay(LP_WAIT(minor))
+
static inline int lp_char_polled(char lpchar, int minor)
{
int status, wait = 0;
@@ -86,23 +88,17 @@
stats->chars++;
/* must wait before taking strobe high, and after taking strobe
low, according spec. Some printers need it, others don't. */
- while(wait != LP_WAIT(minor)) wait++;
+ lp_wait(minor);
/* control port takes strobe high */
outb_p(( LP_PSELECP | LP_PINITP | LP_PSTROBE ), ( LP_C( minor )));
- if(LP_F(minor)&LP_STRICT)
- {
- /* Wait until NBUSY line goes high */
- count = 0;
- do {
- status = LP_S(minor);
- count++;
- } while (LP_READY(minor, status) && (count<LP_CHAR(minor)));
- }
- else while(wait) wait--;
+ lp_wait(minor);
/* take strobe low */
outb_p(( LP_PSELECP | LP_PINITP ), ( LP_C( minor )));
+
+ lp_wait(minor);
+
/* update waittime statistics */
if (count > stats->maxwait) {
#ifdef LP_DEBUG
@@ -137,13 +133,13 @@
stats->chars++;
/* must wait before taking strobe high, and after taking strobe
low, according spec. Some printers need it, others don't. */
- wait = 0;
- while(wait != LP_WAIT(minor)) wait++;
+ lp_wait(minor);
/* control port takes strobe high */
outb_p(( LP_PSELECP | LP_PINITP | LP_PSTROBE ), ( LP_C( minor )));
- while(wait) wait--;
+ lp_wait(minor);
/* take strobe low */
outb_p(( LP_PSELECP | LP_PINITP ), ( LP_C( minor )));
+ lp_wait(minor);
/* update waittime statistics */
if (count) {
if (count > stats->maxwait)
@@ -439,12 +435,6 @@
LP_F(minor) |= LP_CAREFUL;
else
LP_F(minor) &= ~LP_CAREFUL;
- break;
- case LPSTRICT:
- if (arg)
- LP_F(minor) |= LP_STRICT;
- else
- LP_F(minor) &= ~LP_STRICT;
break;
case LPWAIT:
LP_WAIT(minor) = arg;
--- /home/andrea/devel/kernel-tree/linux-2.0.36/include/linux/lp.h Mon Dec 7 18:58:47 1998
+++ lp.h Mon May 10 01:22:29 1999
@@ -20,7 +20,6 @@
#define LP_ABORT 0x0040
#define LP_CAREFUL 0x0080
#define LP_ABORTOPEN 0x0100
-#define LP_STRICT 0x0200
/* timeout for each character. This is relative to bus cycles -- it
* is the count in a busy loop. THIS IS THE VALUE TO CHANGE if you
@@ -39,7 +38,7 @@
* I'll initialize it to 0.
*/
-#define LP_INIT_WAIT 0
+#define LP_INIT_WAIT 1
/* This is the amount of time that the driver waits for the printer to
* catch up when the printer's buffer appears to be filled. If you
@@ -71,7 +70,6 @@
#define LPRESET 0x060c /* reset printer */
#define LPGETSTATS 0x060d /* get statistics (struct lp_stats) */
#define LPGETFLAGS 0x060e /* get status flags */
-#define LPSTRICT 0x060f /* enable/disable strict compliance */
/* timeout for printk'ing a timeout, in jiffies (100ths of a second).
This is also used for re-checking error conditions if LP_ABORT is
Comments?
Andrea Arcangeli
-- To unsubscribe, send mail to: linux-parport-request@torque.net --
-- with the single word "unsubscribe" in the body of the message. --
This archive was generated by hypermail 2.0b3 on Sun 09 May 1999 - 20:00:22 EDT