From owner-freebsd-usb@FreeBSD.ORG Wed Mar 28 15:05:35 2007 Return-Path: X-Original-To: freebsd-usb@freebsd.org Delivered-To: freebsd-usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 6A84F16A401 for ; Wed, 28 Mar 2007 15:05:35 +0000 (UTC) (envelope-from freebsdusb@bindone.de) Received: from mail.bindone.de (sidewinder.bindone.de [62.146.109.98]) by mx1.freebsd.org (Postfix) with SMTP id C296D13C457 for ; Wed, 28 Mar 2007 15:05:34 +0000 (UTC) (envelope-from freebsdusb@bindone.de) Received: (qmail 53482 invoked from network); 28 Mar 2007 15:02:32 -0000 Received: from unknown (HELO bombat.bindone.de) (84.151.241.193) by mail.bindone.de with SMTP; 28 Mar 2007 15:02:32 -0000 Message-ID: <460A8439.70107@bindone.de> Date: Wed, 28 Mar 2007 17:05:29 +0200 From: grem User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.8.1.2) Gecko/20070318 SeaMonkey/1.1.1 MIME-Version: 1.0 To: freebsd-usb@freebsd.org References: <4609D885.8070505@bindone.de> <20070328080000.bhakk1rou88ww8ks@webmail.leidinger.net> In-Reply-To: <20070328080000.bhakk1rou88ww8ks@webmail.leidinger.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: Support for new device, important fix and enhancement to umass.c X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Mar 2007 15:05:35 -0000 Hi Alexander, Alexander Leidinger wrote: > Quoting grem (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