zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.32k stars 6.32k forks source link

PWM Capture on STM32 doesn't function on Timer 5 Channel 2 #57631

Closed heinwessels closed 1 year ago

heinwessels commented 1 year ago

Describe the bug The current implementation of PWM capture on the STM32 only works for timer channel 1, while it's designed to work for channel 1 and 2. When channel 2 is selected the capture gets stuck with the ISR constantly being triggered forever.

Some symptoms I observed, although it's hard to say which values correct:

I think the base issue is that the CC2IF interrupt is constantly raised. This is because in #57607 this bug still persists, even if I turn off all interrupts except CC2IF.


On further investigation by both @FRASTM and me it seems like this issue only appears on Timer 5 Channel 2. As well as Timer 5 Channel 4 if #57607 is used. Other timers seem to function as expected.


To Reproduce Steps to reproduce the behavio on Nucleo H743zi.

  1. Apply the following patch to change the pwm_loopback test to use Timer 5 Channel 2 for PWM Capture. Note: The same happens when there's a valid input on the capture pin as well.
    
    diff --git a/tests/drivers/pwm/pwm_loopback/boards/nucleo_h743zi.overlay b/tests/drivers/pwm/pwm_loopback/boards/nucleo_h743zi.overlay
    index 5007b962ec..275f6d6398 100644
    --- a/tests/drivers/pwm/pwm_loopback/boards/nucleo_h743zi.overlay
    +++ b/tests/drivers/pwm/pwm_loopback/boards/nucleo_h743zi.overlay
    @@ -11,7 +11,7 @@
                compatible = "test-pwm-loopback";
                /* first index must be a 32-Bit timer */
                pwms = <&pwm2 4 0 PWM_POLARITY_NORMAL>,
    -                       <&pwm5 1 0 PWM_POLARITY_NORMAL>;
    +                       <&pwm5 2 0 PWM_POLARITY_NORMAL>;
        };
    };

@@ -29,7 +29,7 @@ status = "okay"; pwm5: pwm { status = "okay";

Expected behavior The tests to executing normally by timing out. For example, here is the output if I make no changes to the test, except grounding the capture pin. Meaning this is capturing on timer channel 1. It's clear the capture functions as expected, as gracefully times out.

*** Booting Zephyr OS build zephyr-v3.3.0-3460-gb10817ba25b2 ***
Running TESTSUITE pwm_loopback
===================================================================
START - test_capture_busy
E: PWM Capture already in progress
E: PWM capture already active
 PASS - test_capture_busy in 0.006 seconds
===================================================================
START - test_capture_timeout
W: pwm capture timed out
 PASS - test_capture_timeout in 1.003 seconds
===================================================================
START - test_continuous_capture

... not posting the entire log ...

------ TESTSUITE SUMMARY END ------

===================================================================
PROJECT EXECUTION FAILED

Impact Showstopper. It still persists in #57607 which is important for our project. I'm trying to debug why it's happening, but haven't had any luck yet. Any insights will be very helpful.

Environment (please complete the following information):

FRASTM commented 1 year ago

Is it due to timer 5 ? How is the PWM input capture chosen for the test case ?

On my nucleo_h743zi board, I configure

    pwm_loopback_0 {
        compatible = "test-pwm-loopback";
        /* first index must be a 32-Bit timer */
        pwms = <&pwm2 2 0 PWM_POLARITY_NORMAL>,
            <&pwm5 1 0 PWM_POLARITY_NORMAL>;

and as well

    pwm_loopback_0 {
        compatible = "test-pwm-loopback";
        /* first index must be a 32-Bit timer */
        pwms = <&pwm5 1 0 PWM_POLARITY_NORMAL>,
            <&pwm2 2 0 PWM_POLARITY_NORMAL>;

timer2 channel 2 on pb3 (<&tim2_ch2_pb3>; /* CN7 PIN15 D23 */ ) and let timer5 ch1 on PA0 (<&tim5_ch1_pa0>; /* CN10 PIN29 D32 */) --> test PASSED

heinwessels commented 1 year ago

Is it due to timer 5 ? How is the PWM input capture chosen for the test case ?

Thanks for having a look! I was actually also looking into this yesterday, and saw the same. With the following inputs the tests passed for me.

I've only had it fail for Timer 5 on Channel 2 and 4. Can you confirm that it fails for you too? If so it means it could possibly be a bug in the LL drivers?

FRASTM commented 1 year ago

Is it due to timer 5 ? How is the PWM input capture chosen for the test case ?

Thanks for having a look! I was actually also looking into this yesterday, and saw the same. With the following inputs the tests passed for me.

  • Timer 2 Channel 2
  • Timer 2 Channel 4
  • Timer 3 Channel 2 (some tests failed, but it's because it's a 16-bit timer, and the tests are designed for 32-bit)

I've only had it fail for Timer 5 on Channel 2 and 4. Can you confirm that it fails for you too? If so it means it could possibly be a bug in the LL drivers?

Well, not sure about the order in

    pwm_loopback_0 {
        compatible = "test-pwm-loopback";
        pwms = <&pwm5 2 0 PWM_POLARITY_NORMAL>,
            <&pwm2 2 0 PWM_POLARITY_NORMAL>;
};

--> with Timer 5 first :

heinwessels commented 1 year ago

Well, not sure about the order in

The first pwm is the output, meaning the PWM generator, and the second is the input, or PWM capture, which we are testing. So you need to set the second one to TImer 5 Channel 2. There's a diff on how to achieve this in the issue description.

FRASTM commented 1 year ago

Well, not sure about the order in

The first pwm is the output, meaning the PWM generator, and the second is the input, or PWM capture, which we are testing. So you need to set the second one to TImer 5 Channel 2. There's a diff on how to achieve this in the issue description.

OK, In anycase PWM capture "only CH1 and CH2 are supported" With timer5, channel 2 for capture (second position) <&tim5_ch2_pa1> then is the testcase blocked:

START - test_capture_busy
E: counter overflow during PWM capture

(which does not occur when the capture is on timer2)

There is a difference between TIMER 2 and TiMER 5 that the refMan RM0433 mentions in the TIMx status register (TIMx_SR)(x = 2 to 5) _UIF: Update interrupt flag This bit is set by hardware on an update event. It is cleared by software. 0: No update occurred 1: Update interrupt pending. This bit is set by hardware when the registers are updated: At overflow or underflow (for TIM2 to TIM4) and if UDIS=0 in the TIMxCR1 register.

FRASTM commented 1 year ago

The pb seems specific to Timer 5 channel 2 --> I see the same error with nucleo_h563zi Note that I also have set the st,prescaler = <255>; for each Timer

This is due to the HW of the nucleo board pin PA1 of the TIMER5 ch2 : tim5_ch2_pa1 --> it is also connected through the SB57 to the RMII_REF_CLK of the ethernet transceiver Open the SB57 and the test PASSED.

TESTSUITE pwm_loopback succeeded                                                                                                                                    

------ TESTSUITE SUMMARY START ------                                                                                                                               

SUITE PASS - 100.00% [pwm_loopback]: pass = 8, fail = 0, skip = 0, total = 8 duration = 5.147 seconds                                                               
 - PASS - [pwm_loopback.test_capture_busy] duration = 0.006 seconds                                                                                                 
 - PASS - [pwm_loopback.test_capture_timeout] duration = 1.003 seconds                                                                                              
 - PASS - [pwm_loopback.test_continuous_capture] duration = 1.201 seconds                                                                                           
 - PASS - [pwm_loopback.test_period_capture] duration = 0.588 seconds                                                                                               
 - PASS - [pwm_loopback.test_period_capture_inverted] duration = 0.587 seconds                                                                                      
 - PASS - [pwm_loopback.test_pulse_and_period_capture] duration = 0.587 seconds                                                                                     
 - PASS - [pwm_loopback.test_pulse_capture] duration = 0.587 seconds                                                                                                
 - PASS - [pwm_loopback.test_pulse_capture_inverted] duration = 0.588 seconds                                                                                       

------ TESTSUITE SUMMARY END ------    

(The other pin for TIMER 5 channel 2 is PH11 which not present on this stm32h743zi package of the nucleo_h743zi board )

erwango commented 1 year ago

@heinwessels Can you confirm @FRASTM 's observation ?

heinwessels commented 1 year ago

Yeah, @FRASTM seems to be correct.