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

Use QUANTUM_LIB_SRC for i2c_master.c inclusion #5617

Merged
merged 1 commit into from
Jul 9, 2019
Merged

Use QUANTUM_LIB_SRC for i2c_master.c inclusion #5617

merged 1 commit into from
Jul 9, 2019

Conversation

drashna
Copy link
Member

@drashna drashna commented Apr 14, 2019

Using QUANTUM_LIB_SRC prevents the warning when multiple sources add the i2c_master.c file. Boards such as the Ergodox EZ Glow see this warning every time they compile because the board uses the file in general, and because the RGB LED Matrix requires it, as well.

Types of Changes

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

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@drashna
Copy link
Member Author

drashna commented Apr 15, 2019

Oh boy. It looks like this is not working for ARM, at all. All of the ARM keyboards that use i2c_master.c for LEDs are erroring out after this:

Making cannonkeys/satisfaction75/prototype with keymap default                                         [ERRORS]
 | 
 | arm-none-eabi-ar: invalid option -- '.'
 | Usage: arm-none-eabi-ar [emulation options] [-]{dmpqrstx}[abcDfilMNoPsSTuvV] [--plugin <name>] [member-name] [count] archive-file file...

I'm not sure how to fix this.... at all.

And worse, if this is the case, then it WILL break ARM support for split keyboards. Eg, we will run into this issue at that time.

Tagging @mtei, since you introduced the QUANTUM_LIB_SRC functionality. And tagging @pelrun, just to make aware of the issue.

@pelrun
Copy link
Contributor

pelrun commented Apr 15, 2019

Sussed it; tmk_core/chibios.mk doesn't configure the ar tool correctly.

Line 146 needs to be AR = arm-none-eabi-ar rcs (i.e. add the 'rcs' at the end.)

@mtei mtei mentioned this pull request Apr 15, 2019
Merged
13 tasks
@mtei
Copy link
Contributor

mtei commented Apr 15, 2019

What pelrun says is correct. I opened #5623.

mtei
mtei requested changes Apr 24, 2019
View reviewed changes
common_features.mk Show resolved Hide resolved
common_features.mk Show resolved Hide resolved
common_features.mk Show resolved Hide resolved
common_features.mk Show resolved Hide resolved
common_features.mk Show resolved Hide resolved
drivers/qwiic/qwiic.mk Outdated Show resolved Hide resolved
keyboards/ergodox_ez/rules.mk Outdated Show resolved Hide resolved
@drashna
Copy link
Member Author

drashna commented Apr 25, 2019

Updated based on @mtei's feedback. Thanks!

@mtei
Copy link
Contributor

mtei commented Apr 25, 2019

Sorry, I made a mistake. One of the following actions is required.

  • Modify build_keyboard.mk as follows.
diff --git a/build_keyboard.mk b/build_keyboard.mk
index 5d633f271..469a947f5 100644
--- a/build_keyboard.mk
+++ b/build_keyboard.mk
@@ -324,7 +324,6 @@ ifneq ("$(wildcard $(KEYMAP_PATH)/config.h)","")
 endif
 
 # # project specific files
-SRC += $(patsubst %.c,%.clib,$(LIB_SRC))
 SRC += $(KEYBOARD_SRC) \
     $(KEYMAP_C) \
     $(QUANTUM_SRC)
@@ -343,6 +342,7 @@ include $(TMK_PATH)/protocol.mk
 include $(TMK_PATH)/common.mk
 include bootloader.mk
 
+SRC += $(patsubst %.c,%.clib,$(LIB_SRC))
 SRC += $(patsubst %.c,%.clib,$(QUANTUM_LIB_SRC))
 SRC += $(TMK_COMMON_SRC)
 OPT_DEFS += $(TMK_COMMON_DEFS)
  • Or change LIB_SRC in common_features.mk to QUANTUM_LIB_SRC.

@drashna drashna force-pushed the fix/i2c_master_calls branch 2 times, most recently from b90581d to 21872d8 Compare April 25, 2019 01:14
@mtei
Copy link
Contributor

mtei commented Apr 29, 2019

LIB_SRC and SRC cannot be used in pairs now, so I opened #5731 because it is considered a bug.

@drashna drashna force-pushed the fix/i2c_master_calls branch 2 times, most recently from ad7421c to 3c484b6 Compare July 8, 2019 23:08
@drashna
Use QUANTUM_LIB_SRC for i2c_master.c inclusion
1508c48 
Using QUANTUM_LIB_SRC prevents the warning when multiple sources add the i2c_master.c file. Boards such as the Ergodox EZ Glow see this warning every time they compile because the board uses the file in general, and because the RGB LED Matrix requires it, as well.
@drashna drashna mentioned this pull request Jul 9, 2019
Closed
mechmerlin
mechmerlin approved these changes Jul 9, 2019
View reviewed changes
Copy link
Contributor

@mechmerlin mechmerlin left a comment

Choose a reason for hiding this comment

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

Yay!

@mechmerlin mechmerlin merged commit 6cccc22 into qmk:master Jul 9, 2019
@drashna drashna deleted the fix/i2c_master_calls branch July 9, 2019 17:26
drashna added a commit to zsa/qmk_firmware that referenced this pull request Jul 9, 2019
@drashna
Use QUANTUM_LIB_SRC for i2c_master.c inclusion (qmk#5617)
8e4408a
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Jul 10, 2019
@drashna @fdidron
Use QUANTUM_LIB_SRC for i2c_master.c inclusion (qmk#5617)
dba5ec7
doughsay pushed a commit to doughsay/qmk_firmware that referenced this pull request Aug 31, 2019
@drashna @doughsay
Use QUANTUM_LIB_SRC for i2c_master.c inclusion (qmk#5617)
87f4b33 
Using QUANTUM_LIB_SRC prevents the warning when multiple sources add the i2c_master.c file. Boards such as the Ergodox EZ Glow see this warning every time they compile because the board uses the file in general, and because the RGB LED Matrix requires it, as well.
ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
@drashna
Use QUANTUM_LIB_SRC for i2c_master.c inclusion (qmk#5617)
353db05 
Using QUANTUM_LIB_SRC prevents the warning when multiple sources add the i2c_master.c file. Boards such as the Ergodox EZ Glow see this warning every time they compile because the board uses the file in general, and because the RGB LED Matrix requires it, as well.
ridingqwerty pushed a commit to ridingqwerty/qmk_firmware that referenced this pull request Jan 10, 2020
@drashna @mechmerlin
Use QUANTUM_LIB_SRC for i2c_master.c inclusion (qmk#5617)
3d3f462 
Using QUANTUM_LIB_SRC prevents the warning when multiple sources add the i2c_master.c file. Boards such as the Ergodox EZ Glow see this warning every time they compile because the board uses the file in general, and because the RGB LED Matrix requires it, as well.
JeffreyPalmer pushed a commit to JeffreyPalmer/qmk_firmware that referenced this pull request Feb 27, 2020
@drashna @JeffreyPalmer
Use QUANTUM_LIB_SRC for i2c_master.c inclusion (qmk#5617)
1dc3e23 
Using QUANTUM_LIB_SRC prevents the warning when multiple sources add the i2c_master.c file. Boards such as the Ergodox EZ Glow see this warning every time they compile because the board uses the file in general, and because the RGB LED Matrix requires it, as well.
swamp09 pushed a commit to swamp09/qmk_firmware that referenced this pull request Mar 11, 2020
@drashna @swamp09
Use QUANTUM_LIB_SRC for i2c_master.c inclusion (qmk#5617)
657756b 
Using QUANTUM_LIB_SRC prevents the warning when multiple sources add the i2c_master.c file. Boards such as the Ergodox EZ Glow see this warning every time they compile because the board uses the file in general, and because the RGB LED Matrix requires it, as well.
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@drashna @mechmerlin
Use QUANTUM_LIB_SRC for i2c_master.c inclusion (qmk#5617)
7803589 
Using QUANTUM_LIB_SRC prevents the warning when multiple sources add the i2c_master.c file. Boards such as the Ergodox EZ Glow see this warning every time they compile because the board uses the file in general, and because the RGB LED Matrix requires it, as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtei mtei mtei requested changes

@jackhumbert jackhumbert jackhumbert approved these changes

@XScorpion2 XScorpion2 XScorpion2 approved these changes

@mechmerlin mechmerlin mechmerlin approved these changes

@algernon algernon Awaiting requested review from algernon

@ezuk ezuk Awaiting requested review from ezuk

@yanfali yanfali Awaiting requested review from yanfali

@skullydazed skullydazed Awaiting requested review from skullydazed

@fredizzimo fredizzimo Awaiting requested review from fredizzimo

@noroadsleft noroadsleft Awaiting requested review from noroadsleft

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

Successfully merging this pull request may close these issues.
6 participants
@drashna @pelrun @mtei @jackhumbert @XScorpion2 @mechmerlin