From 93c9bf4d1838d5851a18ca398b0ad66397f05056 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 31 Oct 2014 14:49:47 -0400 Subject: usb-storage: handle a skipped data phase Sometimes mass-storage devices using the Bulk-only transport will mistakenly skip the data phase of a command. Rather than sending the data expected by the host or sending a zero-length packet, they go directly to the status phase and send the CSW. This causes problems for usb-storage, for obvious reasons. The driver will interpret the CSW as a short data transfer and will wait to receive a CSW. The device won't have anything left to send, so the command eventually times out. The SCSI layer doesn't retry commands after they time out (this is a relatively recent change). Therefore we should do our best to detect a skipped data phase and handle it promptly. This patch adds code to do that. If usb-storage receives a short 13-byte data transfer from the device, and if the first four bytes of the data match the CSW signature, the driver will set the residue to the full transfer length and interpret the data as a CSW. This fixes Bugzilla #86611. Signed-off-by: Alan Stern CC: Matthew Dharm Tested-by: Paul Osmialowski CC: Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/transport.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c index 22c7d4360fa..b1d815eb6d0 100644 --- a/drivers/usb/storage/transport.c +++ b/drivers/usb/storage/transport.c @@ -1118,6 +1118,31 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us) */ if (result == USB_STOR_XFER_LONG) fake_sense = 1; + + /* + * Sometimes a device will mistakenly skip the data phase + * and go directly to the status phase without sending a + * zero-length packet. If we get a 13-byte response here, + * check whether it really is a CSW. + */ + if (result == USB_STOR_XFER_SHORT && + srb->sc_data_direction == DMA_FROM_DEVICE && + transfer_length - scsi_get_resid(srb) == + US_BULK_CS_WRAP_LEN) { + struct scatterlist *sg = NULL; + unsigned int offset = 0; + + if (usb_stor_access_xfer_buf((unsigned char *) bcs, + US_BULK_CS_WRAP_LEN, srb, &sg, + &offset, FROM_XFER_BUF) == + US_BULK_CS_WRAP_LEN && + bcs->Signature == + cpu_to_le32(US_BULK_CS_SIGN)) { + usb_stor_dbg(us, "Device skipped data phase\n"); + scsi_set_resid(srb, transfer_length); + goto skipped_data_phase; + } + } } /* See flow chart on pg 15 of the Bulk Only Transport spec for @@ -1153,6 +1178,7 @@ int usb_stor_Bulk_transport(struct scsi_cmnd *srb, struct us_data *us) if (result != USB_STOR_XFER_GOOD) return USB_STOR_TRANSPORT_ERROR; + skipped_data_phase: /* check bulk status */ residue = le32_to_cpu(bcs->Residue); usb_stor_dbg(us, "Bulk Status S 0x%x T 0x%x R %u Stat 0x%x\n", -- cgit v1.2.3-70-g09d2 From aee0ce3ae73c566ace9958302e001d3cbbb0a623 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 31 Oct 2014 14:37:32 +0100 Subject: uas: Add US_FL_NO_ATA_1X quirk for 1 more Seagate model These drives hang when receiving ATA12 commands, so set the US_FL_NO_ATA_1X quirk to filter these out. Cc: stable@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/unusual_uas.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 8511b54a65d..d57e138567a 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -61,6 +61,13 @@ UNUSUAL_DEV(0x0bc2, 0xab20, 0x0000, 0x9999, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), +/* https://bbs.archlinux.org/viewtopic.php?id=183190 */ +UNUSUAL_DEV(0x0bc2, 0xab21, 0x0000, 0x9999, + "Seagate", + "Backup+ BK", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + /* Reported-by: Claudio Bizzarri */ UNUSUAL_DEV(0x152d, 0x0567, 0x0000, 0x9999, "JMicron", -- cgit v1.2.3-70-g09d2 From 673029fe9c16c95600bdaca4760673527af32edf Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 9 Oct 2014 17:27:56 +0200 Subject: uas: Add NO_ATA_1X for VIA VL711 devices Just like some Seagate enclosures, these devices do not seem to grok ata pass through commands. Cc: stable@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/unusual_uas.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index d57e138567a..ea793c1f304 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -82,3 +82,10 @@ UNUSUAL_DEV(0x174c, 0x5106, 0x0000, 0x9999, "ASM1051", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_IGNORE_UAS), + +/* Reported-by: Hans de Goede */ +UNUSUAL_DEV(0x2109, 0x0711, 0x0000, 0x9999, + "VIA", + "VL711", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), -- cgit v1.2.3-70-g09d2 From ec5633ba677761b44ba94ae29c906ba79dd6eaa0 Mon Sep 17 00:00:00 2001 From: Luis Henriques Date: Thu, 2 Oct 2014 00:10:57 +0100 Subject: usb: storage: fix build warnings !CONFIG_PM Functions fw5895_init() and config_autodelink_before_power_down() are used only when CONFIG_PM is defined. drivers/usb/storage/realtek_cr.c:699:13: warning: 'fw5895_init' defined but not used [-Wunused-function] drivers/usb/storage/realtek_cr.c:629:12: warning: 'config_autodelink_before_power_down' defined but not used [-Wunused-function] Signed-off-by: Luis Henriques Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/realtek_cr.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/realtek_cr.c b/drivers/usb/storage/realtek_cr.c index 8591d89a38e..27e4a580d2e 100644 --- a/drivers/usb/storage/realtek_cr.c +++ b/drivers/usb/storage/realtek_cr.c @@ -626,6 +626,7 @@ static int config_autodelink_after_power_on(struct us_data *us) return 0; } +#ifdef CONFIG_PM static int config_autodelink_before_power_down(struct us_data *us) { struct rts51x_chip *chip = (struct rts51x_chip *)(us->extra); @@ -716,6 +717,7 @@ static void fw5895_init(struct us_data *us) } } } +#endif #ifdef CONFIG_REALTEK_AUTOPM static void fw5895_set_mmc_wp(struct us_data *us) -- cgit v1.2.3-70-g09d2 From d1d9548256fbdf2e049d6413a5266c41e73658ee Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 23 Oct 2014 14:40:57 +0200 Subject: uas: Add US_FL_NO_ATA_1X quirk for 2 more Seagate models These drives hang when receiving ATA12 commands, so set the US_FL_NO_ATA_1X quirk to filter these out. Cc: stable@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/unusual_uas.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index ea793c1f304..2fefaf923e4 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -54,6 +54,20 @@ UNUSUAL_DEV(0x0bc2, 0x3312, 0x0000, 0x9999, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), +/* Reported-by: Hans de Goede */ +UNUSUAL_DEV(0x0bc2, 0x3320, 0x0000, 0x9999, + "Seagate", + "Expansion Desk", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + +/* Reported-by: Bogdan Mihalcea */ +UNUSUAL_DEV(0x0bc2, 0xa003, 0x0000, 0x9999, + "Seagate", + "Backup Plus", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + /* https://bbs.archlinux.org/viewtopic.php?id=183190 */ UNUSUAL_DEV(0x0bc2, 0xab20, 0x0000, 0x9999, "Seagate", -- cgit v1.2.3-70-g09d2 From ac0225f94f2af14d5db5a3ca2e9b151bb5488a52 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 5 Nov 2014 11:12:17 -0800 Subject: Revert "storage: Replace magic number with define in usb_stor_euscsi_init()" This reverts commit bda9893c50fb56253d3c206c14e3f933e5f68b3c as it was incorrect. Reported-by: Mark Knibbs Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/initializers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/initializers.c b/drivers/usb/storage/initializers.c index 4bc2fc98636..5a8b5ff1e45 100644 --- a/drivers/usb/storage/initializers.c +++ b/drivers/usb/storage/initializers.c @@ -52,7 +52,7 @@ int usb_stor_euscsi_init(struct us_data *us) us->iobuf[0] = 0x1; result = usb_stor_control_msg(us, us->send_ctrl_pipe, 0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR, - 0x01, 0x0, us->iobuf, 0x1, USB_CTRL_SET_TIMEOUT); + 0x01, 0x0, us->iobuf, 0x1, 5000); usb_stor_dbg(us, "-- result is %d\n", result); return 0; -- cgit v1.2.3-70-g09d2 From a88098bdb272fb631a59fb152bfef7c827531294 Mon Sep 17 00:00:00 2001 From: Mark Knibbs Date: Tue, 4 Nov 2014 13:00:24 +0000 Subject: USB: storage: Fix timeout in usb_stor_euscsi_init() and usb_stor_huawei_e220_init() The timeout argument to usb_stor_control_msg() is specified in jiffies, not milliseconds. Signed-off-by: Mark Knibbs Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/initializers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/initializers.c b/drivers/usb/storage/initializers.c index 5a8b5ff1e45..73f125e0cb5 100644 --- a/drivers/usb/storage/initializers.c +++ b/drivers/usb/storage/initializers.c @@ -52,7 +52,7 @@ int usb_stor_euscsi_init(struct us_data *us) us->iobuf[0] = 0x1; result = usb_stor_control_msg(us, us->send_ctrl_pipe, 0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR, - 0x01, 0x0, us->iobuf, 0x1, 5000); + 0x01, 0x0, us->iobuf, 0x1, 5 * HZ); usb_stor_dbg(us, "-- result is %d\n", result); return 0; @@ -100,7 +100,7 @@ int usb_stor_huawei_e220_init(struct us_data *us) result = usb_stor_control_msg(us, us->send_ctrl_pipe, USB_REQ_SET_FEATURE, USB_TYPE_STANDARD | USB_RECIP_DEVICE, - 0x01, 0x0, NULL, 0x0, 1000); + 0x01, 0x0, NULL, 0x0, 1 * HZ); usb_stor_dbg(us, "Huawei mode set result is %d\n", result); return 0; } -- cgit v1.2.3-70-g09d2