drivers: pwm: nrf: nrf pwm driver modification to use DT #11726

gamnes · 2018-11-29T09:11:09Z

Kconfig options clock_prescaler removed.
Kconfig options CH pin selection moved to DT.
pwm device node added with alias to all
nRF5x DTSI files.
Added yaml binding for Nordic PWM node.
Rewritten pwm_nrfx.c nRF HW PWM driver.
Modified fade_led PWM example to also
include nRF HW PWM option.

Signed-off-by: Gaute Gamnes [email protected]

gamnes · 2018-11-29T09:16:55Z

This pull request is done as part of issue 8758.

This pull request fixes issue 10035.

This pull request assumes issue 6958 will be merged and function get_cycles_per_sec() will eventually be removed.

Setting duty to either 0% or 100% for all channels of a PWM instance will disable the peripheral, see 9507 - if better semantics are introduced, this should be fixed in this driver as well.

dts/arm/nordic/nrf52840.dtsi

dts/bindings/pwm/nordic,nrf-pwm.yaml

drivers/pwm/pwm_nrfx.c

codecov-io · 2018-11-29T10:40:11Z

Codecov Report

Merging #11726 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #11726   +/-   ##
=======================================
  Coverage   48.13%   48.13%           
=======================================
  Files         281      281           
  Lines       43425    43425           
  Branches    10406    10406           
=======================================
  Hits        20902    20902           
  Misses      18368    18368           
  Partials     4155     4155

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e21d7cc...43c18de. Read the comment docs.

ioannisg

Why is only the nrf52_pca10040 board DTS fixed?

zephyrbot · 2018-11-30T14:44:10Z

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

addressed

carlescufi · 2018-12-03T10:57:34Z

@ioannisg @anangl @Mierunski can you re-review please?

dts/bindings/pwm/nordic,nrf-pwm.yaml

drivers/pwm/pwm_nrfx.c

anangl · 2018-12-04T12:31:00Z

