Re: [PATCH 4/4] usb: storage: Implement 64-bit LBA support
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 --- 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 ?
Re: [PATCH 4/4] usb: storage: Implement 64-bit LBA support
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 >> --- >> 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 == 0x1) { >> +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 0x 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
Re: [PATCH 4/4] usb: storage: Implement 64-bit LBA support
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 --- 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 == 0x1) { + 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 0x 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