Skip to content

Remove extraneous bitwise assignment operation on NRF5X_common gpio.c #20737

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
Merged
merged 1 commit into from
Jun 9, 2024
Merged

Remove extraneous bitwise assignment operation on NRF5X_common gpio.c #20737

merged 1 commit into from
Jun 9, 2024

Conversation

steverpalmer
Copy link
Contributor

Contribution description

Removing an extraneous bitwise assignment operation for NRF5X_common gpio.c

Testing procedure

I've been running this on my micro-bit v2 with this fix for a couple of days with no ill effects. However, it is not expected that the changed the behaviour of the code at all.

On my code base, it saves a whole 16 bytes of code :-).

Issues/PRs references

Fixes #20736

@steverpalmer steverpalmer requested a review from aabadie as a code owner June 8, 2024 12:05
@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Jun 8, 2024
@dylad
Copy link
Member

dylad commented Jun 8, 2024

Thanks for your PR !
While we are at it, would you mind also fix the same issue a few lines above ?

@steverpalmer
Copy link
Contributor Author

@dylad I've made the change as requested (I hope).

I've also search every file in the codebase looking for lines that include the character sequences "INTENSET" and "|=" and not found any. Of course, such searches are no guarantee, but I doubt that this is a systemic issue across the code base, just a bit of muscle-memory gone wrong in this one file.

Saves another 16 bytes ;-)

@dylad
Copy link
Member

dylad commented Jun 9, 2024

@steverpalmer this looks good to me.
Please squash into a single commit as these changes are on the same file.

@steverpalmer
Copy link
Contributor Author

Hopefully now squashed.

@dylad
Copy link
Member

dylad commented Jun 9, 2024

Great ! One last thing I promise.
Could you rename your commit as per our commit convention ?
Something like 'cpu/nrf5x: fix errorneous mask with INTENSET reg'
You may add the issue reference in the commit description (the second -m option when using 'git commit')

@steverpalmer
Copy link
Contributor Author

Third attempt luck? Cheers.

@dylad dylad added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 9, 2024
dylad
dylad approved these changes Jun 9, 2024
View reviewed changes
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

ACK.

@riot-ci
Copy link

riot-ci commented Jun 9, 2024

Murdock results

✔️ PASSED

2164352 cpu/nrf5x: fix erroneous mask with INTENSET reg

Success Failures Total Runtime
10160 0 10161 27m:45s

Artifacts

@dylad dylad added this pull request to the merge queue Jun 9, 2024
Merged via the queue into RIOT-OS:master with commit 8156bb8 Jun 9, 2024
@dylad
Copy link
Member

dylad commented Jun 10, 2024

Thanks for your fix !

@mguetschow mguetschow added this to the Release 2024.07 milestone Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylad dylad dylad approved these changes

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Erroneous, though benign, bit operation for nrf5x gpio
4 participants
@steverpalmer @dylad @riot-ci @mguetschow