On Sun, 7 May 2017, Andreas Hartmann wrote:
> Am 07.05.2017 um 04:11 schrieb Alan Stern:
> > On Sat, 6 May 2017, Andreas Hartmann wrote:
> >
> >> Am 06.05.2017 um 19:27 schrieb Andreas Hartmann:
> >>> On 05/06/2017 at 06:43 PM Alan Stern wrote:
> >>>> For future tests, usbmon traces won't be useful. The patches don't
> >>>> make any difference to the USB traffic (other than fixing that original
> >>>> issue involving non-DMA-able memory). Instead they change the
> >>>> communication between the driver and the SCSI core. USB_STORAGE_DEBUG
> >>>> will be most useful for tracking this.
> >>>
> >>> Tested now w/ patched 4.7.x and 4.8.x - they behave exactly the same as
> >>> 4.9.x. I don't think other versions will behave differently.
> >>> Tried to patch 4.4.x - but the patch doesn't apply.
> >>>
> >>> Most probably you need a USB_STORAGE_DEBUG with the unpatched driver -
> >>> true? 4.8.x would be ok?
> >>
> >> I attached an original 4.8.x DEBUG output. Hope this helps!
> >
> > Good. And just for a clean comparison, DEBUG output for 4.8.x with the
> > patched driver would be very helpful.
> >
> > I suspect the change in behavior is related to the fix I made for the
> > INQUIRY command. The original version of the driver returned invalid
> > information, so the kernel did not realize the media was removable.
>
> media = sd card?
Yes.
> But it's true, that the media is removable. After unmount (of all
> mounted partitions) the device / media should be deactivated as it is
> done with echo 1 > /sys/block/sdb/device/delete to stop any activity on
> this device (this makes the medium to disappear in the device manager).
It is indeed true that the media is removable. However, when it is
removed the card reader disconnects itself from the USB bus -- so it's
never possible for the computer to see the device without any media
present. In addition, the card reader doesn't have any way to prevent
the media from being removed (as compared to a CD drive where the tray
can be locked). And the driver doesn't understand the SCSI commands
that are connected with media removal.
Therefore there's no disadvantage in saying that the media is not
removable. That's what the patch below does.
> > My guess is that your GUI automatically rescans when you click on
> > Remove for a device on removable media.
> >
> > If that's the cause, we can fix it by changing the INQUIRY data to
> > indicate the media is non-removable. We'll see...
>
> Attached the 4.8.x debug with patched kernel. The orig-file is the debug
> output you already know.
>
> The interesting part (remove) starts at
>
> dmesg-4.8.ene.patched.gz dmesg-4.8.ene.orig.gz
> ------------------------------------------------------
> 166.450393 103.610444
>
> MEDIUM_REMOVAL MEDIUM_REMOVAL
> 167.468480 -
The two logs show numerous differences, almost all of which are due to
missing INQUIRY data for the unpatched driver. It's a reasonable
assumption that this is also the reason for the rescan. Testing with
the patch below will give us the answer.
Alan Stern
Index: usb-4.x/drivers/usb/storage/ene_ub6250.c
===================================================================
--- usb-4.x.orig/drivers/usb/storage/ene_ub6250.c
+++ usb-4.x/drivers/usb/storage/ene_ub6250.c
@@ -600,6 +600,18 @@ static int do_scsi_request_sense(struct
return USB_STOR_TRANSPORT_GOOD;
}
+static int do_scsi_inquiry(struct us_data *us, struct scsi_cmnd *srb)
+{
+ unsigned char data_ptr[36] = {
+ 0x00, 0x00, 0x02, 0x00, 0x1F, 0x00, 0x00, 0x00, 0x55,
+ 0x53, 0x42, 0x32, 0x2E, 0x30, 0x20, 0x20, 0x43, 0x61,
+ 0x72, 0x64, 0x52, 0x65, 0x61, 0x64, 0x65, 0x72, 0x20,
+ 0x20, 0x20, 0x20, 0x20, 0x20, 0x30, 0x31, 0x30, 0x30 };
+
+ usb_stor_set_xfer_buf(data_ptr, 36, srb);
+ return USB_STOR_TRANSPORT_GOOD;
+}
+
static int sd_scsi_test_unit_ready(struct us_data *us, struct scsi_cmnd *srb)
{
struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;
@@ -614,18 +626,6 @@ static int sd_scsi_test_unit_ready(struc
return USB_STOR_TRANSPORT_GOOD;
}
-static int sd_scsi_inquiry(struct us_data *us, struct scsi_cmnd *srb)
-{
- unsigned char data_ptr[36] = {
- 0x00, 0x80, 0x02, 0x00, 0x1F, 0x00, 0x00, 0x00, 0x55,
- 0x53, 0x42, 0x32, 0x2E, 0x30, 0x20, 0x20, 0x43, 0x61,
- 0x72, 0x64, 0x52, 0x65, 0x61, 0x64, 0x65, 0x72, 0x20,
- 0x20, 0x20, 0x20, 0x20, 0x20, 0x30, 0x31, 0x30, 0x30 };
-
- usb_stor_set_xfer_buf(data_ptr, 36, srb);
- return USB_STOR_TRANSPORT_GOOD;
-}
-
static int sd_scsi_mode_sense(struct us_data *us, struct scsi_cmnd *srb)
{
struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;
@@ -1473,19 +1473,6 @@ static int ms_scsi_test_unit_ready(struc
return USB_STOR_TRANSPORT_GOOD;
}
-static int ms_scsi_inquiry(struct us_data *us, struct scsi_cmnd *srb)
-{
- /* pr_info("MS_SCSI_Inquiry\n"); */
- unsigned char data_ptr[36] = {
- 0x00, 0x80, 0x02, 0x00, 0x1F, 0x00, 0x00, 0x00, 0x55,
- 0x53, 0x42, 0x32, 0x2E, 0x30, 0x20, 0x20, 0x43, 0x61,
- 0x72, 0x64, 0x52, 0x65, 0x61, 0x64, 0x65, 0x72, 0x20,
- 0x20, 0x20, 0x20, 0x20, 0x20, 0x30, 0x31, 0x30, 0x30};
-
- usb_stor_set_xfer_buf(data_ptr, 36, srb);
- return USB_STOR_TRANSPORT_GOOD;
-}
-
static int ms_scsi_mode_sense(struct us_data *us, struct scsi_cmnd *srb)
{
struct ene_ub6250_info *info = (struct ene_ub6250_info *) us->extra;
@@ -2251,7 +2238,7 @@ static int sd_scsi_irp(struct us_data *u
result = do_scsi_request_sense(us, srb);
break; /* 0x03 */
case INQUIRY:
- result = sd_scsi_inquiry(us, srb);
+ result = do_scsi_inquiry(us, srb);
break; /* 0x12 */
case MODE_SENSE:
result = sd_scsi_mode_sense(us, srb);
@@ -2296,7 +2283,7 @@ static int ms_scsi_irp(struct us_data *u
result = do_scsi_request_sense(us, srb);
break; /* 0x03 */
case INQUIRY:
- result = ms_scsi_inquiry(us, srb);
+ result = do_scsi_inquiry(us, srb);
break; /* 0x12 */
case MODE_SENSE:
result = ms_scsi_mode_sense(us, srb);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
