Skip to content

Add support for half-duplex SPI to CPy#5990

Merged
dhalbert merged 6 commits into
adafruit:mainfrom
sgauche:stm_spi_3wire
Feb 9, 2022
Merged

Add support for half-duplex SPI to CPy#5990
dhalbert merged 6 commits into
adafruit:mainfrom
sgauche:stm_spi_3wire

Conversation

@sgauche

@sgauche sgauche commented Feb 8, 2022

Copy link
Copy Markdown

I have a need for half-duplex SPI on a custom board. I thought it would be worth adding to CircuitPython too. I have tested this on the hardware and it is working.

tannewt
tannewt previously requested changes Feb 8, 2022

@tannewt tannewt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it'd be good to add an explicit way to setup half duplex and document it in the matching portion of shared-bindings. I think it's best to provide an explicit half duplex bool. I'm worried that the majority of folks do want read to mean read from MISO and removing the exception will make it less clear what their coding error is.

It crossed my mind to make mosi == miso mean half duplex but I suspect that'd be a common copy and paste bug.

@sgauche

sgauche commented Feb 8, 2022

Copy link
Copy Markdown
Author

I did think about that... instead of using if MISO and MOSI are defined, just let the user specify SPI_DIRECTION_2LINES_RXONLY, SPI_DIRECTION_2LINES, or SPI_DIRECTION_1LINE via a parameter when calling spi configure like:
spi.configure(baudrate=5000000, com_mode=[full, half, simplex], phase=0, polarity=0)

Would something like that be possible?

There are 3 modes, full-duplex (normal 4-wire SPI), half-duplex (3-wire SPI with bidirectional data), and simplex (3-wire SPI with unidirectional data, RX only). Cpy was already set up to auto set SPI_DIRECTION_2LINES/full-duplex and SPI_DIRECTION_2LINES_RXONLY/simplex based on MISO/MOSO being defined. I don't think we can use a boolean, since there are 3 modes, not 2.

@sgauche

sgauche commented Feb 8, 2022

Copy link
Copy Markdown
Author

I looked at that but only saw ways to pass bools and uints through the configure parameters, didn't see a way to pass strings.

@sgauche

sgauche commented Feb 8, 2022

Copy link
Copy Markdown
Author

Now I'm realizing if we change the spi configure parameters, that touches all of the different ports, not just stm... hmm.

@dhalbert

dhalbert commented Feb 8, 2022

Copy link
Copy Markdown
Collaborator

Now I'm realizing if we change the spi configure parameters, that touches all of the different ports, not just stm... hmm.

Right, we need to change the common_hal routines even if it's not implemented.

I think a half_duplex boolean would be sufficient. You can complain if both pins are specified and half_duplex is true.

@dhalbert dhalbert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! Minor style request.

Comment thread ports/atmel-samd/common-hal/busio/SPI.c Outdated
@sgauche

sgauche commented Feb 9, 2022

Copy link
Copy Markdown
Author

I'm guessing the too little flash failures are because I added mp_raise_NotImplementedError(translate("Half duplex SPI is not implemented"));. I could just do a mp_raise_NotImplementedError(Null);?

@dhalbert

dhalbert commented Feb 9, 2022

Copy link
Copy Markdown
Collaborator

Don't worry about these "too little flash" errors. I will push some commits that squeeze the builds that are overflowing. An unlabeled "NotImplementedError" is confusing. This happens all the time when we add functionality to tiny builds.

@sgauche sgauche changed the title Add support for half-duplex SPI on STM parts Add support for half-duplex SPI to CPy Feb 9, 2022
@sgauche sgauche requested review from dhalbert and tannewt February 9, 2022 16:09

@dhalbert dhalbert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for your perseverance on this!

@dhalbert dhalbert merged commit da035fe into adafruit:main Feb 9, 2022
Comment thread shared-bindings/busio/SPI.c
@sgauche sgauche deleted the stm_spi_3wire branch February 26, 2022 17:34
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.

3 participants