drivers: pwm_nrf5_sw: Fix the driver and make it support nRF9x SoCs #15287

anangl · 2019-04-09T11:27:32Z

This PR makes the blink_led sample working (again) on boards with nRF52 series SoCs. The SW PWM driver is selected for them instead of the default HW one, as the latter cannot handle periods longer than ~262 ms (this is a hardware limitation).
It also fixes a couple of issues (details below) found in the pwm_nrf5_sw driver and makes it support nRF9x SoCs as well, so that the blink_led sample can be run on boards equipped with those.

drivers: pwm_nrf5_sw: Use dynamically adjusted timer prescaler

Adjust the timer prescaler used by the pwm_nrf5_sw driver according to
the requested period length, so that it does not need to be specified
at the build time. Remove the incorrect "scaling" that was done to the
requested pulse and period cycles to fit them into 16 bits.

samples: blink_led: Use nRF SW PWM driver for nRF52 boards

The nrfx Hardware PWM driver cannot be used in this sample
since the driver does not allow to set a period longer than ~262 ms.

drivers: pwm_nrf5_sw: Correct the use of GPIOTE channels and TIMER

GPIOTE base channel number taken from DeviceTree was not used
consistenly, hence the driver could properly work only when this
number had the value of 0.
TIMER was not stopped during the reconfiguration of the channel
and this could lead to the inversion of the PWM polarity.
The config structure was removed on the occasion as it unnecessarily
complicated the code of this currently single-instance only driver.

drivers: pwm_nrf5_sw: Use nrfx HALs instead of direct register accesses

Now instead of accessing peripheral registers directly, the driver
uses the nrfx HAL functions. This makes the code easier to maintain
and also gives the possibility to use pins from GPIO port P1 on
nRF52840 (absolute numbers 32-47) as the GPIO HAL provides the needed
translation.

drivers: pwm_nrf5_sw: Refactor the code

Move all channel initialization code that does not need to be executed
on every pin reconfiguration (like initialization of the PPI channels)
to the driver initialization function. Simplify the routines that check
the requested period and find a suitable channel for driving the pin.

drivers: pwm_nrf5_sw: Make the driver support nRF9x series SoCs as well

Now the driver uses DPPI or PPI depending on the presence of one of
these interfaces in a given SoC, hence it can be used on nRF9x SoCs
as well. Remove the '5' from its name then, and make the blink_led
sampe use it for the nrf9160_pca10090 board, thus making the sample
working properly on this board.

Fixes #15323.

zephyrbot · 2019-04-09T11:34:02Z

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.

anangl · 2019-04-09T12:09:52Z

Fixed checkpatch and codeowners issues.

ioannisg · 2019-04-09T12:11:27Z

Is there a filed bug for this PR? Otherwise, it shouldn't go to 1.14 - we are close to release, AFAIK

anangl · 2019-04-09T12:19:17Z

Is there a filed bug for this PR? Otherwise, it shouldn't go to 1.14 - we are close to release, AFAIK

I am not aware of any. The bug was reported on slack:

Aaron Tsui 9:15 AM, March 22nd
BTW, As the PWM_CLK lowest is 125khz(16M/128) and COUNTERTOP max is 0x7fff on nrf52, my understanding the nrf52 HW PWM longest period is about 262ms, cannot reach period as 1000ms. right?
@gaute this will cause failure on sample/blink_led, which need a 1000ms period.

Sure. Could you file an issue in GH with "bug" and 1.14" that relates to the slack report?

anangl · 2019-04-09T12:21:49Z

cc @overheat

overheat · 2019-04-09T13:18:54Z

@anangl @ioannisg I didn't raise an issue specific for this topic. But I think this is related to #14749, which is verifying samples. Since this is a basic sample bug, I think it's better to be solved before the LTS version released.

anangl · 2019-04-10T08:54:27Z

Added one line that I somehow lost during splitting the changes into several logical commits. Sorry about that.

