Date: Thu, 29 Mar 2007 08:50:42 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: freebsd-usb@freebsd.org Subject: Re: usb/110989: [patch] Handling of quirk IGNORE_RESIDUE is umass.c is broken Message-ID: <200703290850.42858.hselasky@c2i.net> In-Reply-To: <200703290017.l2T0HXe9037447@www.freebsd.org> References: <200703290017.l2T0HXe9037447@www.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 29 March 2007 02:17, Michael Gmelin wrote:
> >Number: 110989
> >Category: usb
> >Synopsis: [patch] Handling of quirk IGNORE_RESIDUE is umass.c is
> > broken Confidential: no
> >Severity: serious
> >Priority: medium
> >Responsible: freebsd-usb
> >State: open
> >Quarter:
> >Keywords:
> >Date-Required:
> >Class: sw-bug
> >Submitter-Id: current-users
> >Arrival-Date: Thu Mar 29 00:30:09 GMT 2007
> >Closed-Date:
> >Last-Modified:
> >Originator: Michael Gmelin
> >Release: FreeBSD 6.2-RELEASE-p3 i386
> >Organization:
>
> /bin/done digital solutions GmbH
>
> >Environment:
>
> FreeBSD bombat.bindone.de 6.2-RELEASE-p3 FreeBSD 6.2-RELEASE-p3 #21: Wed
> Mar 28 04:08:44 CEST 2007
> root@bombat.bindone.de:/usr/src/sys/i386/compile/bombat i386
>
> >Description:
>
> I had to add a new device to usbdevs/umass.c which requires the
> IGNORE_RESIDUE quirk to be set (extra PR will follow). It didn't work,
> because IGNORE_RESIDUE isn't handled properly in umass.c (it isn't really
> handled at all, since Residue is set in lines are 1668-1672 in umass.c in
> the following was:
>
> int Residue;
> Residue = UGETDW(sc->csw.dCSWDataResidue);
> if (Residue == 0 &&
> sc->transfer_datalen - sc->transfer_actlen != 0)
> Residue = sc->transfer_datalen - sc->transfer_actlen;
>
> The patch below fixes this issue (tested and proven to work).
>
> >How-To-Repeat:
>
> Use a really broken USB device which returns "random" values for
> sc->csw.dCSWDataResidue (like devices that use the SuperTop IDEDEVICE USB
> controller, e.g. the ICY BOX IB-220U-Wh). Every attempt to use the device
> will lead to error messages, like:
>
> dd if=/dev/zero of=/dev/da0 count=10
> da0: end of device
>
> or
>
> disklabel da0
> read: Unknown error
> etc.
>
> >Fix:
>
> Apply the attached patch, which forces residue to be calculated if
> IGNORE_RESIDUE is set.
>
>
>
> Patch attached with submission follows:
>
> --- umass.c.orig Thu Mar 29 02:07:04 2007
> +++ umass.c Thu Mar 29 02:08:06 2007
> @@ -1666,7 +1666,10 @@
> }
>
> int Residue;
> - Residue = UGETDW(sc->csw.dCSWDataResidue);
> + if (sc->quirks & IGNORE_RESIDUE)
> + Residue = 0;
> + else
> + Residue = UGETDW(sc->csw.dCSWDataResidue);
> if (Residue == 0 &&
> sc->transfer_datalen - sc->transfer_actlen != 0)
> Residue = sc->transfer_datalen - sc->transfer_actlen;
>
> >Release-Note:
> >Audit-Trail:
> >Unformatted:
>
I think that we should auto-detect bad residue. If the residue is some fixed
constant way off the actual length, then maybe something like this is better:
Residue = UGETDW(sc->csw.dCSWDataResidue);
if (Residue > sc->transfer_datalen) {
Residue = sc->transfer_datalen - sc->transfer_actlen;
}
--HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200703290850.42858.hselasky>