Upgrade block protocol to use ioctl method, and support fatfs sectors > 512 bytes #1830
Conversation
|
This has been tested on pyboard with a RAMFS driver. |
| case GET_SECTOR_SIZE: { | ||
| vfs->u.ioctl[2] = MP_OBJ_NEW_SMALL_INT(2 /* SEC_SIZE */); | ||
| mp_obj_t ret = mp_call_method_n_kw(1, 0, vfs->u.ioctl); | ||
| *((WORD*)buff) = mp_obj_get_int(ret); |
There was a problem hiding this comment.
@pfalcon in your PR you had this as a DWORD, but I think it should be a WORD pointer.
There was a problem hiding this comment.
Yep, I just copy-pasted it, thinking that return arg size is standard in all cases, but it doesn't have to.
|
Look good, thanks!
Do you remember my idea that some well-known value should be used for "query interface" call? That can be 0, or 1 (with 0 still unused).
Yes (can wait).
Yes, let's have standard ioctl format of ioctl(req_no, arg) (plus self). |
|
Ok, I'll change it so cmd=0 is invalid, and ioctl is ioctl(self, cmd, arg). As for the ioctl cmds, we could add an INIT command to tell the device to power up, as well as UNMOUNT so it can power down if it wants. Note that fatfs supports initialisation of drives, so INIT would be called from this location (disk_initialize). We could make it so INIT returned a tuple of sec_size and sec_count, instead of separate cmds for getting these values. Thoughts? |
This kind of questions is hard to answer before we integrated a dozen of FSes. I'd say stay close to actual FS API we have at hand, and be ready to refactor it all. But even without it, I don't see why you want to tie sector number and sector size into one call. Consider for example that getting "sector number" for a type may require rewinding it, then you want that rewinding to happen twice. |
The new block protocol is: - readblocks(self, n, buf) - writeblocks(self, n, buf) - ioctl(self, cmd, arg) The new ioctl method handles the old sync and count methods, as well as a new "get sector size" method. The old protocol is still supported, and used if the device doesn't have the ioctl method.
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. Note that FatFs reserves static buffer of MICROPY_FATFS_MAX_SS size for each filesystem in use, so that value should be set sparingly. Initial patch provided by @pfalcon.
dpgeorge
force-pushed
the
block-protocol
branch
from
312c1cf
to
13a4c12
Compare
|
Ok, this has been updated:
Ok to merge? |
Yes, thanks, please go ahead! |
This PR adds the ioctl method to the block protocol, and retains backwards compatibility with the old version (which would now be deprecated).
New protocol consists of:
Some open questions: