search

ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.97k forks source link

USBDevice not working on STM32H743, debug and trace problems #13641

Closed JojoS62 closed 3 years ago

JojoS62 commented 4 years ago

Description of defect

I tried to activate USBDevice on a custom target with Devebox H743 board. The USB code compiles for this target, but the host does not get a valid USB device information. The same issue is observed on a Nucleo H743ZI2 board and reported in the forum: https://forums.mbed.com/t/mbed-nucleo-h7743zi2-doesnt-support-usbdevice/8241/7 So it does not look target hardware specific. Tests with different sources for USB clock (HSI48, PLL3Q) were not successfull. So I tried to set breakpoints in the HAL_USB callbacks. These breakpoints were not hit. Only when I set a breakpoint in USBPhyHw::_usbisr(void) the target stops and after continue, also the bp in the SOF callback is reached. Next step was to add tr_info() calls and enable mbed_trace. This did not work also. An initial printf("hello world\n") is printed, followed by a 0xff hex.

A test with the same program and modified USBPhy_STM32.cpp on a F401 MCU is working as expected. The USBMidi is recognized by the windows host and midi output from the sample code is generated. The trace outputs from the callbacks are pinted as well.

Don't know it it maybe related to this H743 issue: https://github.com/ARMmbed/mbed-os/issues/11422

Target(s) affected by this defect ?

STM32H743xI NUCLEO_H743ZI2

Toolchain(s) (name and version) displaying this defect ?

GNU Tools for Arm Embedded Processors 9-2019-q4-major) 9.2.1 20191025

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.3.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli

How is this defect reproduced ?

#include "mbed.h"
#include "USBMIDI.h"
#include "mbed_trace.h"

DigitalOut led1(LED1);

int main() {
    mbed_trace_init();
    sleep_manager_lock_deep_sleep();
    printf("hello world\n");

    USBMIDI midi(false);
    midi.connect();

    while (1) {
        for(int i=48; i<83; i++) {     // send some messages!
            led1 = !led1;
            if (midi.ready())
                midi.write(MIDIMessage::NoteOn(i));
            ThisThread::sleep_for(250ms);
            if (midi.ready())
                midi.write(MIDIMessage::NoteOff(i));
            ThisThread::sleep_for(500ms);
        }
   }
}

An updated USBPHy_STM32.cpp with test traces can be found in this branch: https://github.com/JojoS62/mbed-os/tree/test-USB-H743

ciarmcom commented 4 years ago

@JojoS62 thank you for raising this issue.Please take a look at the following comments:

We cannot automatically identify a release based on the version of Mbed OS that you have provided. Please provide either a single valid sha of the form #abcde12 or #3b8265d70af32261311a06e423ca33434d8d80de or a single valid release tag of the form mbed-os-x.y.z . E.g. 'mbed-os-99.99.99' signifies master branch and is not a unique single snapshot. NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'.This indicates to us that at least all the fields have been considered. Please update the issue header with the missing information, the issue will not be mirroredto our internal defect tracking system or investigated until this has been fully resolved.

JojoS62 commented 4 years ago

I'm a step further, Debugging and tracing failed because PA9/PA10 was used for serial output and interferred with VBus/ID detection. These functions are not enabled in USBPhy init, but the AF functions in the PinMap_USB_FS are set. But USB is still not working after this fix. Did the NUCLEO_H743ZI2 pass the USB tests? The log looks now like:

hello world
[INFO][main]: main starting
[INFO][usb-stm32]: USBPhyHw::connect()
[INFO][usb-stm32]: USBPhyHw::process()
[INFO][usb-stm32]: HAL_PCD_SuspendCallback()
[INFO][usb-stm32]: USBPhyHw::process()
[INFO][usb-stm32]: HAL_PCD_ResumeCallback()
[INFO][main]: main looping
[INFO][main]: main looping
[INFO][usb-stm32]: USBPhyHw::process()
[INFO][usb-stm32]: HAL_PCD_SuspendCallback()
[INFO][main]: main looping
[INFO][main]: main looping
[INFO][main]: main looping
[INFO][main]: main looping

Excluding PA9/PA10 from PinMap_USB_FS makes no difference.

I tried again to use HSI48 as clock source, but in this case USB_CoreReset() runs into timeout when waiting for reset done. Increasing the timeout up to 10 s did not help. It looks also that CRS must be enabled for HSI48 USB clock, but still timeout in USB_CoreReset(). With PLL3Q as USB clock the USB is starting, but is going into PCD_Suspend state.

JojoS62 commented 4 years ago

After lots of testing, I found a sensible line in the HAL code in https://github.com/ARMmbed/mbed-os/blob/66423948e0d8ee8331b700fead20fac604e2a9a1/targets/TARGET_STM/TARGET_STM32H7/STM32Cube_FW/STM32H7xx_HAL_Driver/stm32h7xx_ll_usb.c#L1078-L1086

When stepped through this code, I got more communication. I increased the HAL_Delay from 3 to 200ms and now I'm getting the follwing trace:

Test STM32H743VI USB
[INFO][main]: main starting
[INFO][usb-stm32]: USBPhyHw::init() done.
[INFO][usb-stm32]: USBPhyHw::connect()
[INFO][usb-stm32]: HAL_PCD_ResetCallback()
[INFO][usb-stm32]: USBPhyHw::endpoint_add()
[INFO][usb-stm32]: USBPhyHw::endpoint_add()
[INFO][usb-stm32]: HAL_PCD_SOFCallback()
[INFO][usb-stm32]: HAL_PCD_SetupStageCallback()
[INFO][usb-stm32]: USBPhyHw::ep0_setup_read_result()
dataTransferDirection: 1
Type: 0
Recipient: 0
bRequest: 6
wValue: 256
wIndex: 0
wLength: 64
[INFO][usb-stm32]: HAL_PCD_SOFCallback()
get descr: type: 1
device descr
[INFO][usb-stm32]: USBPhyHw::ep0_write()
[INFO][usb-stm32]: USBPhyHw::endpoint_write()
[INFO][usb-stm32]: USBPhyHw::ep0_read()
[INFO][usb-stm32]: HAL_PCD_SOFCallback()
[INFO][usb-stm32]: HAL_PCD_DataInStageCallback()
ep0_in
[INFO][usb-stm32]: HAL_PCD_DataOutStageCallback()
[INFO][usb-stm32]: HAL_PCD_ResetCallback()
[INFO][usb-stm32]: USBPhyHw::endpoint_add()
[INFO][usb-stm32]: USBPhyHw::endpoint_add()
[INFO][usb-stm32]: HAL_PCD_SuspendCallback()

The delay after clearing the SDIS bit must be >=180 ms, strange long.

But the device is still not recognized correctly.

My MCU is a H743VI with Revision "V". I 've got same results on a NUCLEO_H743ZI with an older Rev "Y" MCU. A difference is, that on Rev "Y" I can use HSI48 as clock source for USB. On Rev V, HSI48 and PLL1Q do not work, USB is hanging in USB_CoreReset on wait for while((USBx->GRSTCTL & USB_OTG_GRSTCTL_CSRST) == USB_OTG_GRSTCTL_CSRST).

ciarmcom commented 4 years ago

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers. Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2305

JojoS62 commented 3 years ago

I've generated a simple USB program in STM32CubeIDE and it works fine. Initialisation is almost the same, but I couldn't find the difference that makes it working.

JojoS62 commented 3 years ago

Its driving me nuts, it seems to be related to the Mbed ticks. With MCU_STM32, lse_enable=1 and LP_TICKER is used. Now USB enumeration works, but only for a short time, until a sleep is called. This sleep is running forever, the lp ticker is fired about 3 times and then no more. When I remove LP_TICKER, then the USB enumeration fails, the PCD seems to hang and no USB Ints are fired. But the sleep_for() is working. @jeromecoutant do you have any idea? I tried also the HAL update, there are some differences in SystemInit but it does not help either.

jeromecoutant commented 3 years ago

You could try to disable deepsleep to check if issue comes when MCU goes in STOP MODE 2 ? (use debug profile for ex)

JojoS62 commented 3 years ago

thanks, I had added a DeepSleepLock already. But it seems to have no effect, It looks also like the deepsleep (deep power down) is disabled when a debugger is attached.

JojoS62 commented 3 years ago

But this seems to be the origin of the problem. Now I have removed "SLEEP" from the target and USB and timer is running.

JojoS62 commented 3 years ago

I have checked again lp_ticker. The configuration with LSE seems to be ok, but the wakeup interrupts are fired very slow and randomly, in a range of about 30 ... 150 s. Then I routed LSE to MCO1 and I have seen no clock here, so that was a hardware problem, something wrong with the 32k xtal.

Then I have changed lp_ticker to use LSI and now the MCU wakes up as intended. But with this config, USB does not work, ints are not fired. USB is only working with "target.device_has_remove": ["SLEEP"], what I still don't understand. Shouldn't be a call to sleep_manager_unlock_deep_sleep(); enough?

JojoS62 commented 3 years ago

I have checked again lp_ticker. The configuration with LSE seems to be ok, but the wakeup interrupts are fired very slow and randomly, in a range of about 30 ... 150 s. Then I routed LSE to MCO1 and I have seen no clock here, so that was a hardware problem, something wrong with the 32k xtal.

Then I have changed lp_ticker to use LSI and now the MCU wakes up as intended. But with this config, USB does not work, ints are not fired. USB is only working with "target.device_has_remove": ["SLEEP"] also "target.macros_remove": ["MBED_TICKLESS"] does not help. 

Test STM32H743VI USB
[INFO][main]: main starting
LOCK: mbed-os\platform\source\DeepSleepLock.cpp, ln: 29, lock count: 1

Test STM32H743VI USB
[INFO][main]: main starting
LOCK: mbed-os\platform\source\SysTimer.cpp, ln: 112, lock count: 2
Sleep locks held:
[id: mbed-os\platfo, count: 2]
UNLOCK: mbed-os\platform\source\SysTimer.cpp, ln: 151, lock count: 1
LOCK: mbed-os\targets\TARGET_STM\USBPhy_STM32.cpp, ln: 192, lock count: 2
LOCK: mbed-os\platform\source\SysTimer.cpp, ln: 112, lock count: 3
Sleep locks held:
[id: mbed-os\platfo, count: 2]
[id: mbed-os\target, count: 1]
UNLOCK: mbed-os\platform\source\SysTimer.cpp, ln: 151, lock count: 2
LOCK: mbed-os\platform\source\SysTimer.cpp, ln: 112, lock count: 3
Sleep locks held:
[id: mbed-os\platfo, count: 2]
[id: mbed-os\target, count: 1]
jeromecoutant commented 3 years ago

Shouldn't be a call to sleep_manager_unlock_deep_sleep(); enough?

Call sleep_manager_lock_deep_sleep(), not unlock ? But it is already done in USBPhyHw::init

JojoS62 commented 3 years ago

I think I've got it, that was really hard. There I found the clue: https://github.com/micropython/micropython/pull/3639#issuecomment-369796668

and adding this to USBPhyHw::init for USE_USB_OTG_FS is working now:

    __HAL_RCC_USB_OTG_FS_CLK_ENABLE();
    __HAL_RCC_USB2_OTG_FS_CLK_SLEEP_ENABLE();
    __HAL_RCC_USB2_OTG_FS_ULPI_CLK_SLEEP_DISABLE();

Its strange that ULPI clock must be disabled, but without it is also not working.

I found a comment above the HAL macros

JojoS62 commented 3 years ago

Call sleep_manager_lock_deep_sleep(), not unlock ?

yes, I have used lock, not unlock. That was a copy error.

jeromecoutant commented 3 years ago

Made some grep... Found __HAL_RCC_USB2_OTG_FS_ULPI_CLK_SLEEP_DISABLE() in https://github.com/STMicroelectronics/STM32CubeH7/blob/master/Projects/STM32H743I-EVAL/Applications/USB_Device/HID_Standalone/Src/usbd_conf.c#L74-L75 .... But not generated by CubeMx...

JojoS62 commented 3 years ago

yes, that works also, only disabling ULPI sleep clock. So the comment about default is clock enabled is true. Maybe this hint about ULPI is somewhere in the reference manual...

I have used a CubeMX project as reference and it worked, but more likely because it does not use WFI for sleeping.

JojoS62 commented 3 years ago

I have tested this fix also for the new target H7A3 from your PR. It works also, but some devices have two USB interfaces and it is not clear for me which to use. Now I do

    #ifdef __HAL_RCC_USB1_OTG_FS_ULPI_CLK_SLEEP_DISABLE
        __HAL_RCC_USB1_OTG_FS_ULPI_CLK_SLEEP_DISABLE();
    #endif
    #ifdef __HAL_RCC_USB2_OTG_FS_ULPI_CLK_SLEEP_DISABLE
        __HAL_RCC_USB2_OTG_FS_ULPI_CLK_SLEEP_DISABLE();
    #endif

There are macros without interface number, but they were moved to the legacy hal folder. I can prepare a PR, but I see there is a chain of PRs dependending on others.

jeromecoutant commented 3 years ago

@JojoS62 Could we close this issue now that #13780 is merged ? Thx

JojoS62 commented 3 years ago

yes, fixed.