Date: Wed, 28 Mar 2007 17:05:29 +0200 From: grem <freebsdusb@bindone.de> To: freebsd-usb@freebsd.org Subject: Re: Support for new device, important fix and enhancement to umass.c Message-ID: <460A8439.70107@bindone.de> In-Reply-To: <20070328080000.bhakk1rou88ww8ks@webmail.leidinger.net> References: <4609D885.8070505@bindone.de> <20070328080000.bhakk1rou88ww8ks@webmail.leidinger.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Alexander, Alexander Leidinger wrote: > Quoting grem <freebsdusb@bindone.de> (from Wed, 28 Mar 2007 04:52:53 > +0200): > > [analysis of the problem] > >> Any feedback is welcome, since I'm not an expert in how USB works/is >> implemented in FreeBSD. > > Please submit this as a problem report. Quirks have to be registered in > GNATS before we can commit them so that we are able to reevaluate them > if the need arises. I thought that is only true for new quirks (IGNORE_RESIDUE is an already existing quirk). Do you have a link that describes how to do it (in the least possible amount of time), PR + potentially GNATS. Then again, I would have to file different PRs, cause one is critical while the others are feature requests? > >> @@ -1665,6 +1673,8 @@ >> USETDW(sc->csw.dCSWSignature, >> CSWSIGNATURE); >> } >> >> + if (sc->quirks & IGNORE_RESIDUE) >> + USETDW(sc->csw.dCSWDataResidue, 0); >> int Residue; >> Residue = UGETDW(sc->csw.dCSWDataResidue); >> if (Residue == 0 && > > Wrong indent for the USETDW line. Hey c'mon, copy and paste :) > I don't know much about the USB code. > If the residue is not used somewhere else, wouldn't it be better to do > "if quirk set the Residue variable to 0 else get it from the device" The logic is: "If quirk is set, calculate residue, otherwise get it and if it's not 0 calculate it" Although maybe the original logic is suboptimal, and it would be best todo sth like: int Residue; if ((sc->quirks & IGNORE_RESIDUE) || !(Residue = UGETDW(sc->csw.dCSWDataResidue))) Residue = sc->transfer_datalen - sc->transfer_actlen; Since the extra check for (sc->transfer_datalen - sc->transfer_actlen) in the original code seems redundant. /michael
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?460A8439.70107>