Skip to content
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.

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

ports/rp2: Optional spi_id and optional disabling of MISO pin #16922

Conversation

Gadgetoid
Copy link
Contributor

Summary

This PR includes two changes:

  1. Bring machine.SPI() into parity with machine.I2C(), selecting the default instance where none is specified. See: feature/optional_i2c_bus #16671
  2. Make machine.SPI(mosi=None) explicitly disable MISO, for configurations which use this as a register-select or data/command pin (very common for write-only LCDs). See: When miso=None is set, the default miso pin changed state. #16912

I avoided making this change for all pins, since it rarely makes sense for MOSI or SCLK to be disabled.

Testing

Tested various invocations of machine.SPI() on a Pico 2 W.

Notably this change introduces a difference in behaviour that always resets MISO back to the default unless it's specified or set to None. Eg:

>>> machine.SPI()
SPI(0, baudrate=1000000, polarity=0, phase=0, bits=8, sck=18, mosi=19, miso=16)
>>> machine.SPI(miso=None)
SPI(0, baudrate=1000000, polarity=0, phase=0, bits=8, sck=18, mosi=19, miso=None)
>>> machine.SPI()
SPI(0, baudrate=1000000, polarity=0, phase=0, bits=8, sck=18, mosi=19, miso=16)

This is materially different from, for example, mosi where it's impossible to implicitly set a pin back to its default value:

>>> machine.SPI(mosi=23)
SPI(0, baudrate=1000000, polarity=0, phase=0, bits=8, sck=18, mosi=23, miso=16)
>>> machine.SPI()
SPI(0, baudrate=1000000, polarity=0, phase=0, bits=8, sck=18, mosi=23, miso=16)
>>> machine.SPI(mosi=None)
SPI(0, baudrate=1000000, polarity=0, phase=0, bits=8, sck=18, mosi=23, miso=16)

🤔 Maybe this changeset should also require that a pin is specified or that it is otherwise always set back to its default value.

Trade-offs and Alternatives

The latter change is more about explicit vs implicit. It's possibly just to set upt he MISO pin after setting up SPI and it will work as expected. A documentation change establishing this as the accepted pattern might be an alternative... though it might not be possible in all cases for the pin setup to happen before SPI.

Consider the following very crude example driver pattern where a user wishes to save a pin (normally eaten by SPI) and reuse it for register select:

class LCD:
    def __init__(self, spi=None, rs=None):
        self._spi = spi or machine.SPI()

my_led_screen = LCD(rs=machine.Pin(16))

However because the pin is constructed before it's passed into the driver class, its mux will be overwritten by machine.SPI()

With this change, a driver author could use:

class LCD:
    def __init__(self, spi=None, rs=None):
        self._spi = spi or machine.SPI(miso=None)

my_led_screen = LCD(rs=machine.Pin(16))

And SPI would no longer trample the "rs" pin config by setting it up as MISO.

Copy link

github-actions bot commented Mar 13, 2025

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:   +72 +0.008% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@dpgeorge dpgeorge added this to the release-1.25.0 milestone Mar 13, 2025
If the "spi_id" arg is not supplied and then the board default specified
by PICO_DEFAULT_SPI will be used. If this is not set, it's defaulted to 0.

Note: spi_id is still required when MICROPY_HW_SPI_NO_DEFAULT_PINS is set.

Signed-off-by: Phil Howard <[email protected]>
@Gadgetoid Gadgetoid force-pushed the patch-rp2-machine-spi-optional-id-miso branch from 8975d9e to 1183bd0 Compare March 13, 2025 13:40
It's common with write-only SPI displays for MISO to be repurposed as
a register select or data/command pin.

While that was possible by setting up the pin after a call to
`machine.SPI()` this change makes `machine.SPI(miso=None)` explicit.

Signed-off-by: Phil Howard <[email protected]>
@Gadgetoid Gadgetoid force-pushed the patch-rp2-machine-spi-optional-id-miso branch from 1183bd0 to 6d66eef Compare March 25, 2025 15:59
@dpgeorge
Copy link
Member

Rebased and merged in a861223 and dd7a950

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants