Subject: Re: Fix for bug in vr(4)
To: Julio M. Merino Vidal <jmmv84@gmail.com>
From: Jachym Holecek <freza@liberouter.org>
List: tech-kern
Date: 01/22/2005 18:42:22
> I don't know why this happens, but ignoring these packets avoids the
> crashes.  However, just ignoring them leaves the card in a state which
> is inconsistent.  The driver will keep "receiving" these packets
> periodically and the card does not work (it won't transmit anything).

Haven't read the datasheet, but it would seem that enqueing packet
with VR_RXSTAT_RLINK is chip's way to inform you about some very
important event (such that it justifies emitting a fake zero length
packet). The chip probably expects you to take proper steps. But that's
just guessing, it would certainly be worth finding out what VR_RXSTAT_RLINK
means.

> Issuing a reset command after receiving any of these zero-length
> packets solves the problem.  After that, the driver does not receive
> them any more and the card works fine.  (I.e., the conditional gets
> triggered only once.)
> 
> Here is the patch:
> 
> Index: if_vr.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/pci/if_vr.c,v
> retrieving revision 1.71
> diff -u -r1.71 if_vr.c
> --- if_vr.c	13 Jan 2005 14:51:28 -0000	1.71
> +++ if_vr.c	22 Jan 2005 16:23:23 -0000
> @@ -650,6 +650,13 @@
>  
>  		/* No errors; receive the packet. */
>  		total_len = VR_RXBYTES(le32toh(d->vr_status));
> +		if (total_len == 0) {
> +			printf("%s: got packet of zero length; status = 0x%x\n",
> +			       sc->vr_dev.dv_xname, d->vr_status);
> +			printf("%s: restarting\n", sc->vr_dev.dv_xname);
> +			(void) vr_init(ifp);
> +			continue;
> +		}
>  
>  #ifdef __NO_STRICT_ALIGNMENT

Hmm, I would do the following instead, since length zero doesn't seem to
be the root of the problem. Also casts away one unnecessary le32toh().

	Regards,
		-- Jachym Holecek

--- if_vr.c     2005-01-13 15:51:28.000000000 +0100
+++ if_vr.c.new 2005-01-22 18:37:01.000000000 +0100
@@ -645,11 +645,18 @@
                        continue;
                }
 
+               if (rxstat & VR_RXSTAT_RLINK) {
+                       printf("%s: VR_RXSTAT_RLINK, resetting chip\n",
+                           sc->vr_dev.dv_xname);
+                       (void) vr_init(ifp);
+                       continue ;
+               }
+
                bus_dmamap_sync(sc->vr_dmat, ds->ds_dmamap, 0,
                    ds->ds_dmamap->dm_mapsize, BUS_DMASYNC_POSTREAD);
 
                /* No errors; receive the packet. */
-               total_len = VR_RXBYTES(le32toh(d->vr_status));
+               total_len = VR_RXBYTES(rxstat);