[PATCH 0/4] USB fixes: Mass Storage bugs & 64bit support
This series fixes some bugs in the USBMS driver and adds 64-bit LBA support. This is required to make USB HDDs >=4TB work.
Note that the increased timeout won't actually work right now, due to broken handling in the underlying USB infrastructure. That will be fixed in a follow-up series, which depends on [1] being applied first. The USBMS part is logically stand-alone and can be applied in parallel before that.
[1] https://lore.kernel.org/u-boot/20231029-usb-fixes-1-v2-0-623533f6316e@marcan...
Signed-off-by: Hector Martin marcan@marcan.st --- Hector Martin (4): scsi: Fix a bunch of SCSI definitions. usb: storage: Increase read/write timeout usb: storage: Use the correct CBW lengths usb: storage: Implement 64-bit LBA support
common/usb_storage.c | 164 ++++++++++++++++++++++++++++++++++++++++++--------- drivers/ata/ahci.c | 9 ++- drivers/scsi/scsi.c | 4 +- include/scsi.h | 8 ++- 4 files changed, 150 insertions(+), 35 deletions(-) --- base-commit: 8ad1c9c26f7740806a162818b790d4a72f515b7e change-id: 20231029-usb-fixes-3-c72f829ba61b
Best regards,
0x9e isn't Read Capacity, it's a service action and the read capacity command is a subcommand.
READ16 is not 0x48, it's 0x88. 0x48 is SANITIZE and that sounds like we might have been destroying data instead of reading data. No bueno.
Signed-off-by: Hector Martin marcan@marcan.st --- drivers/ata/ahci.c | 9 ++++++--- drivers/scsi/scsi.c | 4 ++-- include/scsi.h | 8 ++++++-- 3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 2062197afcd3..b252e9e525db 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -906,15 +906,18 @@ static int ahci_scsi_exec(struct udevice *dev, struct scsi_cmd *pccb) case SCSI_RD_CAPAC10: ret = ata_scsiop_read_capacity10(uc_priv, pccb); break; - case SCSI_RD_CAPAC16: - ret = ata_scsiop_read_capacity16(uc_priv, pccb); - break; case SCSI_TST_U_RDY: ret = ata_scsiop_test_unit_ready(uc_priv, pccb); break; case SCSI_INQUIRY: ret = ata_scsiop_inquiry(uc_priv, pccb); break; + case SCSI_SRV_ACTION_IN: + if ((pccb->cmd[1] & 0x1f) == SCSI_SAI_RD_CAPAC16) { + ret = ata_scsiop_read_capacity16(uc_priv, pccb); + break; + } + /* Fallthrough */ default: printf("Unsupport SCSI command 0x%02x\n", pccb->cmd[0]); return -ENOTSUPP; diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index d7b33010e469..f2c828eb305e 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -380,8 +380,8 @@ static int scsi_read_capacity(struct udevice *dev, struct scsi_cmd *pccb,
/* Read capacity (10) was insufficient. Use read capacity (16). */ memset(pccb->cmd, '\0', sizeof(pccb->cmd)); - pccb->cmd[0] = SCSI_RD_CAPAC16; - pccb->cmd[1] = 0x10; + pccb->cmd[0] = SCSI_SRV_ACTION_IN; + pccb->cmd[1] = SCSI_SAI_RD_CAPAC16; pccb->cmdlen = 16; pccb->msgout[0] = SCSI_IDENTIFY; /* NOT USED */
diff --git a/include/scsi.h b/include/scsi.h index b47c7463c1d6..89e268586477 100644 --- a/include/scsi.h +++ b/include/scsi.h @@ -141,10 +141,9 @@ struct scsi_cmd { #define SCSI_MED_REMOVL 0x1E /* Prevent/Allow medium Removal (O) */ #define SCSI_READ6 0x08 /* Read 6-byte (MANDATORY) */ #define SCSI_READ10 0x28 /* Read 10-byte (MANDATORY) */ -#define SCSI_READ16 0x48 +#define SCSI_READ16 0x88 /* Read 16-byte */ #define SCSI_RD_CAPAC 0x25 /* Read Capacity (MANDATORY) */ #define SCSI_RD_CAPAC10 SCSI_RD_CAPAC /* Read Capacity (10) */ -#define SCSI_RD_CAPAC16 0x9e /* Read Capacity (16) */ #define SCSI_RD_DEFECT 0x37 /* Read Defect Data (O) */ #define SCSI_READ_LONG 0x3E /* Read Long (O) */ #define SCSI_REASS_BLK 0x07 /* Reassign Blocks (O) */ @@ -158,15 +157,20 @@ struct scsi_cmd { #define SCSI_SEEK10 0x2B /* Seek 10-Byte (O) */ #define SCSI_SEND_DIAG 0x1D /* Send Diagnostics (MANDATORY) */ #define SCSI_SET_LIMIT 0x33 /* Set Limits (O) */ +#define SCSI_SRV_ACTION_IN 0x9E /* Service Action In */ +#define SCSI_SRV_ACTION_OUT 0x9F /* Service Action Out */ #define SCSI_START_STP 0x1B /* Start/Stop Unit (O) */ #define SCSI_SYNC_CACHE 0x35 /* Synchronize Cache (O) */ #define SCSI_VERIFY 0x2F /* Verify (O) */ #define SCSI_WRITE6 0x0A /* Write 6-Byte (MANDATORY) */ #define SCSI_WRITE10 0x2A /* Write 10-Byte (MANDATORY) */ +#define SCSI_WRITE16 0x8A /* Write 16-byte */ #define SCSI_WRT_VERIFY 0x2E /* Write and Verify (O) */ #define SCSI_WRITE_LONG 0x3F /* Write Long (O) */ #define SCSI_WRITE_SAME 0x41 /* Write Same (O) */
+#define SCSI_SAI_RD_CAPAC16 0x10 /* Service Action: Read Capacity (16) */ + /** * struct scsi_plat - stores information about SCSI controller *
On 10/29/23 08:23, Hector Martin wrote:
0x9e isn't Read Capacity, it's a service action and the read capacity command is a subcommand.
READ16 is not 0x48, it's 0x88. 0x48 is SANITIZE and that sounds like we might have been destroying data instead of reading data. No bueno.
Signed-off-by: Hector Martin marcan@marcan.st
drivers/ata/ahci.c | 9 ++++++--- drivers/scsi/scsi.c | 4 ++-- include/scsi.h | 8 ++++++-- 3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 2062197afcd3..b252e9e525db 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -906,15 +906,18 @@ static int ahci_scsi_exec(struct udevice *dev, struct scsi_cmd *pccb) case SCSI_RD_CAPAC10: ret = ata_scsiop_read_capacity10(uc_priv, pccb); break;
- case SCSI_RD_CAPAC16:
ret = ata_scsiop_read_capacity16(uc_priv, pccb);break;
- case SCSI_SRV_ACTION_IN:
if ((pccb->cmd[1] & 0x1f) == SCSI_SAI_RD_CAPAC16) {ret = ata_scsiop_read_capacity16(uc_priv, pccb);break;}
Would it make sense to add an else branch here and report unknown subcommand there ?
With that: Reviewed-by: Marek Vasut marex@denx.de
Some USB devices (like hard disks) can take a long time to initially respond to read/write requests. Explicitly specify a much longer timeout than normal.
Signed-off-by: Hector Martin marcan@marcan.st --- common/usb_storage.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index c9e2d7343ce2..729ddbc75a48 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -53,6 +53,12 @@ #undef BBB_COMDAT_TRACE #undef BBB_XPORT_TRACE
+/* + * Timeout for read/write transfers. This needs to be able to handle very slow + * devices, such as hard disks that are spinning up. + */ +#define US_XFER_TIMEOUT 15000 + #include <scsi.h> /* direction table -- this indicates the direction of the data * transfer for each command code -- a 1 indicates input @@ -394,7 +400,7 @@ static int us_one_transfer(struct us_data *us, int pipe, char *buf, int length) 11 - maxtry); result = usb_bulk_msg(us->pusb_dev, pipe, buf, this_xfer, &partial, - USB_CNTL_TIMEOUT * 5); + US_XFER_TIMEOUT); debug("bulk_msg returned %d xferred %d/%d\n", result, partial, this_xfer); if (us->pusb_dev->status != 0) { @@ -743,7 +749,7 @@ static int usb_stor_BBB_transport(struct scsi_cmd *srb, struct us_data *us) pipe = pipeout;
result = usb_bulk_msg(us->pusb_dev, pipe, srb->pdata, srb->datalen, - &data_actlen, USB_CNTL_TIMEOUT * 5); + &data_actlen, US_XFER_TIMEOUT); /* special handling of STALL in DATA phase */ if ((result < 0) && (us->pusb_dev->status & USB_ST_STALLED)) { debug("DATA:stall\n");
On 10/29/23 08:23, Hector Martin wrote:
Some USB devices (like hard disks) can take a long time to initially respond to read/write requests. Explicitly specify a much longer timeout than normal.
Signed-off-by: Hector Martin marcan@marcan.st
Can we make this configurable with e.g. env variable, similar to usb_pgood_delay , to avoid affecting existing users ?
USB UFI uses fixed 12-byte commands (as does RBC, which is not supported), but SCSI does not have this limitation. Use the correct command block lengths depending on the subclass.
Signed-off-by: Hector Martin marcan@marcan.st --- common/usb_storage.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index 729ddbc75a48..95507ffbce48 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -107,6 +107,7 @@ struct us_data { trans_reset transport_reset; /* reset routine */ trans_cmnd transport; /* transport routine */ unsigned short max_xfer_blk; /* maximum transfer blocks */ + bool cmd12; /* use 12-byte commands (RBC/UFI) */ };
#if !CONFIG_IS_ENABLED(BLK) @@ -349,7 +350,7 @@ static void usb_show_srb(struct scsi_cmd *pccb) { int i; printf("SRB: len %d datalen 0x%lX\n ", pccb->cmdlen, pccb->datalen); - for (i = 0; i < 12; i++) + for (i = 0; i < pccb->cmdlen; i++) printf("%02X ", pccb->cmd[i]); printf("\n"); } @@ -888,7 +889,7 @@ do_retry: psrb->cmd[4] = 18; psrb->datalen = 18; psrb->pdata = &srb->sense_buf[0]; - psrb->cmdlen = 12; + psrb->cmdlen = us->cmd12 ? 12 : 6; /* issue the command */ result = usb_stor_CB_comdat(psrb, us); debug("auto request returned %d\n", result); @@ -989,7 +990,7 @@ static int usb_inquiry(struct scsi_cmd *srb, struct us_data *ss) srb->cmd[1] = srb->lun << 5; srb->cmd[4] = 36; srb->datalen = 36; - srb->cmdlen = 12; + srb->cmdlen = ss->cmd12 ? 12 : 6; i = ss->transport(srb, ss); debug("inquiry returns %d\n", i); if (i == 0) @@ -1014,7 +1015,7 @@ static int usb_request_sense(struct scsi_cmd *srb, struct us_data *ss) srb->cmd[4] = 18; srb->datalen = 18; srb->pdata = &srb->sense_buf[0]; - srb->cmdlen = 12; + srb->cmdlen = ss->cmd12 ? 12 : 6; ss->transport(srb, ss); debug("Request Sense returned %02X %02X %02X\n", srb->sense_buf[2], srb->sense_buf[12], @@ -1032,7 +1033,7 @@ static int usb_test_unit_ready(struct scsi_cmd *srb, struct us_data *ss) srb->cmd[0] = SCSI_TST_U_RDY; srb->cmd[1] = srb->lun << 5; srb->datalen = 0; - srb->cmdlen = 12; + srb->cmdlen = ss->cmd12 ? 12 : 6; if (ss->transport(srb, ss) == USB_STOR_TRANSPORT_GOOD) { ss->flags |= USB_READY; return 0; @@ -1064,7 +1065,7 @@ static int usb_read_capacity(struct scsi_cmd *srb, struct us_data *ss) srb->cmd[0] = SCSI_RD_CAPAC; srb->cmd[1] = srb->lun << 5; srb->datalen = 8; - srb->cmdlen = 12; + srb->cmdlen = ss->cmd12 ? 12 : 10; if (ss->transport(srb, ss) == USB_STOR_TRANSPORT_GOOD) return 0; } while (retry--); @@ -1084,7 +1085,7 @@ static int usb_read_10(struct scsi_cmd *srb, struct us_data *ss, srb->cmd[5] = ((unsigned char) (start)) & 0xff; srb->cmd[7] = ((unsigned char) (blocks >> 8)) & 0xff; srb->cmd[8] = (unsigned char) blocks & 0xff; - srb->cmdlen = 12; + srb->cmdlen = ss->cmd12 ? 12 : 10; debug("read10: start %lx blocks %x\n", start, blocks); return ss->transport(srb, ss); } @@ -1101,7 +1102,7 @@ static int usb_write_10(struct scsi_cmd *srb, struct us_data *ss, srb->cmd[5] = ((unsigned char) (start)) & 0xff; srb->cmd[7] = ((unsigned char) (blocks >> 8)) & 0xff; srb->cmd[8] = (unsigned char) blocks & 0xff; - srb->cmdlen = 12; + srb->cmdlen = ss->cmd12 ? 12 : 10; debug("write10: start %lx blocks %x\n", start, blocks); return ss->transport(srb, ss); } @@ -1407,6 +1408,11 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, printf("Sorry, protocol %d not yet supported.\n", ss->subclass); return 0; } + + /* UFI uses 12-byte commands (like RBC, unlike SCSI) */ + if (ss->subclass == US_SC_UFI) + ss->cmd12 = true; + if (ss->ep_int) { /* we had found an interrupt endpoint, prepare irq pipe * set up the IRQ pipe and handler
On 10/29/23 08:23, Hector Martin wrote:
USB UFI uses fixed 12-byte commands (as does RBC, which is not supported), but SCSI does not have this limitation. Use the correct command block lengths depending on the subclass.
Signed-off-by: Hector Martin marcan@marcan.st
Reviewed-by: Marek Vasut marex@denx.de
This makes things work properly on devices with >= 2 TiB capacity. If u-boot is built without CONFIG_SYS_64BIT_LBA, the capacity will be clamped at 2^32 - 1 sectors.
Signed-off-by: Hector Martin marcan@marcan.st --- common/usb_storage.c | 132 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 114 insertions(+), 18 deletions(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index 95507ffbce48..3035f2ee9868 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -66,7 +66,7 @@ static const unsigned char us_direction[256/8] = { 0x28, 0x81, 0x14, 0x14, 0x20, 0x01, 0x90, 0x77, 0x0C, 0x20, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, + 0x00, 0x01, 0x00, 0x40, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1) @@ -1073,6 +1073,27 @@ static int usb_read_capacity(struct scsi_cmd *srb, struct us_data *ss) return -1; }
+#ifdef CONFIG_SYS_64BIT_LBA +static int usb_read_capacity64(struct scsi_cmd *srb, struct us_data *ss) +{ + int retry; + /* XXX retries */ + retry = 3; + do { + memset(&srb->cmd[0], 0, 16); + srb->cmd[0] = SCSI_SRV_ACTION_IN; + srb->cmd[1] = (srb->lun << 5) | SCSI_SAI_RD_CAPAC16; + srb->cmd[13] = 32; /* Allocation length */ + srb->datalen = 32; + srb->cmdlen = 16; + if (ss->transport(srb, ss) == USB_STOR_TRANSPORT_GOOD) + return 0; + } while (retry--); + + return -1; +} +#endif + static int usb_read_10(struct scsi_cmd *srb, struct us_data *ss, unsigned long start, unsigned short blocks) { @@ -1107,6 +1128,49 @@ static int usb_write_10(struct scsi_cmd *srb, struct us_data *ss, return ss->transport(srb, ss); }
+#ifdef CONFIG_SYS_64BIT_LBA +static int usb_read_16(struct scsi_cmd *srb, struct us_data *ss, + uint64_t start, unsigned short blocks) +{ + memset(&srb->cmd[0], 0, 16); + srb->cmd[0] = SCSI_READ16; + srb->cmd[1] = srb->lun << 5; + srb->cmd[2] = ((unsigned char) (start >> 56)) & 0xff; + srb->cmd[3] = ((unsigned char) (start >> 48)) & 0xff; + srb->cmd[4] = ((unsigned char) (start >> 40)) & 0xff; + srb->cmd[5] = ((unsigned char) (start >> 32)) & 0xff; + srb->cmd[6] = ((unsigned char) (start >> 24)) & 0xff; + srb->cmd[7] = ((unsigned char) (start >> 16)) & 0xff; + srb->cmd[8] = ((unsigned char) (start >> 8)) & 0xff; + srb->cmd[9] = ((unsigned char) (start)) & 0xff; + srb->cmd[12] = ((unsigned char) (blocks >> 8)) & 0xff; + srb->cmd[13] = (unsigned char) blocks & 0xff; + srb->cmdlen = 16; + debug("read16: start %llx blocks %x\n", (long long)start, blocks); + return ss->transport(srb, ss); +} + +static int usb_write_16(struct scsi_cmd *srb, struct us_data *ss, + uint64_t start, unsigned short blocks) +{ + memset(&srb->cmd[0], 0, 16); + srb->cmd[0] = SCSI_WRITE16; + srb->cmd[1] = srb->lun << 5; + srb->cmd[2] = ((unsigned char) (start >> 56)) & 0xff; + srb->cmd[3] = ((unsigned char) (start >> 48)) & 0xff; + srb->cmd[4] = ((unsigned char) (start >> 40)) & 0xff; + srb->cmd[5] = ((unsigned char) (start >> 32)) & 0xff; + srb->cmd[6] = ((unsigned char) (start >> 24)) & 0xff; + srb->cmd[7] = ((unsigned char) (start >> 16)) & 0xff; + srb->cmd[8] = ((unsigned char) (start >> 8)) & 0xff; + srb->cmd[9] = ((unsigned char) (start)) & 0xff; + srb->cmd[12] = ((unsigned char) (blocks >> 8)) & 0xff; + srb->cmd[13] = (unsigned char) blocks & 0xff; + srb->cmdlen = 16; + debug("write16: start %llx blocks %x\n", (long long)start, blocks); + return ss->transport(srb, ss); +} +#endif
#ifdef CONFIG_USB_BIN_FIXUP /* @@ -1145,6 +1209,7 @@ static unsigned long usb_stor_read(struct blk_desc *block_dev, lbaint_t blknr, struct usb_device *udev; struct us_data *ss; int retry; + int ret; struct scsi_cmd *srb = &usb_ccb; #if CONFIG_IS_ENABLED(BLK) struct blk_desc *block_dev; @@ -1190,7 +1255,13 @@ retry_it: usb_show_progress(); srb->datalen = block_dev->blksz * smallblks; srb->pdata = (unsigned char *)buf_addr; - if (usb_read_10(srb, ss, start, smallblks)) { +#ifdef CONFIG_SYS_64BIT_LBA + if (block_dev->lba > ((lbaint_t)0x100000000)) + ret = usb_read_16(srb, ss, start, smallblks); + else +#endif + ret = usb_read_10(srb, ss, start, smallblks); + if (ret) { debug("Read ERROR\n"); ss->flags &= ~USB_READY; usb_request_sense(srb, ss); @@ -1228,6 +1299,7 @@ static unsigned long usb_stor_write(struct blk_desc *block_dev, lbaint_t blknr, struct usb_device *udev; struct us_data *ss; int retry; + int ret; struct scsi_cmd *srb = &usb_ccb; #if CONFIG_IS_ENABLED(BLK) struct blk_desc *block_dev; @@ -1277,7 +1349,13 @@ retry_it: usb_show_progress(); srb->datalen = block_dev->blksz * smallblks; srb->pdata = (unsigned char *)buf_addr; - if (usb_write_10(srb, ss, start, smallblks)) { +#ifdef CONFIG_SYS_64BIT_LBA + if (block_dev->lba > ((lbaint_t)0x100000000)) + ret = usb_write_16(srb, ss, start, smallblks); + else +#endif + ret = usb_write_10(srb, ss, start, smallblks); + if (ret) { debug("Write ERROR\n"); ss->flags &= ~USB_READY; usb_request_sense(srb, ss); @@ -1434,9 +1512,10 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss, struct blk_desc *dev_desc) { unsigned char perq, modi; - ALLOC_CACHE_ALIGN_BUFFER(u32, cap, 2); + ALLOC_CACHE_ALIGN_BUFFER(u32, cap, 8); ALLOC_CACHE_ALIGN_BUFFER(u8, usb_stor_buf, 36); - u32 capacity, blksz; + lbaint_t capacity; + u32 blksz; struct scsi_cmd *pccb = &usb_ccb;
pccb->pdata = usb_stor_buf; @@ -1487,26 +1566,43 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss, return 0; } pccb->pdata = (unsigned char *)cap; - memset(pccb->pdata, 0, 8); + memset(pccb->pdata, 0, 32); if (usb_read_capacity(pccb, ss) != 0) { printf("READ_CAP ERROR\n"); ss->flags &= ~USB_READY; - cap[0] = 2880; - cap[1] = 0x200; + capacity = 2880; + blksz = 512; + } else { + debug("Read Capacity returns: 0x%08x, 0x%08x\n", + cap[0], cap[1]); + capacity = ((lbaint_t)be32_to_cpu(cap[0])) + 1; + blksz = be32_to_cpu(cap[1]); } - debug("Read Capacity returns: 0x%08x, 0x%08x\n", cap[0], cap[1]); -#if 0 - if (cap[0] > (0x200000 * 10)) /* greater than 10 GByte */ - cap[0] >>= 16;
- cap[0] = cpu_to_be32(cap[0]); - cap[1] = cpu_to_be32(cap[1]); +#ifdef CONFIG_SYS_64BIT_LBA + if (capacity == 0x100000000) { + if (usb_read_capacity64(pccb, ss) != 0) { + puts("READ_CAP64 ERROR\n"); + } else { + debug("Read Capacity 64 returns: 0x%08x, 0x%08x, 0x%08x\n", + cap[0], cap[1], cap[2]); + capacity = be64_to_cpu(*(uint64_t *)cap) + 1; + blksz = be32_to_cpu(cap[2]); + } + } +#else + /* + * READ CAPACITY will return 0xffffffff when limited, + * which wraps to 0 with the +1 above + */ + if (!capacity) { + puts("LBA exceeds 32 bits but 64-bit LBA is disabled.\n"); + capacity = ~0; + } #endif
- capacity = be32_to_cpu(cap[0]) + 1; - blksz = be32_to_cpu(cap[1]); - - debug("Capacity = 0x%08x, blocksz = 0x%08x\n", capacity, blksz); + debug("Capacity = 0x%llx, blocksz = 0x%08x\n", + (long long)capacity, blksz); dev_desc->lba = capacity; dev_desc->blksz = blksz; dev_desc->log2blksz = LOG2(dev_desc->blksz);
On 10/29/23 08:23, Hector Martin wrote:
This makes things work properly on devices with >= 2 TiB capacity. If u-boot is built without CONFIG_SYS_64BIT_LBA, the capacity will be clamped at 2^32 - 1 sectors.
Signed-off-by: Hector Martin marcan@marcan.st
common/usb_storage.c | 132 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 114 insertions(+), 18 deletions(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index 95507ffbce48..3035f2ee9868 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -66,7 +66,7 @@ static const unsigned char us_direction[256/8] = { 0x28, 0x81, 0x14, 0x14, 0x20, 0x01, 0x90, 0x77, 0x0C, 0x20, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01,
- 0x00, 0x01, 0x00, 0x40, 0x00, 0x01, 0x00, 0x01,
What changed here ?
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1) @@ -1073,6 +1073,27 @@ static int usb_read_capacity(struct scsi_cmd *srb, struct us_data *ss) return -1; }
[...]
+#ifdef CONFIG_SYS_64BIT_LBA
Could you try and use CONFIG_IS_ENABLED() ?
- if (capacity == 0x100000000) {
if (usb_read_capacity64(pccb, ss) != 0) {puts("READ_CAP64 ERROR\n");} else {debug("Read Capacity 64 returns: 0x%08x, 0x%08x, 0x%08x\n",cap[0], cap[1], cap[2]);capacity = be64_to_cpu(*(uint64_t *)cap) + 1;blksz = be32_to_cpu(cap[2]);}- }
+#else
- /*
* READ CAPACITY will return 0xffffffff when limited,* which wraps to 0 with the +1 above*/- if (!capacity) {
puts("LBA exceeds 32 bits but 64-bit LBA is disabled.\n");capacity = ~0;- } #endif
On 29/10/2023 21.11, Marek Vasut wrote:
On 10/29/23 08:23, Hector Martin wrote:
This makes things work properly on devices with >= 2 TiB capacity. If u-boot is built without CONFIG_SYS_64BIT_LBA, the capacity will be clamped at 2^32 - 1 sectors.
Signed-off-by: Hector Martin marcan@marcan.st
common/usb_storage.c | 132 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 114 insertions(+), 18 deletions(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index 95507ffbce48..3035f2ee9868 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -66,7 +66,7 @@ static const unsigned char us_direction[256/8] = { 0x28, 0x81, 0x14, 0x14, 0x20, 0x01, 0x90, 0x77, 0x0C, 0x20, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01,
- 0x00, 0x01, 0x00, 0x40, 0x00, 0x01, 0x00, 0x01,
What changed here ?
This is an incomplete bitmap specifying the data transfer direction for every possible SCSI command ID. I'm adding the new commands I'm now using.
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1) @@ -1073,6 +1073,27 @@ static int usb_read_capacity(struct scsi_cmd *srb, struct us_data *ss) return -1; }
[...]
+#ifdef CONFIG_SYS_64BIT_LBA
Could you try and use CONFIG_IS_ENABLED() ?
Sure.
- if (capacity == 0x100000000) {
if (usb_read_capacity64(pccb, ss) != 0) {puts("READ_CAP64 ERROR\n");} else {debug("Read Capacity 64 returns: 0x%08x, 0x%08x, 0x%08x\n",cap[0], cap[1], cap[2]);capacity = be64_to_cpu(*(uint64_t *)cap) + 1;blksz = be32_to_cpu(cap[2]);}- }
+#else
- /*
* READ CAPACITY will return 0xffffffff when limited,* which wraps to 0 with the +1 above*/- if (!capacity) {
puts("LBA exceeds 32 bits but 64-bit LBA is disabled.\n");capacity = ~0;- } #endif
- Hector
On 10/29/23 16:54, Hector Martin wrote:
On 29/10/2023 21.11, Marek Vasut wrote:
On 10/29/23 08:23, Hector Martin wrote:
This makes things work properly on devices with >= 2 TiB capacity. If u-boot is built without CONFIG_SYS_64BIT_LBA, the capacity will be clamped at 2^32 - 1 sectors.
Signed-off-by: Hector Martin marcan@marcan.st
common/usb_storage.c | 132 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 114 insertions(+), 18 deletions(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index 95507ffbce48..3035f2ee9868 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -66,7 +66,7 @@ static const unsigned char us_direction[256/8] = { 0x28, 0x81, 0x14, 0x14, 0x20, 0x01, 0x90, 0x77, 0x0C, 0x20, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01,
- 0x00, 0x01, 0x00, 0x40, 0x00, 0x01, 0x00, 0x01,
What changed here ?
This is an incomplete bitmap specifying the data transfer direction for every possible SCSI command ID. I'm adding the new commands I'm now using.
Can you please add a code comment here, so others wouldn't ponder about this too ?
On Sun, Oct 29, 2023 at 3:23 AM Hector Martin marcan@marcan.st wrote:
This series fixes some bugs in the USBMS driver and adds 64-bit LBA support. This is required to make USB HDDs >=4TB work.
Note that the increased timeout won't actually work right now, due to broken handling in the underlying USB infrastructure. That will be fixed in a follow-up series, which depends on [1] being applied first. The USBMS part is logically stand-alone and can be applied in parallel before that.
[1] https://lore.kernel.org/u-boot/20231029-usb-fixes-1-v2-0-623533f6316e@marcan...
Signed-off-by: Hector Martin marcan@marcan.st
Hector Martin (4): scsi: Fix a bunch of SCSI definitions. usb: storage: Increase read/write timeout usb: storage: Use the correct CBW lengths usb: storage: Implement 64-bit LBA support
common/usb_storage.c | 164 ++++++++++++++++++++++++++++++++++++++++++--------- drivers/ata/ahci.c | 9 ++- drivers/scsi/scsi.c | 4 +- include/scsi.h | 8 ++- 4 files changed, 150 insertions(+), 35 deletions(-)
base-commit: 8ad1c9c26f7740806a162818b790d4a72f515b7e change-id: 20231029-usb-fixes-3-c72f829ba61b
Series LGTM.
Reviewed-by: Neal Gompa neal@gompa.dev
participants (3)
-
Hector Martin -
Marek Vasut -
Neal Gompa