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

[build] Allow USB to be set to 0 #4619

Merged
merged 1 commit into from
Apr 24, 2021
Merged

[build] Allow USB to be set to 0 #4619

merged 1 commit into from
Apr 24, 2021

Conversation

tyomitch
Copy link
Collaborator

USB_AVAILABLE was conditionally defined in supervisor.mk,
but never actually used to #ifdef USB-related code.

Loosely related to #4546

@dhalbert
Copy link
Collaborator

I'm confused about USB vs USB_AVAILABLE. Did you find who defines USB? Could you merge the uses of these to into a single toggle, like CIRCUITPY_USB_AVAILABLE or CIRCUITPY_USB?

There's an #if USB_AVAILABLE (not #ifdef) in litex/supervisor/internal_flash.c.

It would also be good to not use #ifdef USB_AVAILABLE, but instead always define it as 0 or 1, and use #if USB_AVAILABLE (or some new name).

@tyomitch
Copy link
Collaborator Author

I'm confused about USB vs USB_AVAILABLE.

USB is a makefile variable, USB_AVAILABLE is a C definition: both were added in 9d91111, but apparently both stayed unused -- except in litex/supervisor/internal_flash.c since 786e79e, which would never work as intended because USB_AVAILABLE is either empty or undefined.

Did you find who defines USB?

None of the in-tree targets; a target would inevitably fail to compile if it did.

microbit_v2, when/if added, should become the first one.

@dhalbert
Copy link
Collaborator

USB is a makefile variable, USB_AVAILABLE is a C definition: both were added in 9d91111, but apparently both stayed unused -- except in litex/supervisor/internal_flash.c since 786e79e, which would never work as intended because USB_AVAILABLE is either empty or undefined.

Could you unify them, so it's simpler? Thanks.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

One tweak. Otherwise it looks good. Thanks!

shared-bindings/supervisor/Runtime.c Outdated Show resolved Hide resolved
Unify USB-related makefile var and C def as CIRCUITPY_USB.

Always define it as 0 or 1, same as all other settings.

USB_AVAILABLE was conditionally defined in supervisor.mk,
but never actually used to #ifdef USB-related code.

Loosely related to adafruit#4546
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thank you for cleaning this up!

@dhalbert dhalbert merged commit 2ba2574 into adafruit:main Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants