Building the Magus firmware

mcc · September 22, 2020, 3:49pm

I’ll give that a try as soon as I’ve got the new bootloader installed and the white card re-inserted in my Magus! I might not be able to test Magus stuff until the weekend tho.

antisvin · September 22, 2020, 4:33pm

V21.0 means we’re not gonna see the release until the end of this cursed year?

antisvin · September 22, 2020, 4:42pm

Errr probably not quite correct in case MidiBoot. It has usbd-fs/usbd-hs dirs, but CubeMX would create the same files for whatever is configured for USB device in Inc/Src directories. I think that Cube’s files should be deleted, because we choose correct files from one of those 2 directories based on config header file.

There’s also usbd_audio_if.c/.h files that should be deleted (for Magus and probably MidiBoot with new USB stack as well).

Other than that this seems to be correct as long as you’re not trying to change any peripherals configuration. You may think that this is not something that you would do, but remember that Magus has expander ports for UART (requiring different settings for serial MIDI or digitalbus) and second one for I2C (again, could use different timings setting for protocol clock)

mcc · September 22, 2020, 4:55pm

Okay. But you’re not saying I shouldn’t revert things-- you’re saying I should also potentially delete some of the loose generated files?

Is it the case I must delete those extra generated files for correct code, or just that I have the option of deleting them?

antisvin · September 22, 2020, 5:18pm

Yeah, I think it would have everything necessary for USB, so extra files should be deleted and changed files reverted.

For non-USB files, reverting something may be required or not depending on reasons why it happens. In most cases cube would be overwriting files that were edited manually - this should be rolled back. There’s a possibility that there were some changes in ST-provided files which should be merged upstream, but this is much less likely.

antisvin · September 23, 2020, 9:19pm

Martin, I’ve noticed a minor regression in rollback of USB midi code in bootloader

New version:

github.com

