usb: add USB mass storage class support

[mikesmitty]

I've only tested it at a very basic level so far, but it's a somewhat faithful port of TinyUSB's mass storage class support and it shows up as a USB disk without errors in linux. There are still a number of things to be cleaned up, a few bits ironed out (clearing stalls doesn't work yet, for example), and I'll soon be implementing a FAT12 emulation package to go with it, but I figured I'd push it up as a draft for any comments

This is a basic program that runs it:

package main

import (
	"machine/usb/msc"
	"time"
)

func main() {
	m := msc.Port()
	for {
		println(m.State())
		time.Sleep(2 * time.Second)
	}
}

Edit: Oh, and tests will be coming as well

[b0ch3nski]

@mikesmitty cool stuff!

Regarding FAT emulation, have you seen @soypat repo? https://github.com/soypat/fat :)

[deadprogram]

I would suggest looking at https://github.com/tinygo-org/tinyfs/tree/release/fatfs since it is already being supported in tinyfs, which would probably be an important user of this feature.

[mikesmitty]

@b0ch3nski @deadprogram Thanks for the info, I should have probably explained a bit more. My goal is to emulate FAT12 with a TinyFS backend. My personal desired use case is to use LittleFS on disk, but present an emulated FAT12 over USB

[mikesmitty]

As I'm thinking about it, adding the FAT12 emulation bits probably make more sense in the TinyFS repo itself. This is the interface I put together for interfacing with the USB SCSI commands. It seems fine to me at first glance, but since it'd be a hard-to-change interface I want to make sure it makes sense:

type Disk interface {
	Ready() bool
	ReadOnly() bool
	BlockCount() uint32
	BlockSize() uint32
	Read(offset uint32, buffer []byte) (uint32, error)
	Write(offset uint32, buffer []byte) (uint32, error)
}

The big potential issue that I can foresee is that because I'm using a uint32 for the offset in Read/Write any emulated disks will be limited to a max of ~4GB in size. I originally was trying to allow it to use io.ReaderAt/io.WriterAt which would require converting the sector + offset used internally into a plain offset, but in hindsight I think that isn't a useful goal given the other required parts of the interface. Since it's already more than just throwing a simple reader/writer at it, abstracting away the concept of sectors and as a result limiting potential max disk size probably isn't very useful

[soypat]

I arrived at the following abstraction for a block device. I'd omit Ready and ReadOnly methods for now. Maybe there's a better way of doing things leveraging abstractions in other Go standard library packages such as the fs package for permissions.

type BlockReader interface {
	ReadBlocks(dst []byte, startBlock int64) (int, error)
}

type BlockWriter interface {
	WriteBlocks(data []byte, startBlock int64) (int, error)
}

// BlockDevice is the basic interface that must be implemented for a device to function properly.
type BlockDevice interface {
	BlockReader
	BlockWriter
	EraseBlocks(startBlock, numBlocks int64) error
	BlockSize() int
	BlockCount() int64
}

[mikesmitty]

Thanks for weighing in @soypat, I could roll with that interface and those suggestions. I think I'll need to implement the UNMAP SCSI command in order for EraseBlocks() to be usable, but given that just about everything this will write to is probably going to be flash storage that's probably for the best anyway. I realized afterwards that it's a pretty likely that this would at some point be used to provide access to an SD card over USB so artificially restricting the interface to low-GB volume sizes instead of TB volume sizes is definitely sub-optimal

[mikesmitty]

So I completely forgot that machine.BlockDevice exists while thinking I needed to create another blockdevice-type interface, and that the erase block size vs write block size is also super important for discard operations. I'm super flexible about whatever shape the interface ends up taking

[soypat]

I guess do with BlockDevice for now since its in the machine package

[deadprogram]

Once #4850 has had the merge conflicts resolved, and also merged, this PR will need the same.

It should, at that point, be a lot easier to review.

Is there some related code that you are using to test out the full file system mount etc @mikesmitty ?

Can I just say now very excited I am about this work, and how much I want/need to use it! 😻

[mikesmitty]

Is there some related code that you are using to test out the full file system mount etc @mikesmitty ?

I'm still working through my port (though it's turning into a bit of a rewrite as it goes on) of the FAT12 emulation layer in https://github.com/oyama/pico-littlefs-usb, but for testing in the mean time I've been using sg_vpd from the sg3-utils tool suite to verify the VPD communication fields:

• Supported VPD pages: sg_vpd -p sv /dev/sda
• Block Device Characteristics: sg_vpd -p bdc /dev/sda
• Block Limits: sg_vpd -p bl /dev/sda
• Logical Block Provsioning: sg_vpd -p lbpv /dev/sda
• Show all VPD page data: sg_vpd -a /dev/sda
  There's also a -H flag to provide a hex output of the returned data if that's useful for CI/CD later

And for testing the UNMAP/discard operations I used the blkdiscard tool, but found that I needed to manually enable UNMAP for the disk through sysfs first:

sudo sh -c 'echo "unmap" >/sys/block/sda/device/scsi_disk/0:0:0:0/provisioning_mode'

Because of the small size of the emulated disk I was only able to get blkdiscard to test single-pass, full-device discard operations so multi-packet UNMAP commands are as of yet untested (the else block on line 62 here: https://github.com/tinygo-org/tinygo/pull/4844/files#diff-28a9c9ce5c40a794d3a2b9f2db9aff7efae6a0f21d4420534cbc8a7243d41dfbR62), but single-packet commands worked fine. If I can't find a way of testing multi-packet UNMAP commands we can also just simplify it down to accept only single-packet UNMAP commands and set the max unmap descriptor block count here to 3: https://github.com/tinygo-org/tinygo/pull/4844/files#diff-7229572bc6cfb2b3f9b13656ea876d7612ec27a3c20e255e8c632bd9436b95aeR31

For read/write commands I've just been using dd to/from the emulated disk, though the emulated disk I'm using doesn't properly verify if writes are occurring, just that it's not throwing errors. Since it's using machine.BlockDevice now I'll have to put together some code to do some read/write/verification to the flash on a pico to make sure the writes are happening as intended

All that said, of course after giving it another read through I noticed a number of comments and little issues that need to be updated/fixed. Once I have a chance to look over the review in #4850 and that gets merged I'll rebase and fix those

[deadprogram]

Now that #4850 has been merged into dev this PR will need to be rebased @mikesmitty if you please.

Getting closer! 😅

[mikesmitty]

Doing some follow-up tests with a real backing flash device I'm seeing some errors. I'll need to do some diagnosing and cleanup before it's ready to be reviewed

[mikesmitty]

After testing it on actual flash hardware I realized that it was trying to perform writes in interrupt context and that needed to be handled differently. Needed to make another change to the RxHandler in order to allow for waiting until a write goes through before ACKing the message, otherwise the USB host will just keep sending data faster than we can write it.

I still need to do some proper testing, but since it's been a hot minute and I finally got it in a stable place I figured I'd push an update

[deadprogram]

I did notice that this PR increased the size of some binaries, such as this failing test: https://github.com/tinygo-org/tinygo/actions/runs/14986938840/job/42102543852#step:17:28

We should either change the size of the expected if we consider this increase acceptable, or else look into what might be able to avoid it, since the example that increased size did not use the features exposed by the new code in this PR.

[mikesmitty]

Ok, got the bugs with using machine.Flash sorted out for linux hosts. Now with a simple msc.Port(machine.Flash) command it will show up as a usb flash drive on linux and I was able to format it/copy files to it/etc. I also fixed one bug preventing macos from recognizing the drive, but there's still at least one more left I'm looking at.

We should either change the size of the expected if we consider this increase acceptable, or else look into what might be able to avoid it, since the example that increased size did not use the features exposed by the new code in this PR.

Sure, I'll look into it once I've got the macos bug fixed

[sago35]

I tried the following source code on Windows 11. Unfortunately, enumeration fails. I'll take a closer look this weekend.

However, I was able to mount it on Android.

package main

import (
	"machine"
	"machine/usb/msc"
	"time"
)

func main() {
	m := msc.Port(machine.Flash)
	for {
		println(m.State())
		time.Sleep(2 * time.Second)
	}
}

[mikesmitty]

I tried the following source code on Windows 11. Unfortunately, enumeration fails. I'll take a closer look this weekend.

Awesome, this will print out the commands it's receiving from the host:

m := msc.Port(machine.Flash)
m.SetMsgLogSize(1024)
for {
	for _, msg := range m.MsgLog() {
		if msg.Valid {
			cmd := msg.CBW.SCSICmd()
			println(cmd.String())
		}
	}
	m.ClearMsgLog()
	println(m.State())
	time.Sleep(2 * time.Second)
}

[sago35]

I also tried the code you provided.

On Windows, enumeration fails, so I can't check the println() output on the PC. In this case, it's likely difficult to confirm even with Wireshark, so I'll use a logic analyzer to inspect the USB bus directly.

Alternatively, I’ll check via the debugger. Lately, I’ve mostly been using the RP2040 Zero, so I’ll prepare a different microcontroller that can connect to a debugger.

[mikesmitty]

Oh, interesting that it doesn't enumerate at all. I've been using -serial=uart for debug output, but the logic analyzer output could be helpful

[mikesmitty]

Ok, finally fixed that bug on macos that was preventing it from doing a successful read, and I bet it will likely fix the windows issue also. It was essentially double-sending a signal message at the end of each read request that linux would ignore, but macos was choking on.

As far as tests go, the only method that has come to mind so far is scripting around dd and sg-utils against real hardware to verify inputs/outputs, but I'm open to suggestions

[deadprogram]

This PR is making amazing progress @mikesmitty

I would suggest adding an example/smoke test to at least ensure that the code builds, and also to give implementers some idea of where to start.

As far as real hardware tests, that would be an interesting thing to add to the TinyHCI.

[mikesmitty]

Thanks! Here's a smoke test and some partial support for the atsamd21/51 and nrf52840

[sago35]

It seems there was an issue with my verification. I was able to confirm that both a1ce6ba and 0ab6e25 are recognized as MSC. I’ll take a closer look at the code tonight.

[sago35]

This could be problematic for other microcontrollers. For example, the atsamd21, atsamd51, and nrf52840 only have a total of 8 endpoints. Some sort of conditional branching might be necessary.

[sago35]

I tested it on the atsamd51, and enumeration fails. I can investigate this part on my end.

$ tinygo flash --target wioterminal --size short --monitor ./src/examples/serial/
   code    data     bss |   flash     ram
   7420     108    7000 |    7528    7108
unable to search for a default USB device - use -port flag, available ports are COM5, COM4

[mikesmitty]

Oh, good to know about the endpoints. I'll do some thinking about that

[sago35]

The reason USB is no longer working on devices other than rp2040 / rp2350 was the following: on samd21 / samd51 / nrf52840, the maximum value that can be set is only 8. There is a part where a for range loop is used on endPoints, so that area needs to be fixed.

tinygo/src/machine/machine_atsamd21_usb.go

Line 176 in e165df3

for i = 1; i < uint32(len(endPoints)); i++ {

tinygo/src/machine/machine_atsamd51_usb.go

Line 179 in e165df3

for i = 1; i < uint32(len(endPoints)); i++ {

tinygo/src/machine/machine_nrf52840_usb.go

Line 173 in e165df3

for i = 1; i < uint32(len(endPoints)); i++ {

tinygo/src/machine/usb.go

Line 261 in e165df3

for i := 1; i < len(endPoints); i++ {

For now, I think the simplest and best approach is to define endPoints separately for each microcontroller. Only rp2040 and rp2350 will have 10, and all others will have 8.

[mikesmitty]

For now, I think the simplest and best approach is to define endPoints separately for each microcontroller. Only rp2040 and rp2350 will have 10, and all others will have 8.

Sure, sounds good to me. I think long-term a refactor to allow dynamic registration of endpoints and/or allowing endpoint numbers to be used for both in or out endpoints would be ideal, and I toyed with implementing them on another branch, but ended up being too much for right now.

[deadprogram]

I think long-term a refactor to allow dynamic registration of endpoints and/or allowing endpoint numbers to be used for both in or out endpoints would be ideal, and I toyed with implementing them on another branch, but ended up being too much for right now.

That seems very reasonable to me.

As far as I can tell this PR seems to be complete enough. @mikesmitty some of the commits probably could be squashed together instead of just squashing them all.

Everyone who can test this out please do so and give any feedback/bug reports so we can try to get this merged! cc/ @sago35 @mateusznowakdev @soypat @b0ch3nski anyone else!

[deadprogram]

The TinyHCi was able to run the normal hardware integration tests successfully on a test branch based off of this branch!

See #4917

[mateusznowakdev]

On SAMD21, the USB serial and HID seems to still work fine with this code applied.

But looking at the endPoints slice, and the lack of MSC there due to hardware limitations, does it mean it's not going to work for SAMD and NRF, at least not yet?

Trying to run the code as is, the machine.ConfigureUSBEndpoint() is trying to access 8th and 9th element from the slice, and of course panics. It took me some time to figure out why this happened.

It would be nice to show a message in serial console (or return error) if ep.Index >= len(endPoints), or handle it some other way.

[mateusznowakdev]

Well, I've just installed CircuitPython to check how they worked around this problem, and it looks like they didn't have to -- endpoints can be bi-directional, at least on SAMD:

This is actually documented in one of Adafruit's tutorials:

The SAMD21, SAMD51, nRF52840, and RP2040 microcontrollers all provide 8 endpoint pairs each. since pair 0 is reserved, 7 pairs are available, and so enabling all the devices above just fits.

https://learn.adafruit.com/customizing-usb-devices-in-circuitpython/how-many-usb-devices-can-i-have

The SAMD21 datasheet claims the same:

Device mode

– Supports 8 IN endpoints and 8 OUT endpoints

https://ww1.microchip.com/downloads/en/DeviceDoc/SAM_D21_DA1_Family_DataSheet_DS40001882F.pdf -- 32.2 Features

However, it doesn't look like the current USB implementation in TinyGo allows for such setup. Maybe we should merge this PR, and then consider what approach is easier / more reliable -- bidirectional endpoints or dynamically registered endpoints.

[mikesmitty]

As far as I can tell this PR seems to be complete enough. @mikesmitty some of the commits probably could be squashed together instead of just squashing them all.

Sure I can do that, or would you prefer that I break out the non-mass storage changes into another PR?

Also, I realized after looking back at it again that in the MSC descriptor only endpoints 1, 2, 3, and 8/9 were being used so I switched to re-using 6/7 instead. I want to say there's still another change that is needed with the samd and nrf chips in order for them to support it, but I forget off the top of my head. I want to say the delayed ack support still needs to be implemented

[deadprogram]

would you prefer that I break out the non-mass storage changes into another PR?

That would make this oh so much easier to review! Please and thank you.

Also, I realized after looking back at it again that in the MSC descriptor only endpoints 1, 2, 3, and 8/9 were being used so I switched to re-using 6/7 instead.

Very good idea, IMO. We need to conserve where we can.

I want to say there's still another change that is needed with the samd and nrf chips in order for them to support it, but I forget off the top of my head. I want to say the delayed ack support still needs to be implemented.

🤔 maybe someone else can spot any missing parts?

[sago35]

My view is that, at a minimum, we should avoid breaking existing features like USBCDC on samd21/51 or nrf52840. So for this PR, I think it’s sufficient to just handle endPoints. It makes sense to position this PR as MSC support for rp2. I don’t think it’s a problem if MSC support isn’t yet available on samd21/51 or nrf52840.