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.

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

Add STM32L432 support and STM32L4 series EEPROM emulation #15050

Closed
wants to merge 37 commits into from
Closed

Add STM32L432 support and STM32L4 series EEPROM emulation #15050

wants to merge 37 commits into from

Conversation

lokher
Copy link
Contributor

@lokher lokher commented Nov 4, 2021

Description

  • Add STM32L432 MCU support
  • Add EEPROM emulation for STM32L4 series

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

KeychronMacro and others added 19 commits October 18, 2021 13:12
@KeychronMacro
Add a new led driver for Keychron's keyboards.
b08389c
@KeychronMacro
Update ckled2001.c/ckled2001.h.
c94ccf6
@KeychronMacro
Update ckled2001.c/ckled2001.h.
8c68c25
@KeychronMacro
Update ckled2001.c/ckled2001.h.
e5ff8ad
@KeychronMacro
Update ckled2001.c
1e7f20d
@KeychronMacro
Add a new led driver
1aabe7b
@KeychronMacro
Merge branch 'develop' of https://github.com/lalalademaxiya1/qmk_firm…
4bd7665 
…ware into develop
@keychron-dev
Merge pull request #1 from lalalademaxiya1/develop
c6b6a8b 
Add new led driver for Keychron's keyboards.
@KeychronMacro
Update ckled2001.c
9337189
@KeychronMacro
Update ckled2001.c
bdafa94
@KeychronMacro
Update ckled2001.c
4459ddf
@KeychronMacro
Update ckled2001.c
9a94daf
@KeychronMacro
Delete ckled2001.c
68f7cd5
@KeychronMacro
Create ckled2001.c
15c4cc0
@KeychronMacro
Update ckled2001.c
067e0d5
@lokher
Merge branch 'qmk:develop' into develop
7a2c8e5
@lokher
Merge branch 'develop' of https://github.com/keychron/qmk_firmware in…
49f6ee4 
…to develop
@lokher
Update chibios-contrib
acfd125
@lokher
Add STM32L432 support and STM32L4 series EEPROM emulation
1070360
sigprof
sigprof requested changes Nov 4, 2021
View reviewed changes
tmk_core/common/chibios/eeprom_stm32_l4.c Outdated Show resolved Hide resolved
tmk_core/common/chibios/eeprom_stm32_l4.c Outdated Show resolved Hide resolved
tmk_core/common/chibios/eeprom_stm32_l4.c Outdated Show resolved Hide resolved
tmk_core/common/chibios/eeprom_stm32_l4.c Outdated Show resolved Hide resolved
tmk_core/common/chibios/eeprom_stm32_l4.c Outdated Show resolved Hide resolved
tmk_core/common/chibios/eeprom_stm32_l4.c Outdated Show resolved Hide resolved
tmk_core/common/chibios/eeprom_stm32_l4.c Outdated Show resolved Hide resolved
@sigprof
Copy link
Contributor

sigprof commented Nov 4, 2021

And the mismatch of the “Word” meaning between EEPROM_*() functions (where Word is 16-bit) and Flash_*() functions (where Word is 32-bit, and a 16-bit quantity is HWord) is really unfortunate; but the existing STM32 code is already like that.

@lokher lokher requested a review from sigprof November 5, 2021 02:27
@lokher @sigprof
Update tmk_core/common/chibios/eeprom_stm32_l4.c
720eb9a 
Co-authored-by: Sergey Vlasov <sigprof@gmail.com>
@drashna
Copy link
Member

drashna commented Feb 8, 2022

Why are you completely deleting necessary files?

@lokher lokher requested a review from tzarc February 8, 2022 04:23
@lokher
Copy link
Contributor Author

lokher commented Feb 8, 2022

These files already exist in latest develop branch.

tzarc
tzarc requested changes Feb 8, 2022
View reviewed changes
Copy link
Member

@tzarc tzarc left a comment

Choose a reason for hiding this comment

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

Still would like to see unit tests validating the algorithm.

platforms/chibios/flash_stm32.c Show resolved Hide resolved
platforms/chibios/flash_stm32.c Show resolved Hide resolved
platforms/chibios/flash_stm32.h Show resolved Hide resolved
@tzarc tzarc requested a review from a team February 8, 2022 22:44
@lokher lokher requested review from tzarc and removed request for a team February 10, 2022 01:14
@tzarc tzarc mentioned this pull request Mar 28, 2022
Closed
14 tasks
@lokher lokher mentioned this pull request Apr 5, 2022
Closed
3 tasks
@stale
Copy link

stale bot commented Apr 16, 2022

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@lorthirk
Copy link

Hey, any update on this one?

@tzarc
Copy link
Member

tzarc commented Apr 20, 2022 •
edited
Loading

Hey, any update on this one?

@lorthirk doesn't seem to be much progress from Keychron -- looks like they're content just using their fork instead.
Getting to the stage where QMK has to handle it themselves -- the requested unit tests don't seem to be in the pipeline. Somewhat annoying.

@tzarc tzarc mentioned this pull request May 3, 2022
Merged
16 tasks
@leon-titan
Copy link

waiting for update

xyzz
xyzz reviewed May 18, 2022
View reviewed changes
platforms/chibios/eeprom_stm32_l4.c
Comment on lines +538 to +539
EEPROM_WriteDataByte((uintptr_t)dest++, *src++);
write_len = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a bug here where you end up incrementing dest and src twice, first the inline post-increment and then when you do dest/src += write_len at the end of the loop

@stale stale bot removed the awaiting changes label May 18, 2022
zykrah added a commit to zykrah/vial-qmk that referenced this pull request May 18, 2022
@zykrah
Fixed EEPROM bug
65a9350 
Based on: qmk/qmk_firmware#15050 (review)
@tzarc
Copy link
Member

tzarc commented Jul 11, 2022

This PR has been superseded by #16996 and #17376.
As previously suggested, QMK decided to handle this themselves -- including the requested unit tests that weren't added here.

@tzarc tzarc closed this Jul 11, 2022
@KeychronMacro KeychronMacro deleted the stm32l432 branch December 12, 2022 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xyzz xyzz xyzz left review comments

@drashna drashna drashna approved these changes

@sigprof sigprof Awaiting requested review from sigprof

@zvecr zvecr Awaiting requested review from zvecr

@tzarc tzarc Awaiting requested review from tzarc

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

Successfully merging this pull request may close these issues.