Cleanest way to block 32F417 USB interrupts?

[peter-h]

I have a 4MB serial flash - Adesto SPI, using SPI2 on the 32F417.

Code was generated / extracted from Cube ST libraries to implement a removable storage device via USB, in 2MB of this flash device. This works great. It uses interrupts.

However, other stuff is also accessing this serial flash. The 2MB file system is also accessed via FatFS and that also works, so files can be exchanged between embedded code and say a Windows computer via USB. And other parts are used as a "linear flash" for data logging etc.

I have started doing thorough tests on the data logging code and it runs for a bit and then spectacularly crashes. The stack dump in Cube shows a ton of total crap. Almost certainly the reason is that the USB interrupts are getting in at any time, and both SPI and the flash device itself are obviously not re-entrant

A lot of the serial flash access code is running in main() before USB interrupts are enabled, which is why this issue has only just now surfaced - a year after starting the project. But now I am running test code as an RTOS thread so it exposes the problem.

I am using mutexes to protect SPI and the flash device on the end of it, but this merely enables FatFS and data logging calls to be done in multiple RTOS threads. One cannot use a mutex to block an ISR.

The obvious hack is to block USB interrupts around the SPI2 functions. These functions are called both before and after USB interrupts are enabled, so the usual way is to save the interrupt enable status, disable ints, and restore the status upon exit.

The Q is where. The USB code, done by someone else a couple of years ago, is so vastly complex nobody understands it. But I found these snippets, in which just two lines are interesting:

Code: [Select]

HAL_StatusTypeDef HAL_PCD_Start(PCD_HandleTypeDef *hpcd)
{
#if defined (USB_OTG_FS) || defined (USB_OTG_HS)
  USB_OTG_GlobalTypeDef *USBx = hpcd->Instance;
#endif /* defined (USB_OTG_FS) || defined (USB_OTG_HS) */

  __HAL_LOCK(hpcd);
#if defined (USB_OTG_FS) || defined (USB_OTG_HS)
  if ((hpcd->Init.battery_charging_enable == 1U) &&
      (hpcd->Init.phy_itface != USB_OTG_ULPI_PHY))
  {
    /* Enable USB Transceiver */
    USBx->GCCFG |= USB_OTG_GCCFG_PWRDWN;
  }
#endif /* defined (USB_OTG_FS) || defined (USB_OTG_HS) */
  (void)USB_DevConnect(hpcd->Instance);
  __HAL_PCD_ENABLE(hpcd);
  __HAL_UNLOCK(hpcd);
  return HAL_OK;
}

/**
  * @brief  Stop the USB device.
  * @param  hpcd PCD handle
  * @retval HAL status
  */
HAL_StatusTypeDef HAL_PCD_Stop(PCD_HandleTypeDef *hpcd)
{
  __HAL_LOCK(hpcd);
  __HAL_PCD_DISABLE(hpcd);

  if (USB_StopDevice(hpcd->Instance) != HAL_OK)
  {
    __HAL_UNLOCK(hpcd);
    return HAL_ERROR;
  }

  (void)USB_DevDisconnect(hpcd->Instance);
  __HAL_UNLOCK(hpcd);

  return HAL_OK;
}

__HAL_PCD_ENABLE(hpcd) ;
__HAL_PCD_DISABLE(hpcd) ;

and these are macros and it's obvious what they do:

USBx->GAHBCFG |= 1;
USBx->GAHBCFG &= ~1;

Looking in the RM, page 1274, I see this is toggling a global USB interrupt enable bit, but the description (int pending presentation to application) puzzles me. Is it the right one?

Stepping through the code to see what USBx should be, I see 0x50000000. It is also defined as USB_OTG_FS_PERIPH_BASE. So the way to obtain that bit 0 value is

uint32_t temp = USB_OTG_FS_PERIPH_BASE->GAHBCFG;  // bit 0 holds current int enable status - wrong syntax though; this works

#define USB_CF *(volatile uint32_t*) (USB_OTG_FS_PERIPH_BASE+GAHBCFG)
uint32_t temp=USB_CF

Does this make sense?

Thank you very much for any pointers.

There are more cunning ways to do this, as always, e.g. getting the USB ISR to save a write or read request which is then processed by an RTOS thread, but this should work It is not a high performance USB application.

[DC1MC]

IMHO, blocking the USB interrupts is a sure way to get USB errors, of course one can use such a "Pfuscherei" solution, but as long as you have the use case of simultaneous flash access. you'll have to deal with it sooner or later.

The proper way to do it is to create a "device driver", that is, a process that exclusively access the flash and the USB and local logging sub-system talk with this process and not directly with the flash. Of course, you can also make the USB access have higher priority, because is relatively time critical. With a RTOS is not such a big deal, there are even some implementation floating around, bare metal is a bit of a challenge.


Cheers,
DC1MC

[voltsandjolts]

I don't see why you would need to block interrupts, that would just break the USB link anyway.

The USB host sends IN requests for data, and you NAK until you are ready, then respond with the data.
As long as you are within the 100ms (or whatever) of the request timeout it's all good.
Similarly for OUT, host keeps trying until you acknowledge receipt, within request timeout.

So, USB timing is very forgiving in that respect.
As long as the other processes accessing FLASH don't lock it for tens/hundreds of ms (i.e. more than the USB request timeout) it should be fine.

[peter-h]

The problem is that the USB ISR calls the SPI2 code which then talks to the serial flash - all inside the ISR.

So if there is a foreground task (in this case an RTOS task) accessing the serial flash, that gets interrupted and if the timing is "just right" the hardware itself gets screwed-up because you can't have "SPI within SPI", obviously.

The hack I proposed above has solved the crashing. Right now it seems unbreakable. I can have test code writing (random numbers) solidly into the flash, and writing a 1MB jpeg to the block device and then reboot the target and read it back and it is fine. But I am getting some artefacts on debugs coming out via the USB VCP, which is something else... that code has always been a bit flakey.

My reading of that RM page is that setting that bit to 0 blocks presentation of the interrupt pending to the ISR, and doesn't bugger up the USB subsystem itself. What that exactly does I don't know; I know almost nothing about USB other than it is a master/slave system.

[voltsandjolts]

The problem is that the USB ISR calls the SPI2 code which then talks to the serial flash - all inside the ISR.

Ahh, yes, there's the problem then.
You are effectively demanding immediate access to the SPI FLASH which ain't gonna happen every time.
That request needs to be handed off to a background task.

[mac.6]

If you use an RTOS, you should not doing more in ISR than ack the interrupt and send a event to a task for further processing.
Only very demanding realtime processing can be done under ISR.

[peter-h]

That is quite complicated, not least because I am accessing the serial flash well before the RTOS is running.

In fact I need to access the file system (FatFS) before the RTOS is running, which is possible but it slightly compromises the FatFS features e.g. the file system cannot be re-entrant without an RTOS, but the solution to that is easy (mutex).

There is also a boot loader which accesses it...

So I would need a queuing process where the serial flash is not accessed directly by anything, but that process has to function before the RTOS starts up.

Sure it could be done (e.g. two paths to the serial flash, one used before RTOS starts and another afterwards). Then spend a few days testing it

Disabling USB interrupts around flash r/w is much simpler. For reading the flash, they are disabled for around 500us. For writing the flash (512 byte sector) they are disabled for about 20ms.

However, writing the flash via USB is rare; it is basically dropping in a firmware update or a config file. Reading it is far more common; Windows is continuously polling USB removable devices for all kinds of stuff.

The other thing is that when an embedded app is accessing the serial flash, via FatFS (file system) or for data logging. Then USB reads are also disabled, potentially for 20ms, if the embedded side is writing the flash. So basically USB may be unresponsive for up to 20ms.

I am hoping someone knows the specifics of the USB subsystem, not the "ideal textbook" way of solving this

[voltsandjolts]

Well, that all sounds like day-one architecture problems.

I am hoping someone knows the specifics of the USB subsystem, not the "ideal textbook" way of solving this

The better solution for all involved would be if you spent some time learning (at least the fundamentals of) USB interfacing.
Rather surprised you didn't do that prior to starting a thread asking about USB.

[peter-h]

"The better solution for all involved", old chum, would be IF you knew something useful, you consider posting it, because it might help somebody

Masking that bit does work very well to stop the crashing but it produces occassional corruption of files written to the serial flash removable block device, via USB.

Maybe the text in the RM
Do not change this
register after the initial programming. The application must program this register before
starting any transactions on either the AHB or the USB.
is a clue - even though ST provide a function which does more or less exactly what I am doing.

