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

Variable PWM #19

Closed
wants to merge 3 commits into from
Closed

Variable PWM #19

wants to merge 3 commits into from

Conversation

stylesuxx
Copy link
Contributor

Split the "variable PWM" changes from the more "general" branch.

Bluejay.asm Outdated Show resolved Hide resolved
@github-actions
Copy link

Here are the build results
bluejay-ci-26
Artifacts will only be retained for 30 days.

decode_pwm_h_2:
;IF PWM_BITS_H == 2 ; Initialize pwm dithering bit patterns
cjne A, #2, decode_pwm_h_1
mov Pwm_H_Mask, #03h
Copy link
Contributor

Choose a reason for hiding this comment

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

Here Pwm_H_Mask should be 4. Please, check line 1028

;ELSEIF PWM_BITS_H == 1
decode_pwm_h_1:
cjne A, #1, decode_pwm_h_0
mov Pwm_H_Mask, #01h
Copy link
Contributor

Choose a reason for hiding this comment

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

Here Pwm_H_Mask should be 2. Please, check line 1028

;ELSEIF PWM_BITS_H == 0
decode_pwm_h_0:
cjne A, #0, decode_pwm_h_done
mov Pwm_H_Mask, #00h
Copy link
Contributor

Choose a reason for hiding this comment

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

Here Pwm_H_Mask should be 1. Please, check line 1028

cjne A, #96, ($+5)
mov Temp2, #2 ; 96 kHz

; TODO: Simplify?
Copy link
Contributor

Choose a reason for hiding this comment

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

Please explain it is applying the formula:
PWM_BITS_H EQU (2 + MCU_48MHZ - PWM_CENTERED - PWM_FREQ)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This I can actually explain: It basically calculates the amount of H bits which then decides the PWM mode. On BB2 this will result in a value of 1-3 and BB1 0-2. Minimum PWM mode is 8 - depending on the caluclated value it will be mode 8-10 on BB1 or 9-11 on BB2.

Page 186 of Reference manual

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I didn't explain me correctly. I meant to add a comment in code to explain what it does, instead of the TODO

mov Flag_Dithering, C ; Set dithering enabled

mov A, Pwm_Bits_H
cjne A, #3, decode_pwm_h_2
; Currently 11-bit pwm is only used on targets with built-in dead time insertion
Copy link
Contributor

Choose a reason for hiding this comment

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

Here Pwm_H_Mask value should be initialized to a correct value, because it is used above

@stylesuxx
Copy link
Contributor Author

Hey @damosvil - this is just a broken out PR from Mathias, so there is no one to explain anything ;-)
This is one of the last things he was working on and to me it seems half finished - which your comments seem to imply too.

The main things I want to know is:

  1. Will this work on L MCUs too?
  2. What needs to be done to finish this up for H/X MCUs?

@stylesuxx stylesuxx linked an issue Oct 2, 2022 that may be closed by this pull request
3 tasks
@stylesuxx
Copy link
Contributor Author

Closing this in favor of #65

@stylesuxx stylesuxx closed this Feb 13, 2023
@stylesuxx stylesuxx deleted the feature/9-variable-pwm branch February 24, 2023 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Variable" PWM
4 participants