Fixes #1018 the weak dcd_edpt0_status_complete for Keil Compiler

[XelaRellum]

The Keil compiler seems to have different semantics and the defined function was never called.

The same is probably true for the other weak functions. I can change those too.

[hathach]

Thank you for the Pr. Do you check with official doc from keil for this behavior for declared weak function ? I don't have keil compiler to test with.

[XelaRellum]

Yes and I have tried this in the code, with Keil and with STM32CubeIDE GCC (since I needed this in my project right now)... Am 30. August 2023 05:06:15 MESZ schrieb Ha Thach ***@***.***>:

…

@hathach commented on this pull request. Thank you for the Pr. Do you check with official doc from keil for this behavior for declared weak function ? -- Reply to this email directly or view it on GitHub: #2239 (review) You are receiving this because you authored the thread. Message ID: ***@***.***>

[hathach]

can you please provide link to keil's official doc for the behavior ?

[XelaRellum]

can you please provide link to keil's official doc for the behavior ?

As usual, the documentation is a bit cryptic:

https://developer.arm.com/documentation/101754/0620/armclang-Reference/Compiler-specific-Function--Variable--and-Type-Attributes/--attribute----weak---function-attribute

Also on the same topic:

https://stackoverflow.com/questions/51656838/attribute-weak-and-static-libraries

In my understanding the problem is the difference of the weak declaration vs. the weak definition of the function.

With the strong declaration, but weak definition of a default/empty function the linker will enforce the symbol to exist, so the test always evaluates to true. But a strong definition of a function will override a weak definition.

The use of a weak declaration seems to be the trick part and which doesn't work here. Although the function in dcd_stm32_fsdev.c gets compiled in the same project, no actual code is generated and the symbol is removed. I don't really get why ARMCC does this, but it does.

ST uses weak definitions for their HAL libraries like here:

/**

• @brief SYSTICK callback.
• @RetVal None
  /
  __weak void HAL_SYSTICK_Callback(void)
  {
  / NOTE : This function should not be modified, when the callback is
  needed, the HAL_SYSTICK_Callback could be implemented in the user file
  */
  }

So, if I want my own implementation, I define it in some of my application's files.

Their solution works with KEIL and GCC and IAR and whatnot (all supported compilers of ST code).

[XelaRellum]

Right now I am progressing further with the project and I now realize the scope of the problem is bigger. My first thought was, the function above is the only one that is really needed for an implementation. But now I have come to realize TU_ATTR_WEAK functions are all over the place (well, at least several of them).

So, the right thing to do would be to change the semantics of all the callbacks and provide a default implementation for any callbacks that are optional. The return value of the function should be the indicator if it is a default implementation (like a NULL pointer).

I will happily work on this topic, if you are okay with that. One question: where should all the default implementations go? My first guess would be the module that uses/calls it, if it is called only from one location.

What about functions like board_init_after_tusb? There is no generic module for them? So maybe put those into "tusb.c"?

[hathach]

never mind, I mixed up with docs. Let me try to download keil and test it on my VM. Weak without default is used a lot within repo, don't worry too much about it. If we have to change, we can do it one by one, you can start with only ones that you needed. We can migrate them all later.

What about functions like board_init_after_tusb? There is no generic module for them? So maybe put those into "tusb.c"?

tinysb/src is selfed-contained, it is the stack source and is meant to be used without other code in tinyusb/hw. Don't worry about this, I could do the migrate for these once I confirmed the issue with keil/armclang.

[KlemenDEV]

I can confirm this patch fixes operation on Keil

[KlemenDEV]

I think this also affects other weak callbacks, eg. tud_cdc_line_state_cb or tud_cdc_rx_cb don't work either in Keil if not modified in a similar way as this PR does

[XelaRellum]

I think this also affects other weak callbacks, eg. tud_cdc_line_state_cb or tud_cdc_rx_cb don't work either in Keil if not modified in a similar way as this PR does

Yes true. Didn't want to change stuff that I don't use and thus can't test. But should be straight forward.,

[hathach]

look good, even though there is tons of other weak functions. We can migrate/update them later on so that it would work with keil better.