Conversation
- precalculate clock idle polarity and remove clockOn()/clockOff() - check for higher (unknown) modes and reject them - move internal functions down, to be grouped together
experimental/devices/bitbang/spi.go
Outdated
| s.spiConn.readAfterClockPulse = mode&spi.Mode1 == spi.Mode1 | ||
|
|
||
| // Set clock idle polarity | ||
| if mode&spi.Mode2 == spi.Mode2 { |
There was a problem hiding this comment.
It's a bit of a hack but you can do:
s.spiConn.clockIdle = gpio.Level(mode&spi.Mode2 == spi.Mode2)
Also, we would want to have the pin setup to idle state right here.
There was a problem hiding this comment.
I wouldn't normally compress the logic so much, but since the rest of the connect options are handled similarly, that sounds reasonable.
|
gohci |
experimental/devices/bitbang/spi.go
Outdated
|
|
||
| // Set clock idle polarity, ensuring an idle clock to start | ||
| s.spiConn.clockIdle = gpio.Level(mode&spi.Mode2 == spi.Mode2) | ||
| s.spiConn.sck.Out(s.spiConn.clockIdle) |
There was a problem hiding this comment.
Please check the error. Also set CS too when NoCS is not set, and MISO and MOSI as In()/Out(Low?)
There was a problem hiding this comment.
Ah ignore MISO/MOSI/CS, that's done above. Just check the error and it'll be fine.
There was a problem hiding this comment.
Actually, I'm not so sure all the pin initialization shouldn't be moved to Connect() anyway, in the realm of expected results. My assumption would be that if there is a separate Connect(), the initial SPI device creation would not modify the hardware. The rest of the codebase, however, does not appear to make that distinction. This is definitely in the nit domain.
There was a problem hiding this comment.
That's a good point.
This was done this way initially so the failure (aka an invalid pin) would occur earlier at SPI instantiation, because Connect() is called within a device driver, which would be confusing for the users.
There was a problem hiding this comment.
Yeah, that is totally understandable. Thanks for the reviewing!
|
gohci |
2 participants
Changes based on feedback from @maruel