So I am looking at other ways to disable USB interrupts. There is GINTMSK which can be used to mask all the USB interrupts, so I am setting that to zero (the reset value; all USB ints masked) around the serial flash access functions (one has to do only page read and page write; the USB block device doesn't use any others). That works considerably better but I get a USB error (the block device disappears) when writing large files via USB.

Probably, the loss of USB activity for a solid succession of 20ms slots is enough to muck it up. So, in my test code which is writing solidly to the serial flash I put in a 10ms delay after each page, which should be insignificant (30ms total page write versus 20ms) and this seems to have fixed the USB breakages. In fact I probably should stick a few ms delay after each USB write too, to give the RTOS time to do other stuff.

The USB subsystem in the 32F4 is immensely complex but IMHO the interrupts should work like any other interrupts. I will test other values to see how much breaks it, although it is pointless because one is just hanging up in an ISR.

I would agree it is dumb to be doing a 20ms flash write inside an ISR (especially as one cannot, for various reasons, make the entire operation non-blocking) but this is the code I have, from an ST library, and sorting that would be quite a lot of work.

[voltsandjolts]

"The better solution for all involved", old chum, would be IF you knew something useful, you consider posting it, because it might help somebody

I did, but your ignoring it. Let me try putting it another way:
Disabling USB interrupts is a really shit idea.
Fix your architecture.

[Deni]

You should also be aware that your local file system (FatFS) is not aware of another "user" of your flash storage - so, you can easily end-up with corrupted FAT table or inconsistent reads on the USB host side. USB MSC device acess your drive on block basis - that is, USB host takes care of the FAT file system on your local flash storage and FatFS has no idea that there's somebody else playing in it's playground too. You can somewhat put it under control using semaphores but if host also writes to the flash storage, then the only way is to temporarily unmount FatFS volume while host is attached. That's what I did on a similar project. It was acceptable, since when host was attached there were no "local" actions that would require flash disk access.

[peter-h]

Yes; absolutely. There is no trivial way around that. One approach to detection of USB host file activity is to monitor the USB traffic and look for specific packets. I can't remember but there is a specific packet which tells you the final FAT update was done.  This may be OS specific. Another is using a timeout but that doesn't work with FAT12 (see below). In the opposite direction, FatFS completing the writing of a file can be notified to USB host by doing an unmount/mount, like you say.

Internally, only FatFS is playing in the 2MB FAT12 filesystem, and that is mutex protected to be thread-safe.

There is a bastard of an issue with testing for file corruption: Windows, even latest, has a bug in that FAT12 volumes (anything below 4MB, IIRC) are not write-finalised until you "safely remove hardware". It's like win95 They fixed it for FAT16 etc some years ago.

[peter-h]

Is there some other method to delay a USB operation, involving returning an "error, please retry" packet to the USB host?

[cgroen]

Would MTP be another solution ? Then the disk will not be mounted by Windows but rather it will access it using "commands" which you can handle in the device, this will make the two systems get along nicer...

[Deni]

Yes, that would definitely be a better way, however MTP drivers are not so common as MSC. I think ST's Cube now supports it.

[peter-h]

Sadly it needs to be a standard USB removable device, like a "flash stick". The main use of this feature is for very occassional dropping of config files, firmware updates, etc. With a FAT file system and with the 100k write endurance of the flash (AT45DB321) one can't use it for anything else anyway.

Where I am at currently is this

- r/w to the flash from "inside" (data logging to the separate area) is rock solid, regardless of USB activity (this is a big "win" of the GINTMSK method, although the GAHBCFG (bit 0) method also achieved this)

- USB reads from the flash are rock solid

- USB writes to the flash are 100% solid if there is no inside activity

- USB writes to the flash are 99.9999% solid if the data logging w/r test is running

Obviously the last one is very hard to debug. I am seeing maybe a few bytes off on a 1MB file. I am writing, via USB, 1-2MB jpegs, on which corruption is instantly obvious.

The reliability of the USB writes approaches 100% (so closely it takes hours to get an issue, and in practice nobody will ever see an issue) if I put a 20ms delay in the data logging test code, so each block write (20ms) is followed by a 20ms delay. Some delay is needed anyway otherwise if the data logging writing was solid on the flash (unlikely but possible) then USB flash ops run extremely slowly.

My feeling is that this is something simple. I can't rule out an issue in the PC-USB interface. It could be something like the write to GINTMSK is affecting something, not just simply masking the interrupts. I've looked at e.g. the USB int happening right as the write to GINTMSK is made, but that should still work ok.

All I am currently doing is this:

Code: [Select]

// Register for disabling USB interrupts around serial FLASH r/w
//#define USB_OTG_FS_PERIPH_BASE 0x50000000 - already defined via stm32f4xx_hal.h
//#define GAHBCFG 8 - not used (bit 0 was a potential alternative)
#define GINTMSK 0x18
#define USB_CF *(volatile uint32_t*) (USB_OTG_FS_PERIPH_BASE+GINTMSK)

Code: [Select]

#ifdef USB_INT_PROTECT
// Disable USB interrupts, if they are enabled here
uint32_t temp = USB_CF;  // bit 0 holds current int enable status
USB_CF = 0;
#endif

flash_wait_busy();   // this is blocking, until any previous flash op finishes
flash_cs(0);
flash_writepage();
        flash_cs(1);

#ifdef USB_INT_PROTECT
// Restore previous USB int status
USB_CF = temp;
#endif
I did try making the exit blocking also. Should of course make no difference, and indeed it doesn't.

On top of this I am having to do the testing really carefully, to "safely remove" the device, and not reboot the target while the data logging activity is running in case that code is crapping over the filespace.

It gets better because I am also sending debugs to the PC (teraterm) via a USB VCP, and any issue there could be corrupting USB data, although USB packets are supposed to be CRC checked??

I am also suspecting that if you write a file via USB to the flash, and read it back, that readback is instant because it comes from the Windows cache, so it is meaningless for data comparison reasons, but the act of that readback does "something" to finalise pending writes. It would kind of make sense...

[gf]

There is a bastard of an issue with testing for file corruption: Windows, even latest, has a bug in that FAT12 volumes (anything below 4MB, IIRC) are not write-finalised until you "safely remove hardware". It's like win95 They fixed it for FAT16 etc some years ago.

My understanding is that Quick Removal disables only write caching, but read caching can still happen, and the cached data can be retained (and not read again from the drive) until the filesystem is unmounted (or in other words "savely removed"). Subsequent writes from the host may depend on the cached data (for instance when FAT or directory are updated), but the cached data may no longer be identical to the on-disk data then if the drive was written locally in the meantime. So don't expect data consistency if you write to the drive locally as long as it is mounted by the host. It is an exclusive or, either the drive is mounted locally, or by the host, but not both simultaneously.

I could imagine hacks to kick the host off from the filesystem (when it is mounted by the host). But the consequence is that a (well-behaved) filesystem driver on the host can only prevent further access to the mounted filesystem then, until it gets unmounted (or remounted). But this may not be desired either. [ For instance, if a storage device identifies itself as "removable media" device (RMB bit in the INQUIRY data), then it can signal a media change to the host by answering (rejecting) the next SCSI command with CHECK CONDITION, and returning sense code UNIT ATTENTION and additional sense code MEDIUM MAY HAVE CHANGED in the response to the hosts's subsequent REQUEST SENSE command. The filesystem driver on the host is then expected to react to the signalled media change then. ]

If simultaneous access is desired, I'd rather consider MTP, too, as others have suggested.

[abyrvalg]

Simpsons Android already did it. Before migration to MTP/PTP there was a big button “allow data access” suspending all app activities until unplug. Even if you solve the interrupts at wrong moments, the FS inconsistency problem will remain.

[peter-h]

I am testing the GAHBCFG method (setting bit 0 to 0 - which I earlier abandoned because of occassional errors) and it appears to work identically in the 99.9999% case

It thus seems to be the same as the GINTMSK method, where I was loading 0 into all the bits, to mask all the interrupts.

This is what I would expect, except the RM contains dark warnings about GAHBCFG as posted above.

In fact, last hour of testing, and doing the device removal ultra diligently, I can't break it, over quite a few MB of files written over USB.

The downside of this int disable method is that it also clobbers the USB VCP used for debugs. That isn't important, and I found that if after each internal flash page write (20ms) I have a delay of around 12ms+, the VCP works error-free.

A better way seems to be to get the USB ISR to return an error code (based on an "internal flash in use" flag being set) and then the USB Host should retry. I posted that further back above. It is some SCSI error code. This is the USB ISR:

Code: [Select]

/**
  * @brief  .
  * @param  lun: .
  * @retval USBD_OK if all operations are OK else USBD_FAIL
  *
  */
int8_t STORAGE_Write_FS(uint8_t lun, uint8_t *buf, uint32_t blk_addr, uint16_t blk_len)
{

if (FatFS_filesystem_mounted) return USBD_BUSY;
WritePage(buf, blk_len * STORAGE_BLK_SIZ, blk_addr * STORAGE_BLK_SIZ);
return (USBD_OK);
 
}
I had not noticed the bit which returns USBD_BUSY and that is probably the key.

But using this method doesn't work. Returning USBD_BUSY if an internal flash op is running does work to protect the flash, but any files written via USB are heavily corrupted even if no internal flash ops are running.

BUSY seems the right code because that is how one implements say a USB to serial converter; if the UART TX is full, you return BUSY.

[gf]

As said, I won't be sufficient to trigger the host to retry the SCSI command. This alone won't ensure data consistency. If you mount the file system locally, then you need to ensure that the hosts discards all cached information about the filesystem (for instance by pretending that the flash disk were a removable media device, and telling the host via SCSI sense keys that the medium was removed or changed).

[peter-h]

That's true, but it isn't a problem. The main issue is how to keep USB out of the flash r/w functions when another process is using the flash.

I originally solved it pretty well by disabling USB interrupts. The 2 places one can do that, in addition to doing it in the NVIC which is probably the best way because it avoids hardware related latencies. The main issue with doing this is that it screws up other USB functions like e.g. running debugs over a USB VCP, if there is heavy flash access (not really a big problem though).

What should be better is using the USBD_BUSY return code but today we found a bug in the ST code, in the USB SCSI layer which supports only codes of 0 and negative numbers, while the USB funcs return 0,1,2,3 The bug was reported here
https://community.st.com/s/question/0D53W00001Ie4WcSAJ/32f417-how-to-block-usbfs-isr-from-a-shared-resource-serial-flash-on-spi?t=1641814294067
It has been fixed today and it is working in that USB now keeps out of the flash in a well behaved manner.

Unfortunately something else has got broken which I am trying to fix and which exists only when some flash test code (writing into another part of the flash) is running... Currently I think that the USB host (Windows) simply doesn't like a USBD_BUSY status returned for as long as 20ms (the single flash page write time). The host retries when it gets this code; I don't know how often but probably at (of the order of) 1kHz. Whereas the host didn't mind at all getting nothing back for 20ms and then get a USBD_OK  Well, the obvious answer to this is to farm the USB write request to an RTOS thread which executes it and returns USBD_OK at the end. I have never done FreeRTOS threads which are thus triggered and which totally terminate, so need to look it up.

[nctnico]

I am testing the GAHBCFG method (setting bit 0 to 0 - which I earlier abandoned because of occassional errors) and it appears to work identically in the 99.9999% case

It thus seems to be the same as the GINTMSK method, where I was loading 0 into all the bits, to mask all the interrupts.

This is what I would expect, except the RM contains dark warnings about GAHBCFG as posted above.

In fact, last hour of testing, and doing the device removal ultra diligently, I can't break it, over quite a few MB of files written over USB.

The downside of this int disable method is that it also clobbers the USB VCP used for debugs. That isn't important, and I found that if after each internal flash page write (20ms) I have a delay of around 12ms+, the VCP works error-free.

A better way seems to be to get the USB ISR to return an error code (based on an "internal flash in use" flag being set) and then the USB Host should retry. I posted that further back above. It is some SCSI error code. This is the USB ISR:

Code: [Select]

/**
  * @brief  .
  * @param  lun: .
  * @retval USBD_OK if all operations are OK else USBD_FAIL
  *
  */
int8_t STORAGE_Write_FS(uint8_t lun, uint8_t *buf, uint32_t blk_addr, uint16_t blk_len)
{

if (FatFS_filesystem_mounted) return USBD_BUSY;
WritePage(buf, blk_len * STORAGE_BLK_SIZ, blk_addr * STORAGE_BLK_SIZ);
return (USBD_OK);
 
}
I had not noticed the bit which returns USBD_BUSY and that is probably the key.

This function seems like the perfect place to put the data into a FreeRTOS queue and process it in user space (using regular SPI mutex to lock the SPI interface).

[peter-h]

It should not be necessary; the "proper way" is to return BUSY so the USB Host retries. That AIUI is how USB (a master/slave system) is supposed to work.

No matter how one shakes this, the USB host will be trying to send data much faster than it can be written into flash, so all one can do is offload just the one sector, and after that one has to do "something", which will presumably be USBD_OK, delayed by exactly the same amount (18ms) as the USBD_OK delayed by disabling USB interrupts. So as I see it, the main benefit of offloading the write to an RTOS task is to enable all the other USB functions to work, but they "work" anyway for the same reasons provided one doesn't go too crazy with how long ints are disabled for. I found, experimentally, that USB VCP works if there is a >12ms gap between flash writes, and requiring that isn't a problem.

[bson]

The NVIC supports interrupt priority levels (IPL).  You can make USB interrupts lower priority so they won't pre-empt more important work.  For USB I prefer to make them signal a condition variable which wakes a service thread, then handle all the servicing there.  (Same as with ethernet or ESP32 modules.) There are some hard time limits for e.g. handling suspend and resume, but they're pretty lax and these processors are fast, even at relatively low clocks like 48MHz or whatever.

[SiliconWizard]

The NVIC supports interrupt priority levels (IPL).  You can make USB interrupts lower priority so they won't pre-empt more important work.

Sure, but to be fair, that would be no different from just disabling the USB interrupts while accessing the Flash for other purposes.

Ideally, flash access would be handled at a single point, called either from the USB "driver" or the rest of the code, so you can more easily control shared access. That's the preferred way of implementing shared peripheral resources IMHO. Not sure what the OP uses for USB, but that may imply significant changes in third-party code, something that isn't always pretty to do.

[peter-h]

I am using some ST code for USB. Being ST, it is ultra messy, but there isn't a whole lot of it. It is a pile of tables, tables of function codes, all kinds of crap. Plus a bug which has been there for years, which we fixed today and which many others must have fixed before but never told anybody.

But, like I said just above, no matter how much you pipeline the flash access, you still have to face the day of reckoning eventually (unless you have enough RAM to store the entire file being written, or you are writing to a RAM disk) and the apparently traditional solution, returning USBD_BUSY while the flash is unavailable (used by another thread, or simply programming itself), works on other targets, but not on mine, and I have only got one life (still working on that one)

Disabling ints is now rock solid. I just had to stick a 20ms delay at the end of the flash write (making it 40ms) but nobody will notice this. One day I can revisit this. I doubt they will have the will though, and why break a working system to make it "classically correct". I've just spent an hour documenting today's work.

[SiliconWizard]

As long as the USB interrupts are not blocked for longer than the allowed USB request time-out, there's no reason you should have a problem.
Yes this is not really pretty, but I understand the pain of having to use a not-so-good third-party library.

Alternatively, you could consider TinyUSB - which I find pretty good so far. The STM32F4 is supported. I understand that you won't switch now, but maybe for a future design. And it would make porting to a different MCU vendor a lot easier too. TinyUSB supports DFU, mass storage and a number of other USB classes.

https://github.com/hathach/tinyusb

[peter-h]

What sort of max latency is likely to be allowed? I did a google but didn't find much. This
https://www.keil.com/pack/doc/mw/USB/html/_u_s_b__interrupt__transfers.html
tells me that USB FS is likely to be polling at 1kHz but that's a different question.
A lot of hits suggest the default timeout is 5 seconds!

That Tiny USB looks good. We are using MSC and CDC only. And it all seems to work ok - until I got to this "shared flash" business and started testing it rigorously.

What always worries me with these things is how much of these open source projects has been tested. People using them for hobbies don't have an incentive to do lots of testing, and people who use them for a product are highly unlikely to report issues they fixed because

a) commercial use may not be allowed
b) the fixes were done in company time and are the property of the company
c) you don't want to be helping your competitors
d) you don't want your boss to find out that you lifted your code from some website

