bcm283x: Don't override previous pull states

[Reflejo]

Currently the pull resistors are being overridden when a new GPIO is set on the same byte space.

This case:

rpi.P1_12.In(gpio.PullDown, gpio.RisingEdge)
fmt.Printf("Pull State (Before) %s\n", rpi.P1_12.Pull())
rpi.P1_16.In(gpio.Float, gpio.NoEdge)
fmt.Printf("Pull State (After) %s %d\n", rpi.P1_12.Pull())

Will end up printing:

Pull State (Before) PullDown
Pull State (After) Float

The fix maintains the bits that are not being touched.

[codecov-commenter]

Codecov Report

Merging #13 (7ec73f5) into main (d93ad22) will increase coverage by 0.0%. The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main     #13   +/-   ##
=====================================
  Coverage   33.9%   34.0%           
=====================================
  Files         49      49           
  Lines       5379    5381    +2     
=====================================
+ Hits        1826    1828    +2     
  Misses      3427    3427           
  Partials     126     126           

Impacted Files	Coverage Δ
bcm283x/gpio.go	50.5% <100.0%> (+0.2%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d93ad22...7ec73f5. Read the comment docs.

[maruel]

gohci

[maruel]

Oh the errors are not your fault, you can ignore. I want to double check something in the datasheet before committing but I think it's fine as is. It's impressive that this bug has lasted for so long.

[Reflejo]

@maruel I don't have a board to test the other branch but it looks like the legacy case is also broken

[berryboy2012]

Hi, nice work, BTW, could you please help fix some links in comments? I copied some discussion in slack to here for your convenience.

At #L409, https://github.com/raspberrypi/documentation/blob/master/hardware/raspberrypi/bcm2711/rpi_DATA_2711_1p0.pdf has been moved and revised to https://datasheets.raspberrypi.org/bcm2711/bcm2711-peripherals.pdf (Pg. No. has also changed to around 65 and 73~76) Also there may be other comments referencing the same broken link, for example #L1031. BCM283x has many quirks that an up-to-date datasheet is really helpful when debugging, thanks in advance!

Another question, could you please give some reference on libs talked here? thx!

NOTE: I'm not familiar with bcm283x but I did notice some python GPIO libraries are doing (previous & ~3) | .. instead. I'm not sure why it needs to clear up the two least significant bits

[berryboy2012]

Another question, could you please give some reference on libs talked here? thx!

NOTE: I'm not familiar with bcm283x but I did notice some python GPIO libraries are doing (previous & ~3) | .. instead. I'm not sure why it needs to clear up the two least significant bits

nvm, https://github.com/RPi-Distro/raspi-gpio/blob/22b44e4765b4b78dc5b22394fff484e353d5914d/raspi-gpio.c#L889 seems to be a good reference, and dropping those two bits are necessary since we are doing a bitwise overwrite operation here.

[Reflejo]

Hi, nice work, BTW, could you please help fix some links in comments? I copied some discussion in slack to here for your convenience.

I rather keep this PR laser focus on this fix, can we do a new PR with the links fix?

[maruel]

Thanks for the change. It took a while because I wanted to confirm locally.

One tricky issue is that there's a subtle race condition, using an atomic compare exchange would get rid of it but it can be done as a follow up.

[maruel]

I've updated the PDF reference in c0259657944fe6732b96afa3ec6b8f516819ecec.