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

boards: adafruit feather nrf52840 uf2 initalize USB at boot #80038

Closed

Conversation

fkokosinski
Copy link
Member

Make Adafruit Feather nrf52840 UF2 variants initialize USB by default.

Adafruit Feather nRF52840 UF2 variants have their USB enabled by default, and use the CDC ACM UART for the console. All other nRF52840 boards that use that, initialize their USB by default. This PR makes it so Adafruit Feather nRF52840 UF2 variants also do that.

@fkokosinski
Copy link
Member Author

CI fails are not caused by the changes from this PR. See #80215.

Adafruit Feather nRF52840 UF2 variants have their USB enabled by default,
and use the CDC ACM UART for console. All other nRF52840 boards that use
that, initialize their USB by default. This change makes it so
Adafruit Feather nRF52840 UF2 variants also do that.

Signed-off-by: Sebastian Kwaśniak <[email protected]>
Signed-off-by: Filip Kokosinski <[email protected]>
@fkokosinski fkokosinski force-pushed the adafruit-nrf52840-usb branch from ad0c6ad to a9639d7 Compare November 12, 2024 10:03
@nordicjm
Copy link
Collaborator

@jacobw can you review?

Copy link
Collaborator

@jfischer-no jfischer-no left a comment

Choose a reason for hiding this comment

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

@jacobw
Copy link
Contributor

jacobw commented Nov 13, 2024

Looks good to me.

@fkokosinski
Copy link
Member Author

See #58349 #81308

Thanks for those links @jfischer-no. I get that in the end we'd prefer to have it done in a more tidy way.

But right now we have a target that is essentially not working correctly, so IMO we should:

  1. first, go ahead with this PR which makes the target work correctly
  2. and later, clean up CDC ACM configuration later (with the PR you linked)

This way we'd avoid keeping a non-working target in-tree and waiting for the more complex effort to land in order to sort this situation out.

WDYT?

@jfischer-no
Copy link
Collaborator

See #58349 #81308

Thanks for those links @jfischer-no. I get that in the end we'd prefer to have it done in a more tidy way.

But right now we have a target that is essentially not working correctly, so IMO we should:

1. first, go ahead with this PR which makes the target work correctly

2. and later, clean up CDC ACM configuration later (with the PR you linked)

This way we'd avoid keeping a non-working target in-tree and waiting for the more complex effort to land in order to sort this situation out.

WDYT?

If you were to read the contents of the linked issue and commit messages in the linked PR, you would realize that this PR is not a correct fix at all.

@fkokosinski
Copy link
Member Author

Closing because the issue this PR was addressing has been fixed in #81308.

@fkokosinski fkokosinski closed this Jan 9, 2025
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.

7 participants