anangl · 2019-04-10T09:12:26Z

@ioannisg

Is there a filed bug for this PR? Otherwise, it shouldn't go to 1.14 - we are close to release, AFAIK

I am not aware of any. The bug was reported on slack:

[..]

Sure. Could you file an issue in GH with "bug" and 1.14" that relates to the slack report?

Created #15323.

anangl · 2019-04-11T08:37:24Z

Removed a no longer needed overlay file that was changing the clock-prescaler property eliminated by this PR.

carlescufi · 2019-06-11T13:45:18Z

@cvinayak @kl-cruz @nordic-krch can you please take a look?

drivers/pwm/pwm_nrf_sw.c

Adjust the timer prescaler used by the pwm_nrf5_sw driver according to the requested period length, so that it does not need to be specified at the build time. Remove the incorrect "scaling" that was done to the requested pulse and period cycles to fit them into 16 bits. Signed-off-by: Andrzej Głąbek <[email protected]>

Remove the no longer used "clock-prescaler" property from the bindings for "nordic,nrf-sw-pwm" compatible nodes and all assignments to this property. This means also the removal of the bbc_microbit.overlay file from the servo_motor sample (as the assignment is the only purpose of this file) together with the entry covering this file in CODEOWNERS. Signed-off-by: Andrzej Głąbek <[email protected]>

GPIOTE base channel number taken from DeviceTree was not used consistenly, hence the driver could properly work only when this number had the value of 0. TIMER was not stopped during the reconfiguration of the channel and this could lead to the inversion of the PWM polarity. The config structure was removed on the occasion as it unnecessarily complicated the code of this currently single-instance only driver. Signed-off-by: Andrzej Głąbek <[email protected]>

anangl · 2019-07-12T13:26:14Z

Rebased on master.

nordic-krch · 2019-07-15T09:29:39Z

@anangl is there any way to detect if there is conflict in TIMER instance usage?

Now instead of accessing peripheral registers directly, the driver uses the nrfx HAL functions. This makes the code easier to maintain and also gives the possibility to use pins from GPIO port P1 on nRF52840 (absolute numbers 32-47) as the GPIO HAL provides the needed translation. Signed-off-by: Andrzej Głąbek <[email protected]>

Move all channel initialization code that does not need to be executed on every pin reconfiguration (like initialization of the PPI channels) to the driver initialization function. Simplify the routine that checks the requested period and the one that finds a suitable channel for driving the pin. Signed-off-by: Andrzej Głąbek <[email protected]>

The nrfx Hardware PWM driver cannot be used in this sample since the driver does not allow to set a period longer than ~262 ms. The sw_pwm node is made enabled by default in dts for all nRF52 series SoCs (just like it is already done for nRF51 series) so that there is no need to do it for particular boards to make the SW PWM driver available for them (this will only cause that the corresponding DT_ macros will be created, the driver will be compiled and used after it is selected in Kconfig). Signed-off-by: Andrzej Głąbek <[email protected]>

Now the driver uses DPPI or PPI depending on the presence of one of these interfaces in a given SoC, hence it can be used on nRF9x SoCs as well. Remove the '5' from its name then, and make the blink_led sample using it for the nrf9160_pca10090 board, thus making the sample working properly on this board. Signed-off-by: Andrzej Głąbek <[email protected]>

This approach provides more flexibility (the PPI channels utilized by this driver no longer need to have consecutive numbers) and eliminates the need of certain hacks in nrfx_glue.h and radio_nrf5_ppi.h. Signed-off-by: Andrzej Głąbek <[email protected]>

anangl · 2019-11-06T10:00:40Z

