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

LED pins always configured as PWM outputs #25337

Closed
henrikbrixandersen opened this issue May 15, 2020 · 11 comments · Fixed by #25456
Closed

LED pins always configured as PWM outputs #25337

henrikbrixandersen opened this issue May 15, 2020 · 11 comments · Fixed by #25456
Assignees
Labels
area: Devicetree area: LED Label to identify LED subsystem area: Pinmux area: PWM Pulse Width Modulation bug The issue is a bug, or the PR is fixing a bug priority: high High impact/importance bug
Milestone

Comments

@henrikbrixandersen
Copy link
Member

Describe the bug
I have come across an issue with the recent devicetree changes. For several boards, the pinmux.c logic previously decided whether to configure LED pins as GPIOs or PWMs based on Kconfig. Now they decide based on DT_NODE_HAS_COMPAT_STATUS(…, okay) on the PWM controller, which - at least for some boards - leads to always configuring the LED pins as PWMs, thus breaking samples using the LEDs as GPIOs.

This is definitely an issue with several NXP boards, but similar issues may exist on other boards.

To Reproduce
Steps to reproduce the behavior:

  1. cd samples/basic/blinky; mkdir build; cd build
  2. cmake -DBOARD=twr_ke18f
  3. make flash
  4. Observe no blinking LED

Expected behavior
Amber LED (led0, D9) should blink.

Impact
Unable to control LEDs using GPIOs.

Environment (please complete the following information):

  • OS: Ubuntu 18.04
  • Toolchain Zephyr SDK
  • Commit SHA: v2.3.0-rc1-99-ga0ae53410e
@henrikbrixandersen henrikbrixandersen added bug The issue is a bug, or the PR is fixing a bug area: PWM Pulse Width Modulation area: Devicetree area: Pinmux area: LED Label to identify LED subsystem labels May 15, 2020
@henrikbrixandersen henrikbrixandersen added this to the v2.3.0 milestone May 15, 2020
@henrikbrixandersen
Copy link
Member Author

See e.g.

#if DT_NODE_HAS_COMPAT_STATUS(DT_NODELABEL(ftm0), nxp_kinetis_ftm_pwm, okay)
for an example of this. Node ftm3 is always enabled in the devicetree leading to the LED pins always being configured as PWM outputs.

@henrikbrixandersen henrikbrixandersen added the priority: high High impact/importance bug label May 15, 2020
@henrikbrixandersen
Copy link
Member Author

Setting priority as high, since this breaks existing board definitions.

@henrikbrixandersen
Copy link
Member Author

Potentially covered by #24745.

@erwango
Copy link
Member

erwango commented May 15, 2020

relates to #25251

@galak
Copy link
Collaborator

galak commented May 15, 2020

In look at this on at least twr_ke18f we need to decide which case should be the default from the board.dts point of view (GPIO or PWM), and then add overlays to any samples that want to behave differently. The reason for this is we desire to move pinmux configuration into DTS going forward and so when that happens we'll run into this again at that point. So lets just address this once right now.

@galak
Copy link
Collaborator

galak commented May 15, 2020

Boards impacted:

  • frdm_k82f
  • frdm_kw41z
  • hexiwear_k64
  • twr_ke18f
  • rv32m1_vega

@galak
Copy link
Collaborator

galak commented May 15, 2020

@henrikbrixandersen , @MaureenHelm would appreciate if you can comment on the default setting for the PINs. I would think GPIO and then have blinky_pwm, fade_led, and rgb_led have overlays to switch to PWM instead of GPIO.

@henrikbrixandersen
Copy link
Member Author

@henrikbrixandersen , @MaureenHelm would appreciate if you can comment on the default setting for the PINs. I would think GPIO and then have blinky_pwm, fade_led, and rgb_led have overlays to switch to PWM instead of GPIO.

@galak I would say that LEDs as GPIOs would be a sane default. Board specific overlays in the PWM samples could very well be the best way of handling this.

@henrikbrixandersen
Copy link
Member Author

Another approach to the same issue for STM32 boards was taken by @erwango in #25288.

@galak
Copy link
Collaborator

galak commented May 19, 2020

Another approach to the same issue for STM32 boards was taken by @erwango in #25288.

It would work, just means punting the issue to 2.4. The problem will come up again when we move pinmux data to DTS. So I'd say we try and make a decision about it now if we can.

@carlescufi
Copy link
Member

Release meeting: Decided to follow the approach in #25288 for the 5 boards listed here. @galak will follow-up with a PR.

galak added a commit to galak/zephyr that referenced this issue May 19, 2020
Add an additional check for CONFIG_PWM to decide if pins associated with
LED are configured for GPIO or PWM.

Fixes zephyrproject-rtos#25337

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
carlescufi pushed a commit that referenced this issue May 20, 2020
Add an additional check for CONFIG_PWM to decide if pins associated with
LED are configured for GPIO or PWM.

Fixes #25337

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
krip-tip pushed a commit to krip-tip/zephyr-local that referenced this issue May 30, 2020
Add an additional check for CONFIG_PWM to decide if pins associated with
LED are configured for GPIO or PWM.

Fixes zephyrproject-rtos#25337

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree area: LED Label to identify LED subsystem area: Pinmux area: PWM Pulse Width Modulation bug The issue is a bug, or the PR is fixing a bug priority: high High impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants