Re: [PATCH 4/4] usb: storage: Implement 64-bit LBA support

2023-10-29 Thread Marek Vasut

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

2023-10-29 Thread Hector Martin
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

2023-10-29 Thread Marek Vasut

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