Replaced (as a fix for #15323) by #20369.

anangl added bug The issue is a bug, or the PR is fixing a bug area: Drivers area: Boards platform: nRF Nordic nRFx labels Apr 9, 2019

anangl added this to the v1.14.0 milestone Apr 9, 2019

anangl requested review from nashif, henrikbrixandersen, ioannisg, MaureenHelm, gamnes, carlescufi and galak April 9, 2019 11:27

anangl requested review from cvinayak, jhedberg, sjanc, thoh-ot and Vudentz as code owners April 9, 2019 11:27

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 95ba0bb to 1babcc9 Compare April 9, 2019 12:08

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 1babcc9 to 333e1ba Compare April 10, 2019 08:53

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 333e1ba to 42c19c9 Compare April 11, 2019 08:33

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 42c19c9 to 12e755a Compare April 11, 2019 08:52

carlescufi requested a review from nordic-krch June 11, 2019 13:45

nordic-krch reviewed Jun 18, 2019

View reviewed changes

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

drivers/pwm/pwm_nrf_sw.c Show resolved Hide resolved

henrikbrixandersen removed their request for review June 21, 2019 18:04

joerchan removed their request for review July 8, 2019 10:22

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 724759a to 8a4c7d1 Compare July 12, 2019 13:17

anangl added 3 commits July 12, 2019 15:19

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 8a4c7d1 to 52ebe69 Compare July 12, 2019 13:24

zephyrbot added area: PWM Pulse Width Modulation area: Devicetree area: Bluetooth area: Samples Samples EXT Has change or related to ext/ (obsolete) labels Jul 12, 2019

anangl added the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Jul 12, 2019

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 52ebe69 to 9cf417d Compare July 15, 2019 09:06

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 9cf417d to 452c3eb Compare July 15, 2019 13:42

anangl added 5 commits July 15, 2019 15:43

anangl force-pushed the fix_pwm_nrf_sw_driver branch from 452c3eb to b13987f Compare July 15, 2019 13:43

pabigot mentioned this pull request Aug 26, 2019

blink_led sample does not work on most of the nRF boards #15323

Closed

anangl closed this Nov 6, 2019

anangl deleted the fix_pwm_nrf_sw_driver branch March 11, 2022 13:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: pwm_nrf5_sw: Fix the driver and make it support nRF9x SoCs #15287

drivers: pwm_nrf5_sw: Fix the driver and make it support nRF9x SoCs #15287

anangl commented Apr 9, 2019 •

edited

Loading

zephyrbot commented Apr 9, 2019 •

edited

Loading

anangl commented Apr 9, 2019

ioannisg commented Apr 9, 2019

anangl commented Apr 9, 2019 •

edited by ioannisg

Loading

anangl commented Apr 9, 2019

overheat commented Apr 9, 2019

anangl commented Apr 10, 2019

anangl commented Apr 10, 2019

anangl commented Apr 11, 2019

carlescufi commented Jun 11, 2019

anangl commented Jul 12, 2019

nordic-krch commented Jul 15, 2019

anangl commented Nov 6, 2019

drivers: pwm_nrf5_sw: Fix the driver and make it support nRF9x SoCs #15287

drivers: pwm_nrf5_sw: Fix the driver and make it support nRF9x SoCs #15287

Conversation

anangl commented Apr 9, 2019 • edited Loading

zephyrbot commented Apr 9, 2019 • edited Loading

anangl commented Apr 9, 2019

ioannisg commented Apr 9, 2019

anangl commented Apr 9, 2019 • edited by ioannisg Loading

anangl commented Apr 9, 2019

overheat commented Apr 9, 2019

anangl commented Apr 10, 2019

anangl commented Apr 10, 2019

anangl commented Apr 11, 2019

carlescufi commented Jun 11, 2019

anangl commented Jul 12, 2019

nordic-krch commented Jul 15, 2019

anangl commented Nov 6, 2019

anangl commented Apr 9, 2019 •

edited

Loading

zephyrbot commented Apr 9, 2019 •

edited

Loading

anangl commented Apr 9, 2019 •

edited by ioannisg

Loading