Skip to content

Conversation

dhalbert
Copy link
Collaborator

@dhalbert dhalbert commented Oct 15, 2025

(There may be other issues fixed by this; I need to check.)

Refactoring and additions done in #9385 and adafruit/samd-peripherals#46 to add SAMD51 SPITarget had a bug involving parsing a struct by value. It mostly worked but caused timing problems. My original debugging on this found that adding a short delay fixed the problem (after a Heisenbug print statement made it going away), but I needed to dig deeper as to why.

The main fix is in adafruit/samd-peripherals#47, and the changes here are to accommodate that and to do some other cleanup.

  • Signatures of shared_dma_transfer_*() routines from samd-peripherals changed.
  • Use new NO_DMA_CHANNEL to indicate an invalid DMA channel.
  • dma_allocate_*() routine name changes.
  • Add direction handling in setup_pin(), and move it to avoid forward declaration.

I am making this a draft to start. It fixes #10663 but I want to test for regression in audio DMA use and also re-test SPITarget on SAMD. I will un-draft it when that testing is done. I will also use the merge commit for adafruit/samd-peripherals#47 when that is merged.

@Randall-Scharpf if you would be able to re-test SPITarget in your use cases that would be great, thanks.

EDIT: I tested audioio and audiobusio on a Metro M4, playing a stereo wav file a few seconds long. Both worked fine.

@dhalbert
Copy link
Collaborator Author

This also seems to fix #10669 (discovered during #10645), but I'd rather do re-enabling USB SD card presentation in a separate PR in case there is some other issue.

@anecdata
Copy link
Member

Initial SPITarget tests still showing issues with the data flowing both ways.

@dhalbert
Copy link
Collaborator Author

dhalbert commented Oct 15, 2025

Initial SPITarget tests still showing issues with the data flowing both ways.

So I should test with the #10504 programs? Do you see any difference in behavior? Thanks.

EDIT: In your testing, are both sides this PR build?

@anecdata
Copy link
Member

anecdata commented Oct 15, 2025

Yes, both sides are M4 running PR artifact. I don't see a difference in behavior, there are patterns to the data, it's not fully random, but I can't discern the pattern. But neither the controller nor target receive what was ostensibly sent.

Target code is like the last comment in 10504, but I added an explicit delay between write and read, and even with BUFLEN = 2 I can't get good data either way except under very strict circumstances (see 10504).

Depending on BUFLEN and delay between write and read, the controller seems to read back at least a byte of what it sent to the target rather than what the target was supposed to return.

controller code.py.
import time
import board
import busio
import digitalio

BUFLEN = 2
BAUDRATE = 250_000
INTERVAL = 1
DELAY = 1

time.sleep(3)  # wait for serial after reset

#### Feather M4
spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
cs = digitalio.DigitalInOut(board.D5)
cs.switch_to_output(value=True)

print(f"{time.monotonic():.03f} SPI Controller {spi.frequency}Hz")

while True:
    for mo in range(65, 91):  # ASCII A-Z
        while not spi.try_lock():
            pass
        # default baudrate=100_000, polarity=0, phase=0, bits=8
        spi.configure(baudrate=BAUDRATE)
        print(f"{time.monotonic():.03f} {spi.frequency}Hz", end=" ")

        cs.value = False
        mo_buf = bytearray(BUFLEN * [mo])
        mi_buf = bytearray(BUFLEN)
        # spi.write_readinto(mo_buf, mi_buf)
        spi.write(mo_buf)
        time.sleep(DELAY)
        spi.readinto(mi_buf)
        cs.value = True

        spi.unlock()
        print(f"{time.monotonic():.03f} mo_len={len(mo_buf)} {mo_buf=} mi_len={len(mi_buf)} {mi_buf=}")
        time.sleep(INTERVAL)
target code.py (eventually hardfaults)
import time
import board
from spitarget import SPITarget

BUFLEN = 2
DELAY = 1

time.sleep(3)
print(f"{time.monotonic():.03f} SPI Target")

with SPITarget(sck=board.D12, mosi=board.D13, miso=board.D11, ss=board.D10) as device:
    while True:
        for so in range(97, 123):  # ASCII a-z
            si_buf = bytearray(BUFLEN)
            so_buf = bytearray(BUFLEN * [so])
            device.load_packet(si_buf, so_buf)
            while not device.wait_transfer(timeout=-1):
                pass
            time.sleep(DELAY)
            print(f"{time.monotonic():.03f} si_len={len(si_buf)} {si_buf=} so_len={len(so_buf)} {so_buf=}")  ####

@anecdata
Copy link
Member

I was in contact with a colleague I believe of Randall-Scharpf on Discord and he started to look into it: https://discord.com/channels/327254708534116352/537365702651150357/1419918988166828073

@dhalbert
Copy link
Collaborator Author

dhalbert commented Oct 15, 2025

DMA is only used for transfers >= 16 bytes, so if you test with shorter transactions, then the DMA code won't be invoked at all, and that would point to some more fundamental problems with SPITarget.

I had forgotten about #10504, but I wouldn't hold this PR up for to attempt a fix for that in any case: this fixes some higher priority things.

@anecdata
Copy link
Member

Right, this PR seems to be independent. I did try some transfers > 16 bytes, but no help, so it does seem there is something separate and more fundamental going on with SPITarget.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

I don't see any issues with this and just merged in the peripherals changes.

@dhalbert dhalbert marked this pull request as ready for review October 16, 2025 18:06
@dhalbert dhalbert requested a review from tannewt October 16, 2025 18:06
@dhalbert
Copy link
Collaborator Author

@tannewt All set for final approval. I updated the samd-peripherals commit and re-tested on PyPortal.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit 3d94daa into adafruit:main Oct 16, 2025
131 checks passed
@dhalbert dhalbert deleted the spi-dma-fix branch October 16, 2025 18:36
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.

sdcardio: "OSError: can't set 512 block size" for some 32GB cards after 9.2.4

3 participants