Date: Mon, 30 Jan 2006 16:24:58 -0800 From: Nate Lawson <nate@root.org> To: Florent Thoumie <flz@xbsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/cam/scsi scsi_da.c src/sys/dev/usb umass.c usbdevs Message-ID: <43DEAE5A.4010904@root.org> In-Reply-To: <200601310108.27007.flz@xbsd.org> References: <20060130202806.DCC7916A4CA@hub.freebsd.org> <43DEA5C3.7090002@root.org> <200601310108.27007.flz@xbsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Florent Thoumie wrote:
> On Tuesday 31 January 2006 00:48, Nate Lawson wrote:
>
>>Florent Thoumie wrote:
>>
>>>flz 2006-01-30 20:27:44 UTC
>>>
>>> FreeBSD src repository (ports committer)
>>>
>>> Modified files:
>>> sys/cam/scsi scsi_da.c
>>> sys/dev/usb umass.c usbdevs
>>> Log:
>>> - Add a scsi_da.c and a umass.c quirk for Genesys 6-in-1 Card Reader.
>>>
>>>--- src/sys/cam/scsi/scsi_da.c:1.185 Thu Jan 26 00:35:53 2006
>>>+++ src/sys/cam/scsi/scsi_da.c Mon Jan 30 20:27:44 2006
>>>@@ -427,6 +427,14 @@
>>> {T_DIRECT, SIP_MEDIA_REMOVABLE, "*" , "USB DISK*",
>>> "*"}, /*quirks*/ DA_Q_NO_SYNC_CACHE
>>> },
>>>+ {
>>>+ /*
>>>+ * Genesys 6-in-1 Card Reader
>>>+ * No PR, reported by anders
>>>+ */
>>>+ {T_DIRECT, SIP_MEDIA_REMOVABLE, "Generic*", "STORAGE DEVICE*",
>>>+ "*"}, /*quirks*/ DA_Q_NO_SYNC_CACHE
>>>+ },
>>> };
>>
>>I think it's bad to deviate from the policy we've established for
>>handling quirks. The reason why we need a PR is to track the following
>>info:
>>
>># Output of "camcontrol inquiry yourdevice"
>># Manufacturer name, model number, etc.
>># Transport type (FC, SCSI, USB, Firewire)
>># Output from dmesg for failed attach attempts
>># Output from dmesg for successful attach attempts (after quirk added)
>># Output of "usbdevs -v" with device attached
>># Valid email address
>>
>>An email address or posting is not enough since the user may discard the
>>device later and it's tough to tell from the quirk itself exactly what
>>device was causing the problem. Case in point: the quirk "Generic
>>STORAGE DEVICE" you've added could match any number of devices. Was
>>this only a particular Genesys product or do all their products have
>>this issue? Without a way to track this info, it becomes impossible to
>>decide. PRs are the best current way to track things.
>>
>>http://root.org/~nate/freebsd/scsi/quirks.html
>>
>>Please stick to the policy of collecting the necessary information to
>>help future maintainers.
>
>
> Hum sorry, I thought I had enough information from anders' mail [1]
The email is definitely better than the comment in the code.
> I've used "*" as revision match since that's what is used for all other
> entries, whether particular revisions are mentioned in the comment or not.
I'm not concerned about the revision. I'm concerned about the vendor
(Generic*) and device name (STORAGE DEVICE*). Why are the *'s needed?
(Again, a PR would help track this kind of conversation as shown in
previous PRs about quirks. Submitters often match way too much.)
> Do you want me to create a PR just for tracking purposes?
> [1] http://docs.freebsd.org/cgi/mid.cgi?20060116193024.GA95183
That would be nice, especially since some of the requested info is
missing (dmesg, usbdevs -v). However, if you cited a email in the
commit msg (maybe SMTP Message-ID) such that we could find it in the
future, that would probably be enough. I'm not trying to create a
bureaucracy, just make sure we don't lose information like we used to on
why a quirk was added in the first place.
--
Nate
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?43DEAE5A.4010904>