cvs commit: src/sys/cam/scsi scsi_da.c src/sys/dev/usb umass.c usbdevs

[Florent Thoumie]

On Tuesday 31 January 2006 01:24, Nate Lawson wrote:
> 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?

Seemed common practice reading the other entries.

> (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.

I only mentioned the freebsd-usb mailing list. I'll contact Anders to get 
additional details and I (or he) will fill a PR so that we can add it to the 
comment.

It seems a lot of devices are concerned by the sync cache problem, would it be 
harmful to just remove this part of the code or could there be a way to 
detect if the device supports it or not?

-- 
Florent Thoumie
flz at FreeBSD.org
FreeBSD Committer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20060131/068ce7d9/attachment.bin