Hence there is a huge amount of code online which has bugs bad enough to make it useless but "somehow" nobody noticed

[voltsandjolts]

As long as the USB interrupts are not blocked for longer than the allowed USB request time-out, there's no reason you should have a problem.

Can you explain or provide references for this statement?

The USB device needs to be ready to handle setup requests or data requests (even if just to NAK them) which can be issued by the host at any time.
Without an immediate response, the USB bus is in an error state and you are depending on the grace period provided by the host driver before being disconnected.
Also SOF interrupt (1ms intervals on FS) might be needed in your system.

[mikeselectricstuff]

You don't say what your product requirements are, but for something like a datalogger, might it be acceptable to simply never access flash from your application while the USB is plugged in?

[Doctorandus_P]

This is the problem:

The problem is that the USB ISR calls the SPI2 code which then talks to the serial flash - all inside the ISR.

In general, lengthy operations should never be done in ISR's.
Normally your ISR flags that something needs to happen, and then when your application has some time it will do whatever that needs to happen, but the ISR has long been exited.
These days with microcontrollers with multiple ISR levels it's maybe less of an issue, but if you only have one ISR level, then it blocks all others. I usually keep ISR's to a few hundred instructions, which means other ISR's never have to wait long before they can get serviced.

The complexity of your application is probably better off with a small RTOS. It probably gen be done without,but an RTOS makes it easier (and therefore quicker (in programmer time), more robust and better maintainable) to organize the scheduling of different parts of the program.

[wek]

As long as the USB interrupts are not blocked for longer than the allowed USB request time-out, there's no reason you should have a problem.

The USB device needs to be ready to handle setup requests or data requests (even if just to NAK them) which can be issued by the host at any time.

Please point me to an USB device implementation which won't NAK a busy endpoint in hardware. In other words, you don't need software/interrupt handling for the NAKs alone.

Nonetheless, SETUP must not be NAK-ed according to usb2.0 8.4.6.4, so that limits the maximum USB service latency to roughly 3 frames (SETUP is retried 3 times, I am lazy to find the verse) i.e. 3ms.

JW

[voltsandjolts]

Please point me to an USB device implementation which won't NAK a busy endpoint in hardware. In other words, you don't need software/interrupt handling for the NAKs alone.

Yes, of course. But you have to tell the hardware to NAK automatically by some bit setting.
The OP is just disabling USB interrupts at some random time, without care of any ongoing transaction or NAK hardware status.

[wek]

Please point me to an USB device implementation which won't NAK a busy endpoint in hardware. In other words, you don't need software/interrupt handling for the NAKs alone.

Yes, of course. But you have to tell the hardware to NAK automatically by some bit setting.

I don't have a very wide knowledge about all the USB device incarnations out there, but I'd be very surprised if any of them would not start NAKing automatically after transmitting/receiving a single packet (or FIFO exhaustion), until further software handling.

Note, that I never said it's a good thing to indiscriminately stop the USB interrupts, see also my comments in the thread on ST"s forum Peter linked to above.

JW

[voltsandjolts]

but I'd be very surprised if any of them would not start NAKing automatically after transmitting/receiving a single packet (or FIFO exhaustion), until further software handling.

OK, I concede and concur. You must set a bit to not NAK a data transfer, but that bit is then cleared after a transfer by hardware so the default is to NAK (and then you are just subject to the timeout set by the host api call).

[peter-h]

Thank you all for your input.

Some random replies:

Evidently, on Windows at least, disabling the USB global interrupt doesn't break anything - for the current flash write period which is about 15-17ms (writing 512 bytes at a time, and at a measured 33kbytes/sec). So it is certainly not the case that after 3ms it bombs out.

I am using FreeRTOS, and apart from init in main() etc, almost everything runs under that. I am fairly familiar with it, and yes it's a wonderful way to write software. In fact I wrote my own RTOS, from scratch, for a Z180 c. 1986 and then a better one for a Z280 in 1988 which ran a pretty complex datacomms product. Software rapidly gets truly horrible without an RTOS.

USB is set up as an RTOS task and then runs mostly under interrupts, which appears to be ST's intention given that they supplied the code IMHO the only point of argument here is that the flash write is also done inside the ISR, but (as I posted above) if one wasn't doing that, say one was farming out the flash write to a one-shot RTOS task, then one needs to get that RTOS task to return a suitable USB response (instead of the ISR doing it) and return that response only when the flash write is finished.

Re disabling flash access when a USB cable is inserted, that's a bit drastic This product one doesn't need super performance, and one could never get "performance" anyway with a 15-17ms flash write time (which translates to a totally crappy but in this case perfectly adequate 33kbytes/sec write speed, and the 21mbps SPI speed limits the read speed to a measured ~1.5mbyte/sec).

[wek]

Evidently, on Windows at least, disabling the USB global interrupt doesn't break anything

I personally try to avoid evidence-based programming and the "it works for me" paradigm, but I understand the tradeoff between expense and gain.

So it is certainly not the case that after 3ms it bombs out.

That's 3ms after host attempts to send a SETUP. That happens only during enumeration (which is not the case here) and when involved protocols need to transfer some control information (e.g. in MSC/BOT, Reset Recovery implies Clear Feature to be sent to the device - but Reset Recovery means host has already decided that transfer is over). Host of course may decide to send a SETUP to the control endpoint for whatever reason whenever it wants (e.g. for some obscure power-saving measure), but in practice this will be rare.

There may be other reasons for timeouts, e.g. would you have isochronous endpoints you'd need to handle them timely otherwise problems occur quite rapidly.

JW

[peter-h]

I am happy to try to do this differently, and for sure the "correct" solution ought to be to return some sort of "try again in a while" status because that must be what all the $3 USB-serial converters must be doing

