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

[grem]

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.