Electronics > Microcontrollers

Cleanest way to block 32F417 USB interrupts?

(1/9) > >>

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: ---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;
}

--- End code ---

__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:

--- Quote from: peter-h on January 08, 2022, 11:43:20 am ---The problem is that the USB ISR calls the SPI2 code which then talks to the serial flash - all inside the ISR.

--- End quote ---

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.

Navigation

[0] Message Index

[#] Next page

There was an error while thanking
Thanking...
Go to full version