Skip to content

Bug: digitalWrite() incorrectly uses DIRSET register instead of DIR register for pin direction checking #727

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
jacoblgit opened this issue Mar 2, 2025 · 0 comments · May be fixed by #728

Comments

@jacoblgit
Copy link

jacoblgit commented Mar 2, 2025

Bug Description

The current implementation of digitalWrite() in the SAMD core incorrectly uses the DIRSET register to determine if a pin is configured as an output, which could lead to inconsistent behavior.

Technical Details

In wiring_digital.c, the function uses the following condition to check if a pin is configured as an output:

if ((PORT->Group[port].DIRSET.reg & pinMask) == 0)
{
  // the pin is not an output, disable pull-up if val is LOW, otherwise enable pull-up
  PORT->Group[port].PINCFG[pin].bit.PULLEN = ((ulVal == LOW) ? 0 : 1);
}

However, according to the SAMD21 datasheet (section 23.8.3), DIRSET is designed to be a write-only register and does not guarantee it matches actual pin direction state. This will cause the function to sometimes incorrectly attempt configuring pull-up resistors even on pins configured as outputs.

Proposed Fix

Replace the DIRSET check with a DIR check, which correctly reflects the current pin direction state:

if ((PORT->Group[port].DIR.reg & pinMask) == 0)
{
  // [rest of the code unchanged]
}

Impact Assessment

This bug has likely been present for some time without causing major functional issues, because

  1. setting pull-up configuration on output pins has minimal effect (output drivers override pull-ups).
  2. modifying DIRCLR and DIRTGL registers updates DIRSET so it aligns with DIR.
  3. Testing shows DIRSET tracks DIR even under circumstances not guaranteed by the datasheet

While not a high impact issue, it's technically incorrect and could potentially cause subtle issues in certain edge cases or confuse developers examining the behavior of the function.

jacoblgit added a commit to jacoblgit/ArduinoCore-samd that referenced this issue Mar 2, 2025
Replace DIRSET with DIR register when checking if a pin is
configured as output in digitalWrite(). DIR is the designated
register for reading current pin direction state.

According to the SAMD21 datasheet, DIRSET is designed as a
write-only register without guaranteed read behavior, while
DIR explicitly represents the current direction state.

Fixes arduino#727
@jacoblgit jacoblgit linked a pull request Mar 2, 2025 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant