Support for new device, important fix and enhancement to umass.c

grem freebsdusb at bindone.de
Wed Mar 28 15:05:35 UTC 2007


Hi Alexander,

Alexander Leidinger wrote:
> Quoting grem <freebsdusb at 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





More information about the freebsd-usb mailing list