drivers/interrupt_controller: stm32: Complete driver factorization #12463
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12463 +/- ##
=======================================
Coverage 53.96% 53.96%
=======================================
Files 242 242
Lines 27671 27671
Branches 6720 6720
=======================================
Hits 14934 14934
Misses 9932 9932
Partials 2805 2805Continue to review full report at Codecov.
|
|
Looks like L0 and L4 code became duplicated somehow. These series are mentioned in shared code, but also have their own copies of it. |
erwango
force-pushed
the
fix_exti_factor
branch
from
eb4e9b3 to
84ce418
Compare
|
@tagunil , txs for review, this should be fixed now |
|
@erwango, thanks, this driver looks so much better now. By the way, could you please also make #ifdefs/#ifs around EXTI_LINES more coherent? |
erwango
force-pushed
the
fix_exti_factor
branch
from
84ce418 to
d7d9adb
Compare
|
recheck |
Complete code factorization in stm32 exti drivers. Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
erwango
force-pushed
the
fix_exti_factor
branch
from
d7d9adb to
dcc96bd
Compare
|
@tagunil , can you review again? |
|
@erwango, it may be outside the scope of this cleanup PR, but why we still treat F2/F4/F7 differently by enabling IRQs for lines >=16 on these series and not enabling on others? Which way it should be done actually? |
|
@tagunil, this is right, support could be enhanced, but this was not the aim of this PR. We can log a ticket for that purpose. |
|
@erwango |
| case 23: | ||
| irqnum = LPTIM1_IRQn; | ||
| break; | ||
| #endif | ||
| default: | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
case F2/F4/F7 seems in line with previous code.
But I'm not sure about F1/F3. In these case if line is above 15 what happens? It seems irqnum is
not assigned.
There was a problem hiding this comment.
@avisconti, I reworked a bit the change to be closer to previous code for L4.
For F1/F3, line > 15, code was obviously wrong, also we missed handling for not supported lines for F2/F4/F7 so reporting an error now.
This PR was meant for refactoring. So I guess some more effort would be required to enhance functionality, but I don't want to address that now.
There was a problem hiding this comment.
For F1/F3, line > 15, code was obviously wrong, also we missed handling for not supported lines for F2/F4/F7 so reporting an error now.
OK, got it now.
|
@avisconti , can you review? |
| @@ -476,7 +384,8 @@ static void __stm32_exti_connect_irqs(struct device *dev) | |||
| { | |||
| ARG_UNUSED(dev); | |||
|
|
|||
| #ifdef CONFIG_SOC_SERIES_STM32F0X | |||
| #if defined(CONFIG_SOC_SERIES_STM32F0X) || \ | |||
| defined(CONFIG_SOC_SERIES_STM32L0X) | |||
| IRQ_CONNECT(EXTI0_1_IRQn, | |||
There was a problem hiding this comment.
So L4 is same as F1/F3 and L0 is same as F0. Is that wanted?
|
I also re-tested the two commits on ArgonKey (microphone and sensor samples) and on |
erwango
force-pushed
the
fix_exti_factor
branch
from
d1b33ac to
e21b592
Compare
| * | ||
| * Chapter 10.2: External interrupt/event controller (EXTI) | ||
| * Driver is currently implemented to support following EXTI lines | ||
| * STM32F0/STM32L0/STM32L4: Lines 0 to 15. Lines > 15 not supported |
There was a problem hiding this comment.
This is F1/F3/L4. Correct?
Complete code factorization in stm32 exti drivers. Add return value in case line is not implemented. Except returned error code, refactor has been done iso-feature compared to previous code. Hence error is reported only when support was not available on previous series. Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
erwango
force-pushed
the
fix_exti_factor
branch
from
e21b592 to
fc095bc
Compare
Complete code factorization in stm32 exti drivers.
Tested and validated against all STM32 series
Signed-off-by: Erwan Gouriou erwan.gouriou@linaro.org