10000 MSC exclusive access support. by iabdalkader · Pull Request #8814 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

MSC exclusive access support. #8814

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

MSC exclusive access support. #8814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions extmod/vfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#define MP_BLOCKDEV_IOCTL_BLOCK_COUNT (4)
#define MP_BLOCKDEV_IOCTL_BLOCK_SIZE (5)
#define MP_BLOCKDEV_IOCTL_BLOCK_ERASE (6)
#define MP_BLOCKDEV_IOCTL_STATUS (7)

// At the moment the VFS protocol just has import_stat, but could be extended to other methods
typedef struct _mp_vfs_proto_t {
Expand Down
26 changes: 20 additions & 6 deletions extmod/vfs_fat_diskio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ DRESULT disk_ioctl(
[GET_SECTOR_COUNT] = MP_BLOCKDEV_IOCTL_BLOCK_COUNT,
[GET_SECTOR_SIZE] = MP_BLOCKDEV_IOCTL_BLOCK_SIZE,
[IOCTL_INIT] = MP_BLOCKDEV_IOCTL_INIT,
[IOCTL_STATUS] = MP_BLOCKDEV_IOCTL_STATUS,
};
uint8_t bp_op = op_map[cmd & 7];
mp_obj_t ret = mp_const_none;
Expand Down Expand Up @@ -147,16 +148,29 @@ DRESULT disk_ioctl(
*((DWORD *)buff) = 1; // erase block size in units of sector size
return RES_OK;

case IOCTL_INIT:
case IOCTL_STATUS: {
DSTATUS stat;
case IOCTL_INIT: {
DSTATUS stat = 0;
if (ret != mp_const_none && MP_OBJ_SMALL_INT_VALUE(ret) != 0) {
// error initialising
stat = STA_NOINIT;
} else if (vfs->blockdev.writeblocks[0] == MP_OBJ_NULL) {
stat = STA_PROTECT;
} else {
stat = 0;
// IOCTL_INIT only returns non 0 for all errors/flags. To return
// a more accurate disk state, IOCTL_STATUS is called again here.
ret = mp_vfs_blockdev_ioctl(&vfs->blockdev, MP_BLOCKDEV_IOCTL_STATUS, 0);
if (vfs->blockdev.writeblocks[0] == MP_OBJ_NULL ||
(ret != mp_const_none && MP_OBJ_SMALL_INT_VALUE(ret) != 0)) {
stat = STA_PROTECT;
}
}
*((DSTATUS *)buff) = stat;
return RES_OK;
}

case IOCTL_STATUS: {
DSTATUS stat = 0;
if (vfs->blockdev.writeblocks[0] == MP_OBJ_NULL ||
(ret != mp_const_none && MP_OBJ_SMALL_INT_VALUE(ret) != 0)) {
stat = STA_PROTECT;
}
*((DSTATUS *)buff) = stat;
return RES_OK;
Expand Down
7 changes: 6 additions & 1 deletion ports/rp2/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ int main(int argc, char **argv) {

#if MICROPY_HW_ENABLE_USBDEV
bi_decl(bi_program_feature("USB REPL"))
tusb_init();
#endif

#if MICROPY_PY_THREAD
Expand Down Expand Up @@ -147,6 +146,12 @@ int main(int argc, char **argv) {
pyexec_frozen_module("_boot.py");
#endif

#if MICROPY_HW_ENABLE_USBDEV
if (!tusb_inited()) {
tusb_init();
}
#endif

// Execute user scripts.
int ret = pyexec_file_if_exists("boot.py");
if (ret & PYEXEC_FORCED_EXIT) {
Expand Down
1 change: 1 addition & 0 deletions ports/rp2/mpconfigport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
#if MICROPY_HW_USB_MSC
#define MICROPY_FATFS_USE_LABEL (1)
#define MICROPY_FATFS_MULTI_PARTITION (1)
#define MICROPY_HW_USB_MSC_EXCLUSIVE_ACCESS (1)
// Set FatFS block size to flash sector size to avoid caching
// the flash sector in memory to support smaller block sizes.
#define MICROPY_FATFS_MAX_SS (FLASH_SECTOR_SIZE)
Expand Down
13 changes: 9 additions & 4 deletions ports/rp2/msc_disk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
#define FLASH_BASE_ADDR (PICO_FLASH_SIZE_BYTES - MICROPY_HW_FLASH_STORAGE_BYTES)
#define FLASH_MMAP_ADDR (XIP_BASE + FLASH_BASE_ADDR)

static bool ejected = false;
bool tud_msc_ejected = true;
extern void tud_msc_remount(void);

// Invoked when received SCSI_CMD_INQUIRY
// Application fill vendor id, product id and revision with string up to 8, 16, 4 characters respectively
Expand All @@ -51,12 +52,13 @@ void tud_msc_inquiry_cb(uint8_t lun, uint8_t vendor_id[8], uint8_t product_id[16
strncpy((char *)vendor_id, vid, 8);
strncpy((char *)product_id, pid, 16);
strncpy((char *)product_rev, rev, 4);
tud_msc_ejected = false;
}

// Invoked when received Test Unit Ready command.
// return true allowing host to read/write this LUN e.g SD card inserted
bool tud_msc_test_unit_ready_cb(uint8_t lun) {
if (ejected) {
if (tud_msc_ejected) {
tud_msc_set_sense(lun, SCSI_SENSE_NOT_READY, 0x3a, 0x00);
return false;
}
Expand All @@ -77,10 +79,13 @@ bool tud_msc_start_stop_cb(uint8_t lun, uint8_t power_condition, bool start, boo
if (load_eject) {
if (start) {
// load disk storage
ejected = false;
tud_msc_ejected = false;
} else {
// unload disk storage
ejected = true;
tud_msc_ejected = true;
#if MICROPY_HW_USB_MSC_EXCLUSIVE_ACCESS
tud_msc_remount();
#endif
}
}
return true;
Expand Down
8 changes: 8 additions & 0 deletions ports/rp2/rp2_flash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
#define MICROPY_HW_FLASH_STORAGE_BASE (PICO_FLASH_SIZE_BYTES - MICROPY_HW_FLASH_STORAGE_BYTES)
#endif

extern bool tud_msc_ejected;

static_assert(MICROPY_HW_FLASH_STORAGE_BYTES <= PICO_FLASH_SIZE_BYTES, "MICROPY_HW_FLASH_STORAGE_BYTES too big");
static_assert(MICROPY_HW_FLASH_STORAGE_BASE + MICROPY_HW_FLASH_STORAGE_BYTES <= PICO_FLASH_SIZE_BYTES, "MICROPY_HW_FLASH_STORAGE_BYTES too big");

Expand Down Expand Up @@ -137,6 +139,12 @@ STATIC mp_obj_t rp2_flash_ioctl(mp_obj_t self_in, mp_obj_t cmd_in, mp_obj_t arg_
// TODO check return value
return MP_OBJ_NEW_SMALL_INT(0);
}
case MP_BLOCKDEV_IOCTL_STATUS:
#if MICROPY_HW_USB_MSC
return MP_OBJ_NEW_SMALL_INT(!tud_msc_ejected);
#else
return MP_OBJ_NEW_SMALL_INT(false);
#endif
default:
return mp_const_none;
}
Expand Down
26 changes: 26 additions & 0 deletions shared/runtime/tinyusb_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,29 @@ void tud_cdc_line_state_cb(uint8_t itf, bool dtr, bool rts) {
}

#endif

#if MICROPY_HW_USB_MSC_EXCLUSIVE_ACCESS
#include "tusb.h"
#include "extmod/vfs.h"
static mp_sched_node_t mp_remount_sched_node;

STATIC void tud_msc_remount_task(mp_sched_node_t *node) {
mp_vfs_mount_t *vfs = NULL;
for (vfs = MP_STATE_VM(vfs_mount_table); vfs != NULL; vfs = vfs->next) {
if (vfs->len == 1) {
nlr_buf_t nlr;
if (nlr_push(&nlr) == 0) {
mp_vfs_umount(vfs->obj);
nlr_pop();
}
mp_obj_t path = mp_obj_new_str(vfs->str, strlen(vfs->str));
mp_vfs_mount_and_chdir_protected(vfs->obj, path);
break;
}
}
}

void tud_msc_remount(void) {
mp_sched_schedule_node(&mp_remount_sched_node, tud_msc_remount_task);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At which state will tud_msc_remount be called? Only when the device is ejected? Or is it possible that the PC may remount the drive? I cannot do that with my Linux box. If the board has always go through a power cycle, then it's fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming that a board with MSC support will start in locally write protected mode. What happens if the board is running stand-alone? Can it tell that there is no MSC mounted? Then the drive must be mounted write enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gets called when the device is ejected. It may be possible to remount a safely-removed/ejected drive, but I think that disconnects power from the port anyway, I will have to test to confirm. However, I think that's a very unlikely case that shouldn't be handled, not sure if it should remount as read-only after giving write access to the device.

Copy link
Contributor
@robert-hh robert-hh Jun 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm chewing on the timing issue. At the moment, the board starts with write enabled, to be able to create a file system, if it does not exist. Then it mounts the flash drive, which must be write protected if MSC is used. It could decide based on the status of the IOCTL_STATUS call. But how long does that status change take?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. So the tud_msc_start_stop_cb() is only called on eject. The ud_msc_inquiry_cb() is called on start, as well as the tud_msc_capacity_cb(). So we could use these as indicator, that a PC is attempting to connect.
The tud_msc_test_unit_ready_cb() is called regularly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the softreboot the file is gone. test without the soft reboot.
But I found a possible reason at my set-up: In tud_msc_remount_task() you have a check for len == 1. That is the length of the mount point name. On my set-up, the length is 6. So the remount never happens.
Changing the it to len == 6, the code is executed, and I can single-step it in the debugger. Then I see a strange phenomenon: while single stepping though unmount, remount, .., the PC still shows: "Unmount in progress, please do not remove the drive". Only after the remount function is finished, this message disappears. So it looks like in addition to the non-matching length check, the remount at the board was too early.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it looks like in addition to the non-matching length check, the remount at the board was too early.

maybe a false alarm: The PC/USB simple code waits for tud_msc_start_stop_cb() to finish.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes maybe because you're single stepping the function. The remount is scheduled, so this function returns immediately. Anyway, the mount point of the MSC filesystem should be defined somewhere, or even better if it's detected dynamically somehow, but I'm not sure how to do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I single stepped in the tud_msc_remount_task() function. So I know it is called.
At the moment, the internal flash is the first mounted file system. I do not know if we can reliably assume that this is always the case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do not find a good solution, we could at least defer opening the file system for write until the next soft boot. That's at least safe.

}
#endif
0