Skip to content

Conversation

@raffarost
Copy link

@raffarost raffarost commented Nov 5, 2024

Use GPIO interrupts to check PWM output timing.

Depends on #80872

Raffael Rostagno added 2 commits November 6, 2024 09:54
Use GPIO to check PWM output timing.

Signed-off-by: Raffael Rostagno <[email protected]>
Update overlay files to add GPIO input for timing checks.

Signed-off-by: Raffael Rostagno <[email protected]>
@raffarost raffarost marked this pull request as ready for review November 6, 2024 18:54
@zephyrbot zephyrbot added area: PWM Pulse Width Modulation area: Devicetree labels Nov 6, 2024
@raffarost raffarost added the platform: ESP32 Espressif ESP32 label Nov 6, 2024
@raffarost raffarost added the DNM This PR should not be merged (Do Not Merge) label Nov 7, 2024
@raffarost raffarost marked this pull request as draft November 14, 2024 13:29
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raffarost
Copy link
Author

This is what https://github.com/zephyrproject-rtos/zephyr/tree/main/tests/drivers/pwm/pwm_loopback is for?

Hi @henrikbrixandersen,
I understand pwm_loopback requires the peripheral to support CC, which is not the case for all drivers (e.g ESP's LEDC pwm).

@henrikbrixandersen
Copy link
Member

This is what main/tests/drivers/pwm/pwm_loopback is for?

Hi @henrikbrixandersen, I understand pwm_loopback requires the peripheral to support CC, which is not the case for all drivers (e.g ESP's LEDC pwm).

Does the chip have another PWM-capture capable peripheral? Producer and consumer does not need to be of the same peripheral type in pwm_loopback.

I'm okay with introducing a GPIO-based PWM loopback test, but I don't think it should be in the main PWM API test suite like this (which is also why I placed the initial loopback test in its own test suite).

@raffarost
Copy link
Author

This is what main/tests/drivers/pwm/pwm_loopback is for?

Hi @henrikbrixandersen, I understand pwm_loopback requires the peripheral to support CC, which is not the case for all drivers (e.g ESP's LEDC pwm).

Does the chip have another PWM-capture capable peripheral? Producer and consumer does not need to be of the same peripheral type in pwm_loopback.

I'm okay with introducing a GPIO-based PWM loopback test, but I don't think it should be in the main PWM API test suite like this (which is also why I placed the initial loopback test in its own test suite).

Some Espressif chips only have LEDC and no MCPWM, thus no CC available. The test was meant to be a simple timing check, because we've had some regressions after HAL updates. I can create a separate test suite for it. I'd just need to think how to avoid repeating code, as the 'configs', 'sets' and test sequence would be the same.

@raffarost
Copy link
Author

raffarost commented Nov 28, 2024

@henrikbrixandersen I created a PR for the new implementation (#82258)

@bjarki-andreasen
Copy link
Contributor

should this one be closed in favor of #82258 ?

@raffarost raffarost closed this Dec 18, 2024
@raffarost raffarost deleted the tests/ledc_loop branch December 18, 2024 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Devicetree area: PWM Pulse Width Modulation DNM This PR should not be merged (Do Not Merge) platform: ESP32 Espressif ESP32

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants