-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support lba/len being u32 #8
Comments
I suppose the idea is to use this lib on 32-bit targets? In this case I'm not sure, but rust should handle it even on targets that don't have hardware 64-bit integer support. (but I'm not 100% sure though ) Could you elaborate on your use-case a bit more? |
Yes, rust can handle all types on all hardware (even u128 on 16-bit) - but that comes with a cost of ram, runtime and codesize. I'm using it on a 32bit target and the disk size is with 15MiB way less than the 10TiB supported by 32bit scsi commands (if using 512 byte sectors). And I know it won't be a big difference but I still like it to be optimized. Thats why I propose a feature "scsi-u64", which is enabled by default, which enables the 64bit lba. The feature would enable the READ16, READ_CAPACITY16, WRITE16 command and uses u64 for lba. (Please be aware that I'm currently only looking at the scsi stuff but ufi has only u32's anyway) Since the length is always only 32bit, why is it a u64? (As I wrote earlier, WRITE16 is not implemented and thus READ16 is only helpful on read-only media.) I'll create a rp2040 scsi example after this is resolved either way. |
Since this library is primarily for embedded devices, I'd like an option to use only u32 for lba/len.
That limits the max. length to 4G and the disk size to 4 billion blocks (512B sector = 10T).
Here is a very first patch to add it:
master...alexkazik:forked-usbd-storage:u32
READ16 is in this case simply not supported, but it seems that this command is usually not used for devices which don't need it.
(WRITE16 is not implemented anyway; maybe a by default u64 feature is a better option)
The text was updated successfully, but these errors were encountered: