qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.01k stars 38.72k forks source link

STM32 with OTG support has differently named USBEndpointConfig struct member #4783

Closed npdawson closed 4 years ago

npdawson commented 5 years ago

Describe the bug STM32 chips with USB OTG support apparently use the OTGv1 HAL code instead of the USBv1 code. In hal_usb_lld.h for OTGv1, the USBEndpointConfig struct has an in_multiplier field:

/**
 * @brief   Determines the space allocated for the TXFIFO as multiples of
 *          the packet size (@p in_maxsize). Note that zero is interpreted
 *          as one for simplicity and robustness.
 */
uint16_t in_multiplier;

instead of USBv1's ep_buffers field:

/**
 * @brief   Reserved field, not currently used.
 * @note    Initialize this field to 1 in order to be forward compatible.
 */
uint16_t ep_buffers;

The problem occurs in tmk_core/protocol/chibios/usb_main.c where the QMK_USB_DRIVER_CONFIG macro defines 3 USBEndpointConfigs, all with .ep_buffers = 2,. When compiling for my STM32F4 Discovery board, I get an error saying that USBEndpointConfig has no member named ep_buffers.

System Information

Additional context

yiancar commented 5 years ago

Could you possibly use the ifdef for stm32f4xx to modify the endpoint defines?

npdawson commented 5 years ago

@yiancar That was my first thought, but would it make more sense to use the same format as used earlier in the file where the member names are implicit?

Like this:

.in_ep_config = { \
  stream##_IN_MODE,        /* Interrupt EP */ \
  NULL,                         /* SETUP packet notification callback */ \
  qmkusbDataTransmitted,                 /* IN notification callback */ \
  NULL,                         /* OUT notification callback */ \
  stream##_EPSIZE,                /* IN maximum packet size */ \
  0,                            /* OUT maximum packet size */ \
  /* The pointer to the states will be filled during initialization */ \
  NULL,             /* IN Endpoint state */ \
  NULL,                         /* OUT endpoint state */ \
  2,                            /* IN multiplier */ \
  NULL                          /* SETUP buffer (not a SETUP endpoint) */ \
}, \

The structure is the same, and the comments earlier in the file already refer to the ep_buffers member as the "in multiplier."

yiancar commented 5 years ago

Ah yes. This seems better! can you confirm it works?

npdawson commented 5 years ago

Yes, it works for my discovery board and compilation is successful for planck rev6.

yiancar commented 5 years ago

@awkannan1 When you can can you also try it with your mcus?

awkannan commented 5 years ago

I will try to check it on my STMF072 discovery and a blue pill this weekend

chipperdoodles commented 5 years ago

I'm trying to compile this for my nucleo stm32f401re, If i set console enable = yes in rules.mk the build fails, not sure if that's just a gimme. It compiles ok with console enable = no. I'm not sure i've configured my build for this mcu right either, if anyone has some examples of a setup for an stm32f4xx I can try testing more!

stale[bot] commented 4 years ago

This issue has been automatically marked as resolved because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs.