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: fix pwm_pin_set_usec on 32 bit MCU #6998

Closed
wants to merge 1 commit into from

Conversation

tmauron
Copy link

@tmauron tmauron commented Apr 10, 2018

Fixes #6015
Signed-off-by: Thibaut Mauron [email protected]

@nashif nashif added area: PWM Pulse Width Modulation Needs review This PR needs attention from Zephyr's maintainers labels Sep 14, 2018
@nashif
Copy link
Member

nashif commented Dec 3, 2018

@MaureenHelm can you take a look at this please?

@galak
Copy link
Collaborator

galak commented Dec 6, 2018

@tmauron curious under what conditions you see this issue.

@carlescufi
Copy link
Member

@tmauron apologies for the huge delay in reviewing this PR. We are now catching up and we've added a few reviewers, hopefully this will be reviewed soon.

@carlescufi
Copy link
Member

@cvinayak @pizi-nordic can you please review this PR?

if (pulse_cycles >= ((u64_t)1 << 32)) {
return -ENOTSUP;
}
period_cycles = ((u32_t)cycles_per_sec / USEC_PER_SEC) * period;
Copy link
Collaborator

@pizi-nordic pizi-nordic Dec 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you are loosing some precision if the cycles_per_sec are not multiple of USEC_PER_SEC,
Moreover, if cycles_per_sec are less than USEC_PER_SEC, you will always end with period_cycles and pulse_cycles = 0.
Could you please elaborate where is the problem you are trying to fix?

@gamnes
Copy link
Collaborator

gamnes commented Dec 6, 2018

If #6958 - "Update PWM API for dynamic prescaler computation", 'drivers should be able to handle directly pwm_pin_set_usec', gets developed and merged, any problems that may exist with calculating period_cycles will be handled in the drivers pin_set function instead.
I suggest that issue #6958 is completed first, then we can revisit this issue if necessary when the calculation if done inside the pin_set function.

@erwango erwango removed their request for review February 28, 2019 14:17
@zephyrproject-rtos zephyrproject-rtos deleted a comment from codecov-io Jun 19, 2019
@nashif nashif closed this Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: PWM Pulse Width Modulation Needs review This PR needs attention from Zephyr's maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PWM on 32bit arch can get 0 pulse_cycle because of 64bit calculation
6 participants