[2.0.x] Simplify HAL_ENABLE_ISRs

[thinkyhead]

Preliminary changes pending an update that drops in_temp_isr as obsolete for platforms that have interrupt priority scheduling. See #10439.

CC: @p3p @Bob-the-Kuhn @xC0000005

[ejtagle]

@thinkyhead : I have reviewed all your changes. I do agree that dropping the in_temp_isr is the way to go.

May I ask something ?

I think the idea behind all the HAL_ENABLE_ISRs / DISABLE_TEMPERATURE_INTERRUPT / ENABLE_TEMPERATURE_INTERRUPT / DISABLE_STEPPER_INTERRUPT / ENABLE_TEMPERATURE_INTERRUPT

macros was to enable preemption of the Temperature ISR by the Stepper ISR. So good so far.
All 32bit CPUs support interrupt priority programming, so under any 32bit CPU, by just exploiting the NVIC (Nested vectored interrupt controller) you can easily prioritize them and there is absolutely no need for the in_temp_isr and the HAL_ENABLE_ISRs macros, as you just can leave temperature interrupts and stepper interrupts always enabled, and hardware will take care of all the prioritization

The only platform that is problematic here is AVR, that completely lacks interrupt prioritization capabilities, BUT ... You can easily implement prioritization in software!.

The idea is very simple: Stepper ISR does not touch interrupt enabling/disabling of peripherals or the global interrrupt flag at all, thus will run with interrupts disabled, and no other interrupt source can interrupt it.

Temperature ISR must disable itself and reenable global interrupts at the start of the ISR. Then any other ISR will be able to preempt it. But the Temperature ISR will be not reentered.
At the very end of the ISR, you disable global interrupts, clean the interrupt pending flag of the timer (used to create the periodic interrupts of the Temperature isr) and then reenable the timer interrupt.

You have implemented interrupt prioritization in AVR (in fact, to be completely honest, you should not clear the interrupt pending flag of the temperature timer at the end of the ISR, so if for some reason a temperature ISR was preempted by the Stepper isr, then it will be executed and not forgotten!)

As you see, there is no reason for complex interdependant enabling/disabling of interrupts as it is done right now ! :D :D

[thinkyhead]

I'm gratified that you can see the gist of this PR and the thread at #10439!
Now that we're on the same page, do you want to implement it or should I?

[ejtagle]

My Ramps can´t be plugged into the AVR, so my changes would be theoretical (untested on a printer). If that is fine with you, i have no problems implementing it :)

[thinkyhead]

Having an implementation would be a good first step so we can then get into testing it, so please go ahead and write it up for 2.0.x. Marlin 1.1.x can be left as it is, as I'm hoping to release 1.1.9 tonight or tomorrow in spite of the skipped steps issue that some users of the latest Cura are seeing.

[xC0000005]

as you just can leave temperature interrupts and stepper interrupts always enabled, and hardware will take care of all the prioritization

I've been trying this in STM32F1. It does look like the planner's buffer_segment call seems to expect that the stepper ISR will not run while it's writing the first split block, but that could be avoided by writing the second block first, then the first one. (Yes, that would be some interesting code, but it would work). Other than that, most of the disable calls don't look like they are necessary with proper interrupt priority.

[thinkyhead]

For the split block concept the important thing is that the second block get added to the planner before the first block starts processing. This ensures that the blocks get properly chained.

[ejtagle]

@thinkyhead : Got a new PR with the proposed changes.
@xC0000005 : Please, properly assign the interrupt priorities to STM32F1. There is no code assigning them . Take a look at #10496 and test it.

And doubts, bug reports, any detailed explanation required, just ask

[xC0000005]

I’m getting a change ready to do exactly that, and init the NVIC priority group so pre-emption works.

…

On Apr 22, 2018, at 10:31 PM, Eduardo José Tagle ***@***.***> wrote: @thinkyhead <https://github.com/thinkyhead> : Got a new PR with the proposed changes. @xC0000005 <https://github.com/xC0000005> : Please, properly assign the interrupt priorities to STM32F1. There is no code assigning them . Take a look at #10496 <#10496> and test it. And doubts, bug reports, any detailed explanation required, just ask — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10485 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AeppcVzcMJYtjMw5joAaq6B3gCywJDizks5trWdAgaJpZM4TewrP>.