-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for custom IRQ Handlers #307
base: develop
Are you sure you want to change the base?
Conversation
Try to add the file manually here |
1c4b5ac
to
fa54ac3
Compare
fa54ac3
to
cf153e2
Compare
The forwarder is working (the naming is a bit ugly though). I haven't optimized the call to the ISR yet but as a proof of concept this should do. |
This is required to receive the buildlog changes from previous post-build steps.
See my branch for some additional commits that you may want to cherry pick. You can drop the work-around commit now with lbuild v1.13.0. |
I also added a validator for the Collector.
I don't like the |
…nium/modm into feature/custom-irq-handler
This reverts commit cf153e2.
Okay I cleaned up the generated code and the generator. However as already noticed the call to the ISR is not free anymore and inlining the driver function is not really working. I'm open to suggestions. Also where should we go from here? Should we the add the option to add own calls to the forwarder? |
Hm, The Internet™ says you can with some effort alias to an external function: https://stackoverflow.com/a/58119272
Own calls? Or do you mean multiple calls per hook? Status right now:
Show: You can optimize for |hooks| = 1 and implement |hooks| >= 2. (14 points) |
Phew that looks kinda dirty to be honest. Does lbuild support to add a step between compiling and building? Plus we had to determine where to find the ISR symbols so we can do the objcopy. |
I don't think this is a solution. It kinda sucks that the most common use-case comes with overhead, but on the other hand the overhead is like 4(?) instructions or so, if you really need that kinda performance, you're not gonna use the built-in interrupt handlers from modm and probably need to write assembler anyways. I would leave it like this for now. It would be interesting if we could use extend this to use C++ static member functions, that would at least maintain the namespacing and reduce naming conflicts with user code (ie. using You can of course declare a partial class again inside the namespace modm::platform {
class Can1
{
public:
static void tx_isr();
} However, you don't know whether So I guess we need to stick to manually declaring C functions, however, we could still wrap it into |
I got a bad feeling that we took a wrong turn with this solution. So I took a step back and looked at the use case again: I actually just want to have code executed when an interrupt occurs additionally to the driver code. So why not just move the calling of the additional code to the driver itself. The idea would be to keep the ISR mechanism as it was. Instead of messing around with the vectors we could define an empty weak default hook function for each ISR inside of the driver and call that from the corresponding ISR. So something like this
This will be called after the body of the CAN1_TX handler of the driver. If it isn't overloaded the compiler will optimize out the function and if we want to do something we can just implement the CustomHook function in the user code. The pros to this are:
What do you think? |
That's basically how ST does it. We could still generalize it into a macro: MODM_ISR_HOOKS(CAN1_TX)
{ /* main body */ }
// expands into
weak modm_extern_c void CAN1_TX_ISR_Enter() {}
weak modm_extern_c void CAN1_TX_ISR_Leave() {}
void CAN1_TX_ISR_Main(); // extern "C" not necessary
modm_extern_c void CAN1_TX_IRQHandler()
{
CAN1_TX_ISR_Enter();
CAN1_TX_ISR_Main();
CAN1_TX_ISR_Leave();
}
static inline void CAN1_TX_ISR_Main()
{ /* main body */} (This obviously depends on whether locally-defined weak functions really are optimized away! This would need to happen by the linker.)
The above is obviously again a "raw" NVIC vector hook, so shared IRQs which change names (Timer IRQs are also pretty bad) from device to device are still and issue. So this would make it driver hook, that happens after checking the ST-specific hardware interrupt flags (if necessary) and it's more difficult to clearly define the semantics here, since a driver provides more events to hook in different ways (possibly with arguments and return values).
Well… we took a lot of wrong turns with modm 😇. It would've solved the issue with shared interrupts in a more generic way. |
Okay so I'll start a new branch tomorrow implementing this method |
So this is the first draft of the custom IRQ implementation (sorry for the messy code).
The collection of the default IRQs for the CAN driver works and the forwarder will be generated in the vectors.c. Also the vector tables have the correct forwarder referenced (I only tried this on a F0 so far).
However the build system won't build and link the
vectors.c
.I'm not really sure how to hack that.