Just making the ISR very short is trivial: copy the 512 byte sector into a buffer, and return USBD_OK. An RTOS task can then write the flash. The "slight problem" is that the host will be back in well under a millisecond, trying to write the next one

[nctnico]

I am happy to try to do this differently, and for sure the "correct" solution ought to be to return some sort of "try again in a while" status because that must be what all the $3 USB-serial converters must be doing

Just making the ISR very short is trivial: copy the 512 byte sector into a buffer, and return USBD_OK. An RTOS task can then write the flash. The "slight problem" is that the host will be back in well under a millisecond, trying to write the next one

I think you can check the previous buffer hasn't been consumed by the OS yet and thus return BUSY. I assume that the other end will try to write the block again later.

[wek]

I am happy to try to do this differently, and for sure the "correct" solution ought to be to return some sort of "try again in a while" status because that must be what all the $3 USB-serial converters must be doing

How many of those USB-serial converters are MSC?

As discussed, there are "wait" opportunities at different layers of the protocol stack, all of them having merits and drawbacks. Plus the badly defined nature of whole USB "norms system" and the desperate need to conform to whatever quirks hosts out there may have.

Just making the ISR very short is trivial: copy the 512 byte sector into a buffer, and return USBD_OK. An RTOS task can then write the flash. The "slight problem" is that the host will be back in well under a millisecond, trying to write the next one

As I've said, I would handle things at the endpoint level. While host may still decide to be impatient and time out, the general wait mechanism in USB at data packet level is the NAK mechanism (see USB 2.0 5.3.2, An endpoint can inform the host that it is busy by responding with NAK. NAKs are not used as a retire condition for returning an IRP to a software client. Any number of NAKs can be encountered during the processing of a given IRP. A NAK response to a transaction does not constitute an error and is not counted as one of the three errors described above.) This appears to be widely accepted by hosts and drivers therein. It means, that you don't re-enable an Out endpoint for Rx (or don't write data for Tx to In endpoint and enable it) until whatever transaction with the slow memory is finished, using whatever mechanism (e.g. buffering and leaving the USB interrupt, the transaction to be finished by "main" or RTOS later); attempts of host to send more data to Out endpoint or to poll In endpoint are NAKed by the USB hardware of device.

Yes, this is not something you can click in Cube or fix by adding a few selected lines. Cube, as any library, inevitably caters only for a miniscule fraction of all possible usage cases, arguably the "most common" ones. OTOH, Cube is not the only option. There are open-source and third-party solutions out there, and there are consultants available.

JW

[peter-h]

"I think you can check the previous buffer hasn't been consumed by the OS yet and thus return BUSY. I assume that the other end will try to write the block again later."

For some reason that doesn't work. The BUSY return screws up USB. We tried that already. As far as the host is concerned, it is exactly the same situation as not having buffering and returning BUSY anytime the flash is not available; the only difference is that there is a delay of one sector. So if the host was trying to write only one sector (and the flash was otherwise busy) it would never get a BUSY, but if it was trying to write 2+ sectors then we are back to the same situation... AFAICT.

From the POV of our end, the difference is that the ISR either takes 18ms, or almost nothing, but either one works ok. I haven't looked at the USB interrupt priority but even if a USB write killed the whole RTOS for 18ms at a time (it certainly does kill timer interrupts spectacularly for the 18ms - presumably USB int has a higher priority) it would not matter because this product will not be doing anything critical while somebody is trying to dump a file onto the USB drive (which will be either a little config file, or a firmware update image).

Basically the concept of "real time" goes out of the window if some process wants to access the flash quickly (even just for reading) IF you are having something writing the flash and blocking it for 18ms at a time

WEK - yes the USB-serial converters use a different device type; you are right. We will see if we can work out what you are suggesting.

[Deni]

Plus a bug which has been there for years, which we fixed today and which many others must have fixed before but never told anybody.

So, can YOU tell us what it was?

[peter-h]

Yes; the details are here
https://community.st.com/s/question/0D53W00001Ie4WcSAJ/32f417-how-to-block-usbfs-isr-from-a-shared-resource-serial-flash-on-spi

The  USB code incorrect passes an error status back to the SCSI layer above it. Only a "no error" status is correctly handled.

EDIT: For those interested, this thread sort of continues here
https://www.eevblog.com/forum/microcontrollers/32f417-usb-fs-(not-hs)-and-dma/msg3958874/#msg3958874

The issue was adequately solved by disabling interrupts around the FLASH reads and writes, including the internal portion of the FLASH programming cycle. That takes a total of about 17ms but it doesn't break the USB block device.