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

PWM silently fails when changing output frequency on stm32 #28278

Closed
bpbradley opened this issue Sep 10, 2020 · 7 comments
Closed

PWM silently fails when changing output frequency on stm32 #28278

bpbradley opened this issue Sep 10, 2020 · 7 comments
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: low Low impact/importance bug Waiting for response Waiting for author's response

Comments

@bpbradley
Copy link
Contributor

Describe the bug
When trying to change the pwm frequency with pwm_pin_set_usec, the output will regularly dropout. The behavior is reproduced more quickly by changing the frequency more often. If I change it at about a 1 second interval, it fails about 10% of the time, and if I change it at 100ms, it fails most of the time.

To Reproduce
Here is a basic example which reproduces the behavior

#include <zephyr.h>
#include <sys/printk.h>
#include <sys/util.h>
#include <drivers/pwm.h>
#include <device.h>
#if DT_NODE_HAS_STATUS(DT_NODELABEL(pwm4), okay)
#define PWM_DEV_NAME DT_LABEL(DT_NODELABEL(pwm4))
#else
#error "Define a PWM device"
#endif
#define DEFAULT_PWM_PORT 1
#define HZ_TO_USEC(f)   (USEC_PER_SEC / f)
static int set_pwm_period(uint32_t port, uint32_t period, uint32_t pulse)
{
	struct device *pwm_dev = device_get_binding(PWM_DEV_NAME);
	if (!pwm_dev) {
		printk("Cannot get PWM device\n");
		return -ENODEV;
	}
	/* Verify pwm_pin_set_usec() */
	if (pwm_pin_set_usec(pwm_dev, port, period, pulse, 0)) {
		printk("Fail to set the period and pulse width\n");
		return -EFAULT;
	}
	return 0;
}
void  main(){
    while(1){
		for (int f = 100 ; f < 5000; f+=100){
			uint32_t per = HZ_TO_USEC(f);
			set_pwm_period(DEFAULT_PWM_PORT, per, per/2);
			k_sleep(K_MSEC(100));
		}
    }
}

Expected behavior
I expect this to perform a sweep through frequencies from 100Hz to 5Khz in 100Hz steps, changing frequencies by 100Hz every 100ms. I would also expect that I could do this at a much higher rate than this.

Impact
Major impact for my use case, as this project requires a frequency sweep on a piezo to be performed, and dropouts aren't acceptable.

Environment (please complete the following information):

  • Windows 10
  • gnuarmemb toolchain
  • master branch

Additional context
I am specificially using the blackpill_f411ce board for these tests, but have disco and nucleo boards available to test if required.

@bpbradley bpbradley added the bug The issue is a bug, or the PR is fixing a bug label Sep 10, 2020
@erwango erwango added platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug labels Sep 10, 2020
@bpbradley
Copy link
Contributor Author

I can now confirm the behavior is identical on stm32f411e_disco.

The behavior is correct on nrf52840 (however I discovered a different issue on there that causes a device hang after a long time)

@erwango
Copy link
Member

erwango commented Sep 10, 2020

^^@gmarull

@erwango erwango self-assigned this Sep 10, 2020
@bpbradley
Copy link
Contributor Author

bpbradley commented Sep 10, 2020

I am not sure if this is related, but I am also noticing that the frequency resolution is really low. Trying to change between 2400 and 2800 for instance doesn't actually change the output. I was hoping to achieve 10Hz resolution on the PWM output. I notice that the result of pwm_stm32_get_cycles_per_sec is surprisingly low (around 9600) which would indicate a prescaler value of 10000. The only prescaler I see is in the defconfig, and it is set to 2.

Edit: I realize the prescaler is set in the device tree, and was indeed set to 10000. Changing this to 100 appears to give the frequency resolution I was hoping for.

Surprisingly, the dropouts are significantly reduced with this change as well. It fails at < ~700Hz, but otherwise appears to work normally. So perhaps it is related in some way?

@gmarull
Copy link
Member

gmarull commented Sep 14, 2020

After some quick tests I think that the problem you are observing is related to the pre-scaler value you're using (default to 10000 unless you change it on DT). Using default prescaler I see that pulse_cycles end up reaching 0 on your sample, for which timer output is disabled (as documented by the API). You will need to adjust timer pre-scaler to meet your requirements.

@bpbradley
Copy link
Contributor Author

After some quick tests I think that the problem you are observing is related to the pre-scaler value you're using (default to 10000 unless you change it on DT). Using default prescaler I see that pulse_cycles end up reaching 0 on your sample, for which timer output is disabled (as documented by the API). You will need to adjust timer pre-scaler to meet your requirements.

Gotcha. I noticed I was still getting occasional dropouts when I lowered the prescaler to 100, but I will need to narrow down those instances specifically and make sure that is all that is happening.

@erwango erwango added priority: low Low impact/importance bug and removed priority: medium Medium impact/importance bug labels Sep 16, 2020
@erwango
Copy link
Member

erwango commented Sep 16, 2020

Lowering bug priority.
@bpbradley don't hesistate to keep us informed is the issue is no more valid.

@erwango erwango added the Waiting for response Waiting for author's response label Sep 16, 2020
@bpbradley
Copy link
Contributor Author

@erwango sorry for the delay in looking back into this.

#29619 has resolved this issue officially. I no longer experience any dropouts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: low Low impact/importance bug Waiting for response Waiting for author's response
Projects
None yet
Development

No branches or pull requests

3 participants