Split these changes into several commits. Use separate commits for logically related changes (for instance, don't add DTS entries and change PWM prescaler handling in one commit as these things are unrelated).

gamnes · 2018-12-04T13:32:16Z

Ok. Will split these changes into multiple commits.

dts/bindings/pwm/nordic,nrf-pwm.yaml

galak

one DT change to the binding.

DT binding was changed to boolean

drivers/pwm/pwm_nrfx.c

anangl · 2018-12-07T11:45:24Z

@anangl: Split up into multiple commits, and all the changes you requested should have been resolved.

The changes are split but all the commits have the same title, which is not always adequate to what is actually changed by the commit (for instance the first commit add only dts entries and bindings, but the title says "drivers: pwm: nrf: ...").

gamnes · 2018-12-07T13:39:10Z

Got it, will change the wording of the commit message to be more clear

gamnes · 2018-12-10T15:32:53Z

@anangl - Please see if changes were OK. Thanks

All my comments were addressed.

anangl

Looks good.
Two minor corrections suggested.

drivers/pwm/Kconfig.nrfx

drivers/pwm/pwm_nrfx.c

gamnes · 2018-12-11T11:44:15Z

Thanks @anangl - The last force-push was just to align some indentation in pwm_nrfx.c

carlescufi · 2018-12-11T11:50:31Z

@gamnes can you fix CI?

Mierunski

Looks good, just a few nitpicks

drivers/pwm/pwm_nrfx.c

carlescufi · 2018-12-11T11:52:31Z

@Mierunski can you take another look?

1. PWM device node added with alias to all nRF52x DTSI files. 1 instance for nRF52810, 3 instances for nRF52832, and 4 instance for nRF52840. 2. Added yaml binding for Nordic PWM node. Signed-off-by: Gaute Gamnes <[email protected]>

1. Kconfig options CH pin removed. 2. Kconfig options CH inverted removed. 3. Modified pwm_nrfx.c driver to use DT defines instead of Kconfig. Signed-off-by: Gaute Gamnes <[email protected]>

1. Kconfig options clock_prescaler removed. 2. Rewritten pwm_nrfx.c nRF HW PWM driver. Signed-off-by: Gaute Gamnes <[email protected]>

1. Modified fade_led PWM example to include nRF HW PWM option. 2. Added fade_led nrf52_pca10040.overlay in order to enable PWM node and choose output PWM GPIO for channel 0. Channel 1 GPIO enable but not used in sample src. Signed-off-by: Gaute Gamnes <[email protected]>

gamnes · 2018-12-11T12:45:32Z

Latest force-push was rebase onto upstream master.

gamnes requested review from carlescufi and Mierunski November 29, 2018 09:11

gamnes requested a review from ioannisg as a code owner November 29, 2018 09:11

gamnes requested a review from anangl November 29, 2018 09:17

anangl previously requested changes Nov 29, 2018

View reviewed changes

dts/arm/nordic/nrf52840.dtsi Show resolved Hide resolved

dts/bindings/pwm/nordic,nrf-pwm.yaml Outdated Show resolved Hide resolved

dts/bindings/pwm/nordic,nrf-pwm.yaml Outdated Show resolved Hide resolved

drivers/pwm/pwm_nrfx.c Outdated Show resolved Hide resolved

carlescufi requested a review from galak November 29, 2018 10:34

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch from 634e847 to a624026 Compare November 29, 2018 10:55

zephyrproject-rtos deleted a comment from zephyrbot Nov 29, 2018

ioannisg previously requested changes Nov 29, 2018

View reviewed changes

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch from a624026 to 4470988 Compare November 30, 2018 13:28

zephyrproject-rtos deleted a comment from zephyrbot Nov 30, 2018

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch 2 times, most recently from 9fd8245 to 21a3041 Compare November 30, 2018 14:12

zephyrproject-rtos deleted a comment from zephyrbot Nov 30, 2018

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch from 21a3041 to 3cde910 Compare November 30, 2018 14:45

anangl previously requested changes Dec 4, 2018

View reviewed changes

galak added area: PWM Pulse Width Modulation area: Devicetree platform: nRF Nordic nRFx labels Dec 4, 2018

galak reviewed Dec 4, 2018

View reviewed changes

dts/bindings/pwm/nordic,nrf-pwm.yaml Outdated Show resolved Hide resolved

galak previously requested changes Dec 4, 2018

View reviewed changes

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch from 3cde910 to 960ccae Compare December 4, 2018 16:53

anangl previously requested changes Dec 7, 2018

View reviewed changes

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch 2 times, most recently from a0ee886 to 7a33b56 Compare December 10, 2018 13:32

anangl reviewed Dec 11, 2018

View reviewed changes

drivers/pwm/Kconfig.nrfx Outdated Show resolved Hide resolved

drivers/pwm/pwm_nrfx.c Outdated Show resolved Hide resolved

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch 2 times, most recently from 203d29a to ea8aa9e Compare December 11, 2018 10:56

anangl approved these changes Dec 11, 2018

View reviewed changes

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch from ea8aa9e to e805c0d Compare December 11, 2018 11:05

Mierunski suggested changes Dec 11, 2018

View reviewed changes

drivers/pwm/pwm_nrfx.c Outdated Show resolved Hide resolved

drivers/pwm/pwm_nrfx.c Outdated Show resolved Hide resolved

drivers/pwm/pwm_nrfx.c Outdated Show resolved Hide resolved

drivers/pwm/pwm_nrfx.c Outdated Show resolved Hide resolved

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch from e805c0d to 4249544 Compare December 11, 2018 12:28

Mierunski approved these changes Dec 11, 2018

View reviewed changes

gamnes added 4 commits December 11, 2018 13:43

drivers: pwm: nrf: Migrate Kconfig options to DT

28f86a5

1. Kconfig options CH pin removed. 2. Kconfig options CH inverted removed. 3. Modified pwm_nrfx.c driver to use DT defines instead of Kconfig. Signed-off-by: Gaute Gamnes <[email protected]>

drivers: pwm: nrf: Removed Kconfig prescaler option. Rewrite HW driver

d69a15b

1. Kconfig options clock_prescaler removed. 2. Rewritten pwm_nrfx.c nRF HW PWM driver. Signed-off-by: Gaute Gamnes <[email protected]>

gamnes force-pushed the issue8758_migrate_nRF_HW_PWM_config_to_dts branch from 4249544 to 43c18de Compare December 11, 2018 12:45

carlescufi approved these changes Dec 11, 2018

View reviewed changes

carlescufi merged commit 596a11c into zephyrproject-rtos:master Dec 11, 2018

gamnes deleted the issue8758_migrate_nRF_HW_PWM_config_to_dts branch December 11, 2018 15:14

anangl mentioned this pull request Feb 1, 2019

fade_led sample doesn't work with pwm_nrfx driver #10037

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: pwm: nrf: nrf pwm driver modification to use DT #11726

drivers: pwm: nrf: nrf pwm driver modification to use DT #11726

gamnes commented Nov 29, 2018 •

edited by carlescufi

Loading

gamnes commented Nov 29, 2018

codecov-io commented Nov 29, 2018 •

edited

Loading

ioannisg left a comment

zephyrbot commented Nov 30, 2018 •

edited

Loading

carlescufi commented Dec 3, 2018 •

edited

Loading

anangl commented Dec 4, 2018

gamnes commented Dec 4, 2018

galak left a comment

anangl commented Dec 7, 2018

gamnes commented Dec 7, 2018

gamnes commented Dec 10, 2018

anangl left a comment

gamnes commented Dec 11, 2018

carlescufi commented Dec 11, 2018

Mierunski left a comment

carlescufi commented Dec 11, 2018

gamnes commented Dec 11, 2018

drivers: pwm: nrf: nrf pwm driver modification to use DT #11726

drivers: pwm: nrf: nrf pwm driver modification to use DT #11726

Conversation

gamnes commented Nov 29, 2018 • edited by carlescufi Loading

gamnes commented Nov 29, 2018

codecov-io commented Nov 29, 2018 • edited Loading

Codecov Report

ioannisg left a comment

Choose a reason for hiding this comment

zephyrbot commented Nov 30, 2018 • edited Loading

carlescufi commented Dec 3, 2018 • edited Loading

anangl commented Dec 4, 2018

gamnes commented Dec 4, 2018

galak left a comment

Choose a reason for hiding this comment

anangl commented Dec 7, 2018

gamnes commented Dec 7, 2018

gamnes commented Dec 10, 2018

anangl left a comment

Choose a reason for hiding this comment

gamnes commented Dec 11, 2018

carlescufi commented Dec 11, 2018

Mierunski left a comment

Choose a reason for hiding this comment

carlescufi commented Dec 11, 2018

gamnes commented Dec 11, 2018

gamnes commented Nov 29, 2018 •

edited by carlescufi

Loading

codecov-io commented Nov 29, 2018 •

edited

Loading

zephyrbot commented Nov 30, 2018 •

edited

Loading

carlescufi commented Dec 3, 2018 •

edited

Loading