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

Readd TIM21/TIM22 to l0xx series #659

Merged
merged 3 commits into from Dec 22, 2021
Merged

Readd TIM21/TIM22 to l0xx series #659

merged 3 commits into from Dec 22, 2021

Conversation

jamwaffles
Copy link
Contributor

@jamwaffles jamwaffles commented Nov 14, 2021

This is a partial revert of 0cb0f60 which I believe removed this line in error.

AN4013 Table 3. marks TIM21 and TIM22 as existing for nearly all l0xx devices so I think this PR is correct.

It does however state that TIM22 isn't available on l0x0 devices, but I'm not sure how to approach that change in this repo, so any guidance would be appreciated if we need to add that as a special case. The SVD file appears to have it present for even the l0x0 devices so maybe we don't need to worry about that.

This branch has been tested on a local copy of stm32l0xx-hal which compiles with the changes here.

@github-actions
Copy link

Memory map comparison

@adamgreig
Copy link
Member

Thanks for this PR, sorry it sat untouched for a while!

Is there a reason to not just include tim21.yaml in the relevant device files (or even in tim_l0.yaml) but instead copy the contents in? And, it looks like you've removed the existing TI2_RMP field, but that's present on TIM21's OR in the reference manual.

@jamwaffles
Copy link
Contributor Author

No worries! I'll take a look at implementing your comments tonight if I have time :)

Is there a reason to not just include tim21.yaml in the relevant device files (or even in tim_l0.yaml) but instead copy the contents in?

IIRC there were some issues but I don't recall what. I'll try that approach again and comment here if I refresh my memory.

And, it looks like you've removed the existing TI2_RMP field, but that's present on TIM21's OR in the reference manual.

Could you clarify "but that's present on TIM21's OR in the reference manual"? I can't make sense of that statement :/

This also removes the CKD from tim21.yaml - this is already included in
tim_basic.yaml
Comment on lines -5 to -8
CKD:
Div1: [0, "t_DTS = t_CK_INT"]
Div2: [1, "t_DTS = 2 × t_CK_INT"]
Div4: [2, "t_DTS = 4 × t_CK_INT"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As mentioned in the commit comment, this is already present in tim_basic.yaml. tim21.yaml is only used in tim_l0 currently so I don't think this should cause any issues.

@jamwaffles
Copy link
Contributor Author

Changes made, I'm now using tim21.yaml and I understand what you mean by the OR comment now - that's fixed too!

@github-actions
Copy link

github-actions bot commented Dec 8, 2021

Memory map comparison

@github-actions
Copy link

Memory map comparison

Copy link
Member

@adamgreig adamgreig left a comment

Choose a reason for hiding this comment

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

Thanks!

bors merge

@bors bors bot merged commit 0248de5 into stm32-rs:master Dec 22, 2021
@jamwaffles jamwaffles deleted the reinstate-l0-tim21 branch June 16, 2022 06:41
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.

None yet

3 participants