lib/fatfs: Support sector sizes beyond 512 bytes.

[pfalcon]

If MICROPY_FATFS_MAX_SS is defined to power of 2 value between 1024 and
4096, support for dynamic sector size in FatFs will be enabled. Initial
target if fsusermount subsystem, whose block device object should provide
secsize() method returning sector size. Note that FatFs reserves static
buffer of MICROPY_FATFS_MAX_SS size for each filesystem in use, so that
value should be set sparingly.

[pfalcon]

Next step towards generic VFS support for FatFs.

[dpgeorge]

Instead of adding a secsize method, we should reuse the count method. If it returns a single int, then that tells the number of sectors, with default size of 512 bytes. If it returns a tuple, then that tells (sec_size, num_sec). The count method will only be called at mount/mkfs so it's not heavily used.

In fact, maybe we should just call the count method when the volume is mounted and store the result in the fs_user_mount_t object. Wouldn't take any extra room because you can get rid of the count[2] item in the struct.

[pfalcon]

Instead of adding a secsize method, we should reuse the count method.

Definitely could, though I'm not sure should. I wouldn't say that it's obvious that count() could return sector size, and there're many other alternatives how to do it, at least current patch is simple.

If it returns a single int, then that tells the number of sectors, with default size of 512 bytes. If it returns a tuple, then that tells (sec_size, num_sec).

Why such strange order? I'd expect optional value to go at the end, (num_sec, sec_size).

In fact, maybe we should just call the count method when the volume is mounted and store the result in the fs_user_mount_t object. Wouldn't take any extra room because you can get rid of the count[2] item in the struct.

And here we come to the alternatives. Maybe there should be single ioctl() method to cover sync(), count(), secsize(), etc. This will also map better to C-level vtable. But still will require to define ioctl constants on Python level. But if we want to call count() once at the mount time, maybe it shouldn't be a method at all, but an instance variable? (Method is flexible.) Replacing secsize[2] with sec_size isn't big saving, but if you look how GET_SECTOR_SIZE is implemented, it calls ioctl callback with pointer to a field in FATFS structure. We could initialize it to needed value and just have empty GET_SECTOR_SIZE handler (which doesn't overwrite existing value).

So many opportunities! Of these, I'd find ioctl() method the most viable, if we want to support block devices abstraction on C level. And the current patch as the simplest way to achieve needed functionality (without pledging that it's stable API).

[dpgeorge]

I think we should design and implement the block protocol from the outset so that we don't expect it to change.

ioctl() sounds like the most general thing to do, although I do like the idea of instance variables for sec_size and sec_count (they would be much more RAM friendly than methods that anyway return an instance variable with the pre-computed value).

@peterhinch has made extensive use of the block protocol, so may have some input here. IIRC, he asked for a way to signal to the block device that it has been unmounted, so it could be turned off.

The things an ioctl method could be used for are:

• ioctl(GET_SEC_SIZE) -> int [or could be an instance variable]
• ioctl(GET_SEC_COUNT) -> int [or could be an instance variable]
• ioctl(SYNC) -> None
• ioctl(UMOUNT) -> None

We'd probably want to specify that ioctl takes a second argument for future upgrades to the API.

Note that with readblocks, writeblocks and ioctl, this protocol is a good counterpart to the stream protocol (read, write, ioctl).

[pfalcon]

Note that with readblocks, writeblocks and ioctl, this protocol is a good counterpart to the stream protocol (read, write, ioctl).

Yes, that's the idea behind it. But that still doesn't answer what to do now, with the aim of bringing up filesystem to esp8266 asap - should support for sync()/count() be removed and replaced with ioctl()?

[dpgeorge]

should support for sync()/count() be removed and replaced with ioctl()?

Let's try to make it backwards compatible for the time being. I will make a PR.

[dpgeorge]

Closed in favour of #1830.