Re: usb/95173: [umass] [patch] cannot mount external usb harddisk VIA Technologies Inc. USB 2.0 IDE Bridge

2007-12-02 Thread Ben Kelly 
The following reply was made to PR usb/95173; it has been noted by GNATS.

From: Ben Kelly <[EMAIL PROTECTED]>
To: [EMAIL PROTECTED],  [EMAIL PROTECTED]
Cc:  
Subject: Re: usb/95173: [umass] [patch] cannot mount external usb harddisk
 VIA Technologies Inc. USB 2.0 IDE Bridge
Date: Sun, 02 Dec 2007 16:45:54 -0500

 This is a multi-part message in MIME format.
 --010907000802070702090307
 Content-Type: text/plain; charset=ISO-8859-1; format=flowed
 Content-Transfer-Encoding: 7bit
 
 I bought a Rocketfish external hard drive enclosure at Best Buy a couple 
 weeks ago and ran into this same problem.  I've streamlined James's 
 patch and updated it for 7.0-BETA3.  In addition, I modified it to fake 
 a successful sync instead of returning an error.  This avoids spamming 
 logs with failure messages and also follows the precedent set by the 
 NO_INQUIRY quirk.  As far as I can tell, this has the same end effect as 
 setting the DA_Q_NO_SYNC_CACHE quirk in the cam layer, so I believe it 
 should be equally safe.
 
 --010907000802070702090307
 Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0";
  name="via_umass.patch"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline;
  filename="via_umass.patch"
 
 --- /usr/src/sys/dev/usb/umass.c   2007-07-05 05:26:08.0 +
 +++ umass.c2007-12-02 18:45:27.576058092 +
 @@ -323,6 +323,12 @@
 * sector number.
 */
  # define READ_CAPACITY_OFFBY1 0x2000
 +  /* Device cannot handle a SCSI synchronize cache command.  Normally
 +   * this quirk would be handled in the cam layer, but for IDE bridges
 +   * we need to associate the quirk with the bridge and not the
 +   * underlying disk device.  This is handled by faking a success result.
 +   */
 +# define NO_SYNCHRONIZE_CACHE 0x4000
  };
  
  static struct umass_devdescr_t umass_devdescrs[] = {
 @@ -804,6 +810,10 @@
  UMASS_PROTO_SCSI | UMASS_PROTO_BBB,
  NO_QUIRKS
},
 +  { USB_VENDOR_VIA, USB_PRODUCT_VIA_USB2IDEBRIDGE, RID_WILDCARD,
 +UMASS_PROTO_SCSI | UMASS_PROTO_BBB,
 +NO_SYNCHRONIZE_CACHE
 +  },
{ USB_VENDOR_VIVITAR, USB_PRODUCT_VIVITAR_35XX, RID_WILDCARD,
  UMASS_PROTO_SCSI | UMASS_PROTO_BBB,
  NO_INQUIRY
 @@ -2878,6 +2888,15 @@
xpt_done(ccb);
return;
}
 +  if ((sc->quirks & NO_SYNCHRONIZE_CACHE) &&
 +  rcmd[0] == SYNCHRONIZE_CACHE) {
 +  struct ccb_scsiio *csio = &ccb->csio;
 +
 +  csio->scsi_status = SCSI_STATUS_OK;
 +  ccb->ccb_h.status = CAM_REQ_CMP;
 +  xpt_done(ccb);
 +  return;
 +  }
if ((sc->quirks & FORCE_SHORT_INQUIRY) &&
rcmd[0] == INQUIRY) {
csio->dxfer_len = SHORT_INQUIRY_LENGTH;
 --- /usr/src/sys/dev/usb/usbdevs   2007-11-28 06:10:16.0 +
 +++ usbdevs2007-12-02 18:44:39.873614696 +
 @@ -2229,6 +2229,9 @@
  /* U.S. Robotics products */
  product USR USR5423   0x0121  USR5423 WLAN
  
 +/* VIA Technologies products */
 +product VIA USB2IDEBRIDGE 0x6204  USB 2.0 IDE Bridge
 +
  /* VidzMedia products */
  product VIDZMEDIA MONSTERTV   0x4fb1  MonsterTV P2H
  
 
 --010907000802070702090307--
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

[patch] possible bug in umass_scsi_transform

2007-12-02 Thread Ben Kelly 
While looking at usb/95173 I noticed what appears to be a bug in 
umass_scsi_transform().  Specifically, a TEST_UNIT_READY command will 
have the FORCE_SHORT_INQUIRY quirk logic applied to it due to the switch 
statement fall-through.  Unless these two commands are equivalent in 
some way, it would seem that you would only want to apply the 
FORCE_SHORT_INQUIRY quirk to INQUIRY commands.


I'm attaching a possible fix for the problem, but I cannot test it as I 
don't have any devices requiring these quirks.


Anyway, I just thought I would mention it.  I'm not that familiar with 
the code so its certainly possible I am off base here.


Thanks.

- Ben

--- /usr/src/sys/dev/usb/umass.c2007-12-02 18:47:00.466926284 +
+++ umass.c 2007-12-02 22:07:44.896977086 +
@@ -3297,7 +3297,7 @@
(*rcmd)[4] = SSS_START;
return 1;
}
-   /* fallthrough */
+   break;
case INQUIRY:
/* some drives wedge when asked for full inquiry 
information. */

if (sc->quirks & FORCE_SHORT_INQUIRY) {
@@ -3306,12 +3306,14 @@
(*rcmd)[4] = SHORT_INQUIRY_LENGTH;
return 1;
}
-   /* fallthrough */
+   break;
default:
-   *rcmd = cmd;/* We don't need to copy it */
-   *rcmdlen = cmdlen;
+   break;
}

+   *rcmd = cmd;/* We don't need to copy it */
+   *rcmdlen = cmdlen;
+
return 1;
 }
 /* RBC specific functions */
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Re: FreeBSD P4 USB project - precompiled kernels available for download

2007-12-28 Thread Ben Kelly 


On Dec 28, 2007, at 8:14 PM, Hans Petter Selasky wrote:
I took the time today to crossbuild a couple of kernels based on  
the FreeBSD
P4 USB Project using the "GENERIC" kernel config so that people can  
try how it
works. The P4 USB Project is currently based on FreeBSD 8.x. I have  
not
tested if the precompiled kernels work on FreeBSD 6.x or FreeBSD  
7.x, but I

presume yes. Things that I want you to try out:


Do these images match the current code in the subversion repository  
referenced at the following page?


http://www.selasky.org/hans_petter/usb4bsd/index.html

It didn't look like there had been any USB changes in there  
recently.  Has the USB development moved completely to p4?


I would like to try the new stack, but I am guessing I will need to  
patch it for usb/95173 in order to use my umass device.


Thanks for working on this!

- Ben

___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Re: FreeBSD P4 USB project - precompiled kernels available for download

2007-12-29 Thread Ben Kelly 

On Dec 29, 2007, at 3:38 AM, Hans Petter Selasky wrote:

On Saturday 29 December 2007, Ben Kelly wrote:

It didn't look like there had been any USB changes in there
recently.  Has the USB development moved completely to p4?


Just for the time being. I will update my SVN repo soon so that you  
can get

the source code and compile for yourself.


Thanks!


I would like to try the new stack, but I am guessing I will need to
patch it for usb/95173 in order to use my umass device.


Usually it is best if patches go into FreeBSD current first,  
because else I
will have to keep track of the patches I take in. Regarding your  
patch, it
looks OK, except it would be nice if the patch could be modified so  
that it
automatically detects these quirks. Because adding quirks is a bad  
solution

in the long run.


I agree that quirks are not the best solution.  Unfortunately,  
though, I don't think it will be possible to detect the problem and  
dynamically disable sync'ing.  At least this particular USB-to-disk  
controller seems to lockup when the sync command is issued.  It  
requires a power cycle to recover the device.


It did occur to me, however, that perhaps it would make sense to  
disable sync'ing for all umass devices by default and then add a  
quirk to enable the feature on devices where its known to be  
beneficial.  This is based on the (perhaps dubious) theory that most  
umass devices probably do not do a lot of caching because of the risk  
of being disconnected at any time.  I didn't pursue this option,  
though, since I thought it would be more controversial and harder to  
get applied to the tree.



Maybe you have another USB mass storage device you can test ?


I think I can probably find one.  Eventually I would like to test my  
hard drive enclosure, though, since I use it the most often  
currently.  I probably won't be able to run the tests until after the  
new year.


Thanks again.

- Ben
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Re: usb/95173: [umass] [patch] cannot mount external usb harddisk VIA Technologies Inc. USB 2.0 IDE Bridge

2008-01-25 Thread Ben Kelly 
The following reply was made to PR usb/95173; it has been noted by GNATS.

From: Ben Kelly <[EMAIL PROTECTED]>
To: [EMAIL PROTECTED], [EMAIL PROTECTED]
Cc:  
Subject: Re: usb/95173: [umass] [patch] cannot mount external usb harddisk VIA 
Technologies Inc. USB 2.0 IDE Bridge
Date: Sat, 26 Jan 2008 01:35:41 -0500

 --Apple-Mail-95--316157853
 Content-Type: text/plain;
charset=US-ASCII;
format=flowed;
delsp=yes
 Content-Transfer-Encoding: 7bit
 
 My last patch did not seem to get incorporated into the bug system  
 properly.  Here is a second attempt.
 
 
 --Apple-Mail-95--316157853
 Content-Disposition: attachment;
filename=via_usb_ata_patch.txt
 Content-Type: text/plain;
x-unix-mode=0700;
name="via_usb_ata_patch.txt"
 Content-Transfer-Encoding: 7bit
 
 Index: src/sys/dev/usb/usbdevs
 ===
 --- src/sys/dev/usb/usbdevs(revision 6)
 +++ src/sys/dev/usb/usbdevs(revision 9)
 @@ -2229,6 +2229,9 @@
  /* U.S. Robotics products */
  product USR USR5423   0x0121  USR5423 WLAN
  
 +/* VIA Technologies products */
 +product VIA USB2IDEBRIDGE 0x6204  USB 2.0 IDE Bridge
 +
  /* VidzMedia products */
  product VIDZMEDIA MONSTERTV   0x4fb1  MonsterTV P2H
  
 Index: src/sys/dev/usb/umass.c
 ===
 --- src/sys/dev/usb/umass.c(revision 6)
 +++ src/sys/dev/usb/umass.c(revision 9)
 @@ -323,6 +323,12 @@
 * sector number.
 */
  # define READ_CAPACITY_OFFBY1 0x2000
 +  /* Device cannot handle a SCSI synchronize cache command.  Normally
 +   * this quirk would be handled in the cam layer, but for IDE bridges
 +   * we need to associate the quirk with the bridge and not the
 +   * underlying disk device.  This is handled by faking a success result.
 +   */
 +# define NO_SYNCHRONIZE_CACHE 0x4000
  };
  
  static struct umass_devdescr_t umass_devdescrs[] = {
 @@ -804,6 +810,10 @@
  UMASS_PROTO_SCSI | UMASS_PROTO_BBB,
  NO_QUIRKS
},
 +  { USB_VENDOR_VIA, USB_PRODUCT_VIA_USB2IDEBRIDGE, RID_WILDCARD,
 +UMASS_PROTO_SCSI | UMASS_PROTO_BBB,
 +NO_SYNCHRONIZE_CACHE
 +  },
{ USB_VENDOR_VIVITAR, USB_PRODUCT_VIVITAR_35XX, RID_WILDCARD,
  UMASS_PROTO_SCSI | UMASS_PROTO_BBB,
  NO_INQUIRY
 @@ -2878,6 +2888,15 @@
xpt_done(ccb);
return;
}
 +  if ((sc->quirks & NO_SYNCHRONIZE_CACHE) &&
 +  rcmd[0] == SYNCHRONIZE_CACHE) {
 +  struct ccb_scsiio *csio = &ccb->csio;
 +
 +  csio->scsi_status = SCSI_STATUS_OK;
 +  ccb->ccb_h.status = CAM_REQ_CMP;
 +  xpt_done(ccb);
 +  return;
 +  }
if ((sc->quirks & FORCE_SHORT_INQUIRY) &&
rcmd[0] == INQUIRY) {
csio->dxfer_len = SHORT_INQUIRY_LENGTH;
 
 --Apple-Mail-95--316157853
 Content-Type: text/plain;
charset=US-ASCII;
format=flowed
 Content-Transfer-Encoding: 7bit
 
 
 
 --Apple-Mail-95--316157853--
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Re: usb/95173: [umass] [patch] cannot mount external usb harddisk VIA Technologies Inc. USB 2.0 IDE Bridge

2008-01-26 Thread Ben Kelly 
The following reply was made to PR usb/95173; it has been noted by GNATS.

From: Ben Kelly <[EMAIL PROTECTED]>
To: [EMAIL PROTECTED], [EMAIL PROTECTED]
Cc:  
Subject: Re: usb/95173: [umass] [patch] cannot mount external usb harddisk VIA 
Technologies Inc. USB 2.0 IDE Bridge
Date: Sat, 26 Jan 2008 14:14:16 -0500

 For the PR I thought I would also just throw in another thought I  
 had.  It occurred to me that since umass devices are usually designed  
 for a "can be pulled at any time" environment, it seems likely that  
 most of them disable write caching anyways.
 
 With that in mind, would it make more sense to disable the SYNC  
 command by default and add a quirk for those devices where its known  
 to be functioning and useful?
 
 This might increase the default compatibility of the FreeBSD USB stack  
 with devices in the future.  I count around 34 entries in scsi_da.c  
 for umass devices simply to add the NO_SYNC quirk.
 
 Just a thought.  If people are interested I can develop a patch for  
 this.
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"