[BUG] 2.4.18-pre/rc broke PLIP

[BUG] 2.4.18-pre/rc broke PLIP

[Mikael Pettersson]

[sent to Tim Waugh and Marcelo previously, but of course
I managed to put a typo in the cc: to lkml]

Someone (sorry I forgot who) reported having problems with
PLIP in recent kernels. I've done some testing and can confirm
that PLIP worked up to 2.4.17 but broke in 2.4.18-pre/rc.
The only thing PLIP does is put "plip0: transmit timeout(1,87)"
messages in the kernel log.

After a lot of testing I've narrowed it down to the following
hunk in the 2.4.18-rc4 patch:

--- linux.orig/drivers/parport/parport_pc.c	Mon Feb 18 20:18:40 2002
+++ linux/drivers/parport/parport_pc.c	Mon Jan 14 19:08:50 2002
@@ -2212,7 +2233,7 @@
 	}
 	memcpy (ops, &parport_pc_ops, sizeof (struct parport_operations));
 	priv->ctr = 0xc;
-	priv->ctr_writable = 0xff;
+	priv->ctr_writable = ~0x10;
 	priv->ecr = 0;
 	priv->fifo_depth = 0;
 	priv->dma_buf = 0;

If I back this hunk out, PLIP starts working again.
Is this fix sufficient or is there something else that need fixing?

/Mikael

Re: [BUG] 2.4.18-pre/rc broke PLIP

[Bill Davidsen]

On Mon, 25 Feb 2002, Mikael Pettersson wrote:

> Someone (sorry I forgot who) reported having problems with
> PLIP in recent kernels. I've done some testing and can confirm
> that PLIP worked up to 2.4.17 but broke in 2.4.18-pre/rc.
> The only thing PLIP does is put "plip0: transmit timeout(1,87)"
> messages in the kernel log.
> 
> After a lot of testing I've narrowed it down to the following
> hunk in the 2.4.18-rc4 patch:
> 
> --- linux.orig/drivers/parport/parport_pc.c	Mon Feb 18 20:18:40 2002
> +++ linux/drivers/parport/parport_pc.c	Mon Jan 14 19:08:50 2002
> @@ -2212,7 +2233,7 @@
>  	}
>  	memcpy (ops, &parport_pc_ops, sizeof (struct parport_operations));
>  	priv->ctr = 0xc;
> -	priv->ctr_writable = 0xff;
> +	priv->ctr_writable = ~0x10;
>  	priv->ecr = 0;
>  	priv->fifo_depth = 0;
>  	priv->dma_buf = 0;
> 
> If I back this hunk out, PLIP starts working again.
> Is this fix sufficient or is there something else that need fixing?

  I'm still looking at this, trying to figure out what they were trying to
do here... but I note the same code in parport_gsc.c was NOT
changed. Putting it back makes it work for me, but the code is not long on
comments, and digging up all the data sheets for various supported chips
is not a casual job.

  I'm going to browse the code more this week, the bit not set isn't
documented where I've looked :-(

  Anyway, back to working, perhaps there was a note with the patch giving
some hint of what it was doing?

-- 
bill davidsen <davidsen@tmr.com> CTO, TMR Associates, Inc
  Programming without software engineering is like sculpting with a chain
saw. The very talented can produce a work of art, the mediocre wind up with
a misshapen lump in a pile of rubble, and in neither case does the end
result have more than a passing resemblance to the original intent.

Re: [BUG] 2.4.18-pre/rc broke PLIP

[Tim Waugh]

[-- Attachment #1: Type: text/plain, Size: 199 bytes --]

On Tue, Mar 05, 2002 at 11:36:59AM -0500, Bill Davidsen wrote:

>   I'm still looking at this, trying to figure out what they were trying to
> do here...

Does 2.4.19-pre2 not work for you?

Tim.
*/

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [BUG] 2.4.18-pre/rc broke PLIP

[Bill Davidsen]

On Tue, 5 Mar 2002, Tim Waugh wrote:

> On Tue, Mar 05, 2002 at 11:36:59AM -0500, Bill Davidsen wrote:
> 
> >   I'm still looking at this, trying to figure out what they were trying to
> > do here...
> 
> Does 2.4.19-pre2 not work for you?

1 - didn't try, I checked that the patch had not been reverted, and
    assumed that if it was broken and not changed it was broken still.
    And I only looked in pre2-ac2, if it was fixed and Alan patched it
    back broken.
2 - understanding vast stretches of uncommented code you may need to
    change is worthwhile reading. The corolary is that comments are worthwhile
    typing.

  I'll try to build a plain 19-pre2 kernel after I do a diff with the
2.4.18 to see that something has really changed. Thanks.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: [BUG] 2.4.18-pre/rc broke PLIP

[Tim Waugh]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

On Tue, Mar 05, 2002 at 04:20:25PM -0500, Bill Davidsen wrote:

> 1 - didn't try, I checked that the patch had not been reverted, and
>     assumed that if it was broken and not changed it was broken still.
>     And I only looked in pre2-ac2, if it was fixed and Alan patched it
>     back broken.

Well, try it.  The correct patch is in there.

> 2 - understanding vast stretches of uncommented code you may need to
>     change is worthwhile reading. The corolary is that comments are worthwhile
>     typing.

(You must have missed the ChangeLog.)

Tim.
*/

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [BUG] 2.4.18-pre/rc broke PLIP

[Bill Davidsen]

On Tue, 5 Mar 2002, Tim Waugh wrote:

> On Tue, Mar 05, 2002 at 04:20:25PM -0500, Bill Davidsen wrote:
> 
> > 1 - didn't try, I checked that the patch had not been reverted, and
> >     assumed that if it was broken and not changed it was broken still.
> >     And I only looked in pre2-ac2, if it was fixed and Alan patched it
> >     back broken.
> 
> Well, try it.  The correct patch is in there.

  Interesting, doesn't work for me, and I have a note from Alan Cox which
seems to indicate it doesn't work for him, either.
 
> > 2 - understanding vast stretches of uncommented code you may need to
> >     change is worthwhile reading. The corolary is that comments are worthwhile
> >     typing.
> 
> (You must have missed the ChangeLog.)

I have looked in the changelog in the driver directory, for ac2, and for
19-pre2, and find nothing about the bits used in the control registers.
Nor would I expect it to, this is in the chip spec sheets, and if I had
written the code would be in very terse form in the definition of the data
structure as well.

I'm still looking for the spec sheets, somewhat limited by time. I found
only the definition of the 0x20 bit in the writable mask, and the problem
seems to stem from the change in init from "0xff" to "~0x10" at least on
one machine. Before I go offering that as a solution I want to understand
what the bits are doing (or at least intended to do). I try very hard not
to offer "this works for me but I don't know why" fixes.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: [BUG] 2.4.18-pre/rc broke PLIP

[Stas Sergeev]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

Hello.

Tim Waugh wrote:
> Does 2.4.19-pre2 not work for you?

I had the similar problem with plip at 2.4.18
and 2.4.19-pre2 seems to fix it for me (though
I have seen a comments that it doesn't work for
others).
But I had another problem with plip for years,
which was not fixed by 2.4.19-pre2:
if I do `ifdown plip0` and then `ifup plip0`,
plip no longer works and the kernel logs timeouts,
just like in 2.4.18.
To work around the problem I removed "DISABLE(dev-irq);"
from plip_close(), and it works nice since.
So, if you are already on this, could you please
look at the attached "patch" and make a real fix
(if this is not the one already)?

[-- Attachment #2: plip.diff --]
[-- Type: text/plain, Size: 514 bytes --]

--- linux/drivers/net/plip.c	Sun Sep 30 23:26:07 2001
+++ linux/drivers/net/plip.c	Thu Mar  7 02:47:43 2002
@@ -163,7 +163,7 @@
 static int plip_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd);
 static int plip_preempt(void *handle);
 static void plip_wakeup(void *handle);
-\f
+
 enum plip_connection_state {
 	PLIP_CN_NONE=0,
 	PLIP_CN_RECEIVE,
@@ -1177,7 +1177,6 @@
 	struct plip_local *rcv = &nl->rcv_data;
 
 	netif_stop_queue (dev);
-	DISABLE(dev->irq);
 	synchronize_irq();
 
 	if (dev->irq == -1)