Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arch/arm/tiva: Add complementary PWM mode support #15872

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sydeney
Copy link

@sydeney sydeney commented Feb 19, 2025

This commit introduces support for complementary PWM mode on the Tiva C-Series microcontrollers. The feature allows configuring specific PWM generators to operate in complementary mode via Kconfig.

Summary

This change adds support for complementary PWM mode on Tiva C-Series microcontrollers in NuttX.

The feature allows configuring individual PWM generators to operate in complementary mode via Kconfig options.
The implementation preserves backward compatibility, ensuring that the default behavior remains unchanged.
The complementary mode can be enabled for each generator separately using:

  • CONFIG_TIVA_PWM_COMPLEMENTARY_G0
  • CONFIG_TIVA_PWM_COMPLEMENTARY_G1
  • CONFIG_TIVA_PWM_COMPLEMENTARY_G2
  • CONFIG_TIVA_PWM_COMPLEMENTARY_G3

The changes are contained within:

  • arch/arm/src/tiva/common/tiva_pwm.c

Impact

This change:

  • Adds new functionality (complementary PWM mode) without breaking existing behavior.
  • Introduces Kconfig options for enabling complementary mode on specific PWM generators.
  • Improves flexibility in PWM signal generation for Tiva MCUs.
  • Does not introduce any API-breaking changes or impact other drivers.

Testing

The implementation was tested on:

  • Target Board: TM4C1294 LaunchPad
  • Build Environment: Ubuntu 22.04, GCC 11.3.0, arm-none-eabi-gcc

Configuration: Custom defconfig with:

CONFIG_TIVA_PWM=y
CONFIG_TIVA_PWM_COMPLEMENTARY_G2=y

Before change

evidence1

After change

evidence3

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Size: S The size of the change in this PR is small labels Feb 19, 2025
@nuttxpr
Copy link

nuttxpr commented Feb 19, 2025

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although it is missing some key information in the Testing section.

While the summary and impact sections are well-written and address all the necessary points, the testing section needs improvement. Specifically, it lacks "Testing logs before change" and "Testing logs after change." These logs are crucial for demonstrating the actual impact of the changes and verifying the functionality. Simply stating the target and build environment is insufficient. The logs should show the PWM output before and after enabling the complementary mode to prove the change is working as intended. Include relevant output from a logic analyzer or oscilloscope demonstrating the complementary PWM signals.

Provide concrete examples of the tests performed and their corresponding output. For example:

  • Before Change: Show PWM output on G2 without complementary mode (standard single-channel PWM).
  • After Change: Show PWM output on G2 with complementary mode enabled, demonstrating the expected inverted signal on the complementary output pin.

By adding these testing logs, the PR will fully comply with the NuttX requirements and provide reviewers with the necessary evidence to evaluate the changes effectively.

@michallenc
Copy link
Contributor

Nice contribution! Apart from those two comments, don't you have to setup GPIO for low PWM output as well? It seems the call for high output GPIO is in tiva_pwm_setup, not sure about the low one, I am not familiar with Tiva drivers.

@acassis
Copy link
Contributor

acassis commented Feb 19, 2025

@sydeney nice work! TIVA arch deserves more care, remember the pandemic when all STM32 are gone. It is good to have alternatives!

@sydeney sydeney force-pushed the feature-tiva-pwm-complementary branch from 7974674 to ac60544 Compare February 19, 2025 15:58
@sydeney
Copy link
Author

sydeney commented Feb 19, 2025

@sydeney nice work! TIVA arch deserves more care, remember the pandemic when all STM32 are gone. It is good to have alternatives!

I agree. My goal is to make TIVA arch as complete as possible to power electronics projects.

@sydeney sydeney force-pushed the feature-tiva-pwm-complementary branch 3 times, most recently from 1fe71c5 to ba8a05b Compare February 19, 2025 17:52
This commit improves the configuration of complementary PWM mode for
Tiva C-Series microcontrollers by moving the complementary flag
definition directly into each channel structure.

Changes made:
- Renamed 'complementary_generation' to 'complementary' for clarity.
- Moved complementary mode configuration into the static PWM
  channel structures.
- Replaced runtime conditional checks with compile-time configuration
  using '#ifdef CONFIG_TIVA_PWM_COMPLEMENTARY_Gx'.
- Improved readability and maintainability of the PWM driver.

These modifications ensure a more efficient initialization process,
reduce runtime conditionals, and align better with NuttX coding practices.

Signed-off-by: Sydeney Araujo <[email protected]>
@sydeney sydeney force-pushed the feature-tiva-pwm-complementary branch from ba8a05b to 8c1dde4 Compare February 19, 2025 19:53
@hartmannathan hartmannathan self-requested a review February 20, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants