Skip to content

Add stm32l4r5 #703

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

Add stm32l4r5 #703

wants to merge 6 commits into from

Conversation

gauteh
Copy link

@gauteh gauteh commented Feb 26, 2022 •
edited
Loading

  • add stm32l4r5 device
  • rename CH7WDATR (the second) to CH7DATINR
  • l4r5: copy ADC1 from l4x5

@github-actions
Copy link

Memory map comparison

@gauteh gauteh mentioned this pull request Feb 26, 2022
Open
@newAM newAM added the stm32l4 label Feb 26, 2022
@adamgreig
Copy link
Member

So far this looks pretty reasonable to me, let me know if you have any questions or any problems as you get it ready!

@gauteh gauteh marked this pull request as ready for review February 27, 2022 08:05
@gauteh
Copy link
Author

gauteh commented Feb 27, 2022

Thanks, I think it is good, I'll do a bit more testing and report back. But should otherwise be ready (for review) from my side.

@github-actions
Copy link

Memory map comparison

@gauteh
Copy link
Author

gauteh commented Feb 27, 2022

ADC peripheral naming:

  • I think it only has one ADC, so maybe the interrupt should be named ADC1 as well.
  • The r9 has peripheral ADC (not ADC1), but naming it ADC1 aligns better with interrupt name and makes it slightly more generic for the HAL.

Should I keep ADC1 as name for the ADC and rename the interrupt to ADC1, or change both to just ADC?

@github-actions
Copy link

Memory map comparison

@github-actions
Copy link

Memory map comparison

@gauteh
Copy link
Author

gauteh commented Feb 28, 2022

Ok, I think the easiest is to go with ADC1 for the peripheral. The interrupt is already named ADC1. But it would make it more consistent to rename the ADC on the r9 to ADC1 as well (@jspngh might have objections to that).

@jspngh
Copy link
Contributor

jspngh commented Feb 28, 2022

I have no problem with renaming it to ADC1 for all L4+ devices

@github-actions
Copy link

Memory map comparison

@gauteh
Copy link
Author

gauteh commented Mar 18, 2022

Hi, I know you are all busy. I think this one is ready if you have the chance to look at it. Thanks, Gaute

richardeoin
richardeoin reviewed Apr 4, 2022
View reviewed changes
Copy link
Member

@richardeoin richardeoin left a comment

Choose a reason for hiding this comment

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

I suspect in general we should prefer to merge PRs like this, and fix errors later. But I did find a few items to fix - and might apply to other L4 parts also.

Reference manual: RM0432 rev 9

  • ADC3 interrupt when there's no adc3 peripheral, should be deleted. It's under ADC1 in the SVD, so you can fix it with something like:
ADC1:
  _delete:
    _interrupts:
      - ADC3
  • TIM1/8 OR1 registers both have a field ETR_ADC3_RMP that doesn't exist in the RM.
  • TIM1/8 OR1 register description is wrong
  • OSPI1 bits are missing from RCC AHB3ENR AHB3RSTR AHB3SMENR (at least some R5 parts have two octospi, although it is hard to tell sometimes)
  • RCC CFGR -> MCOSEL is 3 bits wide, should be 4 (and there is a code 1000 that's used for the HSI48)
  • RCC missing DLYCFGR register
  • PWR missing PDCRI PUCRI CR5 registers
  • Flash ACR -> LATENCY is 3 bits wide, should be 4
  • Flash SR missing PEMPTY bit
  • Flash ECCR bit broken, ADDR_ECC is too small and then BK_ECC SYSF_ECC at the wrong offsets
  • Flash missing CFGR register

@richardeoin
Copy link
Member

Also a missing register on LPUART1: #694

@richardeoin
Copy link
Member

@gauteh Do you plan to respond to the review feedback in this PR? This part does look like a useful one to support

@gauteh
Copy link
Author

gauteh commented May 15, 2022 via email •
edited
Loading

@mattcarp12 mattcarp12 mentioned this pull request May 28, 2022
Merged
bors bot added a commit that referenced this pull request Jun 14, 2022
@bors @mattcarp12
Merge #740
3644ce8 
740: Add Stm32l4r5 r=richardeoin a=mattcarp12

Picking up where `@gauteh` left off [here](#703).

Co-authored-by: Matt Carpenter <mattcarp88@gmail.com>
@richardeoin
Copy link
Member

Closed with #740 🎉

@gauteh gauteh mentioned this pull request Aug 7, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardeoin richardeoin richardeoin left review comments

Assignees
No one assigned
Labels
stm32l4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gauteh @adamgreig @jspngh @richardeoin @newAM