Skip to content

Conversation

eliasnaur
Copy link
Contributor

Found while investigating https://github.com/orgs/tinygo-org/discussions/4936.

Untested.

@eliasnaur eliasnaur requested a review from soypat June 27, 2025 18:52
Copy link
Contributor

@soypat soypat left a comment

Choose a reason for hiding this comment

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

Why are we locking? Is this due to new multicore semantics?

@eliasnaur
Copy link
Contributor Author

I guess so. And because the ADC is shared between the analog pins, it's reasonable to support separate goroutines using it simultaneously.

@soypat
Copy link
Contributor

soypat commented Jun 28, 2025

Either we test it or we could do a side-by-side comparison of existing pico-sdk code... or a datasheet walkthrough would work

@eliasnaur
Copy link
Contributor Author

@soypat
Copy link
Contributor

soypat commented Jun 29, 2025

I've noted it is tested but exhibits no change to behaviour so I'm still curious as to what this CL solves.

@eliasnaur
Copy link
Contributor Author

The patch does 2 things:

  • Move the reading of the ADC inside the lock, on the basis that if the lock is needed, the actual reading the value should be locked as well.
  • Use rp.ADC_CS_AINSEL_Msk instead of the hard-coded 0b111 mask, which is wrong for rp2350.

@soypat
Copy link
Contributor

soypat commented Jun 29, 2025

Alright, LGTM in that case! Thank you for the contribution @eliasnaur !

@soypat soypat merged commit c6b47fe into tinygo-org:dev Jun 29, 2025
27 of 29 checks passed
@eliasnaur eliasnaur deleted the push-orwunllvutmq branch June 29, 2025 18:31
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.

2 participants