analogdevicesinc / no-OS

Software drivers in C for systems without an operating system
http://analogdevicesinc.github.io/no-OS/
Other
927 stars 1.65k forks source link

drivers:api:Add an API to clear pending irq #2136

Closed D-Disha closed 5 months ago

D-Disha commented 6 months ago

No-os API to clear pending irq is added. Corresponding STM32 platform layer api is added and calls to HAL Layer is made.

Pull Request Description

Please replace this with a detailed description and motivation of the changes. You can tick the checkboxes below with an 'x' between square brackets or just check them after publishing the PR. If this PR contains a breaking change, list dependent PRs and try to push all related PRs at the same time.

PR Type

PR Checklist

CiprianRegus commented 6 months ago

Hi @D-Disha,

Do you have a specific use case for this?

D-Disha commented 6 months ago

Hi @CiprianRegus , Yes, it is added for ad7124 project for stm32h563zi support use case for clearing pending interrupts. It is tested out on that and works well. SDP K1 , Nucleol55qe also has a similar HAL Layer call, and this API works for those as well.

rbolboac commented 6 months ago

Hi @D-Disha , Can you please describe the current behavior without this change? Is the interrupt not cleared? If you take a look here: https://github.com/STMicroelectronics/stm32h5xx_hal_driver/blob/main/Src/stm32h5xx_hal_gpio.c#L577 the interrupt is cleared before calling the callback so this functionality should already be implemented in the HAL.

D-Disha commented 6 months ago

@rbolboac Yes, it is already implemented.

However, I was facing issues with spurious interrupts. Interrupts are configured to trigger at a falling edge in my application, but the interrupt was getting triggered even without a falling edge occurring.

So, it is entering the ISR at an unexpected condition. The wrong interrupt is fired immediately after a "legitimate" interrupt, if the flag in peripheral causing the interrupt is cleared late in the ISR.

The fix here was to again clear the interrupt gpio before enabling back the interrupt.

Again, this might not occur in all the applications, my sampling frequency was low and I have somewhat of a lengthy ISR stemming to issues.

rbolboac commented 6 months ago

@rbolboac Yes, it is already implemented.

However, I was facing issues with spurious interrupts. Interrupts are configured to trigger at a falling edge in my application, but the interrupt was getting triggered even without a falling edge occurring.

So, it is entering the ISR at an unexpected condition. The wrong interrupt is fired immediately after a "legitimate" interrupt, if the flag in peripheral causing the interrupt is cleared late in the ISR.

The fix here was to again clear the interrupt gpio before enabling back the interrupt.

Again, this might not occur in all the applications, my sampling frequency was low and I have somewhat of a lengthy ISR stemming to issues.

So... this would be needed for a workaround in your project. I am not sure I would agree 100% with this. The interrupt is cleared in the HAL, I would not interfere with this implementation. I cannot think about a normal use-case in which the user would want to also clear the interrupt if it is already cleared. Let's see what my colleagues have to say as well. Maybe this issue can be solved another way, this fix does not seem to be fixing the root-cause of the behavior you are seeing.. Maybe the interrupt line is too noisy or the ad7124 chip is not working as expected and is sending spurious impulses.

D-Disha commented 6 months ago

@rbolboac No there is nothing wrong with the eval-ad7124 adc I am using and it completely works fine with mbed, only with stm32 this issue is observed. I understand that it is a workaround for my project. I could directly call from the HAL_Layer for now, but for any future projects within the team if this issue is observed, it'd be helpful for us to have an API in No-Os. I understand that this is not needed in most cases.

CiprianRegus commented 6 months ago

Are both the mbed and stm32 examples running on the same hardware (eval board for stm32h563zi)?

D-Disha commented 6 months ago

Are both the mbed and stm32 examples running on the same hardware (eval board for stm32h563zi)?

The mbed example is running for SDP-K1 and STM32H563ZI is for the STM support . Same EVAL-AD7124-4 board is used for both platforms

rbolboac commented 6 months ago

@buha , any input here?

CiprianRegus commented 6 months ago

I would suggest to debug this further and fix the root cause of the issue instead of applying a work around. From what you're saying this seems to be a problem with the microcontroller's interrupt setting. Since we don't even have official support for the STM32H5, there might be some configurations missing.

You said that you have a long interrupt. Is there a possibility of getting another falling edge while it's still processing? Can you also add the commits which make use of this functionality?

gprasadbng commented 6 months ago

I would suggest to debug this further and fix the root cause of the issue instead of applying a work around. From what you're saying this seems to be a problem with the microcontroller's interrupt setting. Since we don't even have official support for the STM32H5, there might be some configurations missing.

You said that you have a long interrupt. Is there a possibility of getting another falling edge while it's still processing? Can you also add the commits which make use of this functionality?

I guess this is the basic API we used in many project and may be first time being used in the PCG firmware. It is a new API and hence will not break any other module. Having this API in the no_os layer is really helpful in my opinion.

D-Disha commented 5 months ago

Hi all, are there any more comments on this PR?