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

grem freebsdusb at bindone.de
Thu Feb 28 20:55:19 UTC 2008


grem wrote:
> Hi,
> 
> I'll continue in English, just in case someone else is interested in this thread...
> 
> Alexander Leidinger wrote:
>> Quoting grem <freebsdusb at bindone.de> (Wed, 28 Mar 2007 15:46:48 +0200):
>>
>>> 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.
>> Ok, ich formuliere um: Ein Quirk Eintrag für ein Device hat einen PR zu
>> haben. Du hast unten einen neues Device fuer den Quirk also brauchen
>> wir noch einen PR Eintrag.
> Ok, so all I have todo is find out how to post PRs (but I'm sure freebsd.org can help)
> 
>>> Then again, I would have to file different PRs, cause one is critical while the others are
>>> feature requests?
>> Zwei PR wäre gut. Einmal der, der das Quirk-Handling fixt, und dann den
>> der den Quirk für Dein Device einträgt. Letzterer sollte den ersten als
>> Abhängigkeit referenzieren (es kann ein bischen dauern bis die Mail von
>> GNATS mit der PR Nummer des ersten PR ankommt).
> ok
> 
>>>>> @@ -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;
>> Ich dachte an
>> ---snip---
>> int Residue;
>> if (sc->quirks & IGNORE_RESIDUE)
>>         Residue = 0;
>> else
>>        Residue = UGETDW(sc->csw.dCSWDataResidue); 
>> ---snip---
>>
>> Und der Rest unverändert. Ist das nicht OK?
> Geht natuerlich genauso :)
> The question is, if it wouldn't be better to fix sc->csw.dCSWDataResidue to the correct
> value, because it might be used somewhere else in the future and introduce obscure bugs
> (right now it's only used in a debug function). But that's beyond my knowledge of the sources.
> 
>> Bye,
>> Alexander.
>>
> bye
> michael
> 
> ps: hab grad erfolgreich einen 100GB dump/restore cycle durchgefuehrt, laeuft also stabil.
> 

Is there any news on these issues (wasn't following the thread any more). Just checked the
7.0 sources and didn't find the device in here. I thought I opened up a PR back then, but
can't find it anywhere...

regards
michael


More information about the freebsd-usb mailing list