pingdynasty/OpenWare/blob/develop/MidiBoot/Src/usb_device.c#L74

    
      
          #else

          #error Invalid USB configuration

          #endif

            {

              Error_Handler();

            }

            if (USBD_RegisterClass(&USBD_HANDLE, &USBD_AUDIO) != USBD_OK)

            {

              Error_Handler();

            }

            if (USBD_AUDIO_RegisterInterface(&USBD_HANDLE, NULL) != USBD_OK)

            {

              Error_Handler();

            }

            if (USBD_Start(&USBD_HANDLE) != USBD_OK)

            {

              Error_Handler();

            }

          }

          /**

            * @}

Old:

github.com

pingdynasty/OpenWare/blob/master/MidiBoot/Src/usb_device.c#L70

    
      
          #if defined USE_USBD_FS

            if (USBD_Init(&USBD_HANDLE, &FS_Desc, DEVICE_FS) != USBD_OK)

          #elif defined USE_USBD_HS

            if (USBD_Init(&USBD_HANDLE, &HS_Desc, DEVICE_HS) != USBD_OK)

          #else

          #error Invalid USB configuration

          #endif

            {

              Error_Handler();

            }

            if (USBD_RegisterClass(&USBD_HANDLE, &USBD_AUDIO) != USBD_OK)

            {

              Error_Handler();

            }

            if (USBD_AUDIO_RegisterInterface(&USBD_HANDLE, NULL) != USBD_OK)

            {

              Error_Handler();

            }

            if (USBD_Start(&USBD_HANDLE) != USBD_OK)

            {

              Error_Handler();

But unlike the audio USB driver, MIDI USB requires non-NULL callback pointer otherwise it returns failure:

github.com

pingdynasty/OpenWare/blob/master/Source/usbd_midi.c#L548-L552

    
      
          #include "device.h"

          #include "midi.h"

          #include "usbd_midi.h"

          #include "usbd_desc.h"

          #include "usbd_ctlreq.h"

          

          static uint8_t  USBD_Midi_Init (USBD_HandleTypeDef *pdev, 

                                         uint8_t cfgidx);

          static uint8_t  USBD_Midi_DeInit (USBD_HandleTypeDef *pdev, 

                                           uint8_t cfgidx);

          static uint8_t  USBD_Midi_Setup (USBD_HandleTypeDef *pdev, 

                                          USBD_SetupReqTypedef *req);

          static uint8_t  *USBD_Midi_GetCfgDesc (uint16_t *length);

          static uint8_t  *USBD_Midi_GetDeviceQualifierDesc (uint16_t *length);

          static uint8_t  USBD_Midi_DataIn (USBD_HandleTypeDef *pdev, uint8_t epnum);

          static uint8_t  USBD_Midi_DataOut (USBD_HandleTypeDef *pdev, uint8_t epnum);

          static uint8_t  USBD_Midi_EP0_RxReady (USBD_HandleTypeDef *pdev);

          static uint8_t  USBD_Midi_EP0_TxReady (USBD_HandleTypeDef *pdev);

          static uint8_t  USBD_Midi_SOF (USBD_HandleTypeDef *pdev);

          static uint8_t  USBD_Midi_IsoINIncomplete (USBD_HandleTypeDef *pdev, uint8_t epnum);

This file has been truncated. show original

So either it should be passed like before or USBD_Midi_RegisterInterface should ignore it if can be used without a CB.

mcc · September 24, 2020, 12:02am

I have reverted all files back to develop + antisvin:update-parameters-broken (except for the files that enable IWDG). I am still not sure how to proceed. I am still getting the usbd_def.h not found error.

@antisvin said above the usbd_def.h header is supposed to be at Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_def.h. It is not there, there is no Middlewares/ or MidiBoot/Middlewares directory even. There is a ./Magus/Middlewares/ST/STM32_USB_Device_Library/Core/Inc/usbd_def.h.

If I look at the STM32CubeMX manual, it gives these instructions for installing a Middleware:

From the Mode panel in the Pinout & Configuration view, configure the RCC
peripheral by enabling the external clocks, master output clocks, audio input clocks
(when relevant for your application). This automatically displays more options on the
Clock configuration view (see Figure 96). Then, select the features (peripherals,
middlewares) and their operating modes relevant to the application.

I think this is the mode panel in question:

It looks?? like the reason there is no usbd is because USB_DEVICE middleware is “disabled”. But I do not know whether to select HS or FS for the Magus, and I do not know which of the several options to select.

So, @mars, I have 4 questions…

Is there some preconfigured build file for MidiBoot for the Magus I have missed? Or do I need to open MidiBoot.ioc and select a USB_DEVICE middleware by hand?
Which USB_DEVICE middleware do I select, if I am building for the Magus?
Are there any other Magus-specific middlewares (or other one-off options) I need to activate in STM32CubeMX before building MidiBoot?
Once I get MidiBoot in I would be happy to test your usbh irq PR. However, what about @antisvin’s issue above? Should I run the usbh irq PR from github directly, or should I wait for the issue antisvin raises to be fixed?

Thanks.

antisvin · September 28, 2020, 10:57am

Since Martin didn’t answer this:

use audio device class
pretty sure that you should rollback/delete anything that Cube generates outside of Middlewares/Drivers directories
current USB registration code would fail at runtime during device initialization, older version should be used. I’ve confirmed that it happens on another device with forked code.

antisvin · September 28, 2020, 11:02am

Actually, I don’t think it’s even necessary to build version of MidiBoot from develop branch. The USB code that needs testing is in Magus FW, not the bootloader.

mcc · September 28, 2020, 3:18pm

Thanks, good to know. But master midiboot at least does the IWDG correct (as long as USE_IWDG is set)? That was all I wanted for midiboot.

One more fiddly question. Do MidiBoot and Magus have to “agree” in terms of whether they’re using full speed vs high speed? That is if I set up MidiBoot with USB high speed, and then later I downgrade Magus to USB full speed, will that confuse anything?

antisvin · September 28, 2020, 4:01pm

Yes, IWDG code is present in master branch and I’ve used it before.

Firmware resets all peripherals when booting, so USB will be reset and configured from scratch after application jump. But it looks like both “full speed” and “high speed” peripherals are configured to use full speed - Martin explained a few pages earlier that high speed USB would required external PHY (additional hardware IC)

mars · September 29, 2020, 12:52pm

Yes in the HAL they refer to OTG_FS/HS to distinguish between the two peripherals, even if they are both configured as FS.

mars · September 29, 2020, 1:01pm

It’s happening! I’ve created a repo for the libs: GitHub - pingdynasty/OpenWareLibraries: Supporting code libraries for OpenWare firmware
and a branch with the submodule and build changes: GitHub - pingdynasty/OpenWare at feature/submodule-libs

Now I just need to go through the different projects and fix any build problems… Wish me luck!

mars · October 1, 2020, 8:33am

Magus and the Magus bootloader builds now.

Check out the branch, do a submodule init and update, and build as before. But pls make sure to do a make clean when you switch from one project to another, ie from bootloader to Magus!

git checkout feature/submodule-libs
git submodule init
git submodule update
make clean
cd MidiBoot && make PLATFORM=Magus all
cd Magus && make clean all

@mcc if you want to try my usbh changes then also do

git merge feature/usbh-irq

antisvin · September 29, 2020, 1:30pm

Sweet! In such case, I’ll add PR for H7 support. I think it should only need ST Cube package.

Also, once this is stable, we could have CI builds for new commits (at least for develop and master branches). Some of existing services offer this for free for open source software, so maybe we can make sure that new commits / PRs at least will build on all devices.

mars · September 29, 2020, 1:32pm

go for it!

with gcc cross-compiler?

antisvin · September 29, 2020, 1:49pm

Yeah, I suppose that there would be a way to install ARM-GCC toolchain. I expect that most of those services would work with makefiles (or docker images), so a compiler would be just another dependency for build pipeline. But I can do some research, maybe it’s too optimistic to expect that as part of their free offers.

antisvin · September 30, 2020, 8:17pm

Just noticed recent commit in development branch. I’m not sure if using MX_USB_Host_Process in loop() function would be a good idea. It runs once per 20ms on devices with screen, so at least screen update code should be changed to use shorter delay and just skip updates until 20ms have passed.

Also, since we’re using RTOS, maybe we should use RTOS support for USB (using “#define USBH_USE_OS”). This would basically call the same function in a separate OS thread , but that thread would be blocked if no new data is available. Could such trade of memory for CPU make some sense?

mars · September 30, 2020, 9:12pm

We’re not using pre-emptive task scheduling though, to avoid overheads. It’s part of the reason we’re getting so much audio performance out of relatively small microcontrollers - something like 98-99% of available clock cycles can be spent exclusively on audio processing.

If we can handle data send and receive in IRQs (as begun with the new usbh-irq branch) then the MX_USB_Host_Process function probably doesn’t have to do anything very time critical. I still haven’t quite got my head around how the library works though. It seems incredibly convoluted, with different attempts at abstraction competing with each other (callbacks, weak declarations, function pointers…). Just like most ST code, in other words!

antisvin · September 30, 2020, 9:44pm

Yeah, I know that cooperative scheduling is used, I’d expect this thread to run when audio task is idle.

Btw, I’m not sure that preemptive MT would make that much problems for performance - it still respects OS task priorities. And IIRC, in FreeRTOS even preemption for the same priority tasks will only happen if you enable time slicing.

My main point was that we’re running owl_loop with 20ms on devices with screen, this looks like a potential source of latency for everything that happens in loop() function that calls it. And it would only be effecting some devices, which can make harder tracking this sort of issues. I’m not sure if among MIDI transmit, USB host processing and digitalbus IO nothing is sensitive to 20ms delay - there are buffers involved, it may be possible to overflow them. It probably makes more sense just to test this and make PR, the change would be fairly trivial, so I’ll do it later.