Skip to content

Fix issue #5119, changed pwmout_api. #7716

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

Merged
merged 2 commits into from
Aug 15, 2018
Merged

Fix issue #5119, changed pwmout_api. #7716

merged 2 commits into from
Aug 15, 2018

Conversation

MateuszMaz
Copy link
Contributor

Description

K64F was not able to generate pwm with 100 ms long period. This PR fix the problem, by computing and setting the optimal prescaler value while setting the pwm period. This way we also don't lose accuracy for short periods, but it is even enhanced a little.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Feature
[ ] Breaking change

The period of pwm we could get was limited to 69.9 ms, because prescaler value was set once only during initialization. base->mod is a 16 bit register, to get longer period we have to slow down the clk.
@fkjagodzinski
Copy link
Member

@0xc0170 0xc0170 requested a review from a team August 7, 2018 08:29
pwm_base_clock = CLOCK_GetFreq(kCLOCK_BusClk);
pwm_clock_mhz = (float)pwm_base_clock / 1000000.0f;
uint32_t mod = (pwm_clock_mhz*(float)us) - 1;
while(mod > 0xFFFF){
Copy link
Contributor

Choose a reason for hiding this comment

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

this is target code but should be same as the rest of the code here, see https://os.mbed.com/docs/latest/reference/style.html

spaces around operators and while (), etc. Please fix

@0xc0170 0xc0170 requested a review from a team August 8, 2018 11:46
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

pwm_clock_mhz * (float)us spaces around operators, same for line 135

@MateuszMaz
Copy link
Contributor Author

@0xc0170 formatting updated

Copy link
Contributor

@mmahadevan108 mmahadevan108 left a comment

Choose a reason for hiding this comment

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

Thanks for the patch

@cmonr
Copy link
Contributor

cmonr commented Aug 14, 2018

Interesting. I'm good with the change, since imo most common applications set a fixed frequency instead of duty cycle.

@ARMmbed/mbed-os-hal Care to take a quick look?

/morph build

@mbed-ci
Copy link

mbed-ci commented Aug 14, 2018

Build : SUCCESS

Build number : 2799
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7716/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Aug 14, 2018

@cmonr
Copy link
Contributor

cmonr commented Aug 14, 2018

/morph test

@cmonr
Copy link
Contributor

cmonr commented Aug 14, 2018

/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Aug 15, 2018

@cmonr cmonr merged commit c4e814d into ARMmbed:master Aug 15, 2018
pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants