Skip to content

Fix DMA to SPI transmits on RP2350 #4903

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.

Sign up for GitHub

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

Merged
merged 16 commits into from
May 24, 2025
Merged

Fix DMA to SPI transmits on RP2350 #4903

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7a312ce
Fix DMA to SPI transfers on RP2350
hb9cwp May 22, 2025
47c8993
Add definitions for DMA DREQ "handshake" lines
hb9cwp May 22, 2025
c207e51
Add definitions for DMA DREQ "handshake" lines
hb9cwp May 22, 2025
970412c
Complete table
hb9cwp May 22, 2025
00bf8e4
Complete table
hb9cwp May 22, 2025
75cee78
Remove redundant DMA_ prefix
hb9cwp May 22, 2025
a3c7c16
Correct name of Datasheet
hb9cwp May 22, 2025
0fd8fad
Correct name of Datasheet
hb9cwp May 22, 2025
c7ed461
Refactor
hb9cwp May 22, 2025
52da289
Refactor
hb9cwp May 22, 2025
fde6365
Refactor
hb9cwp May 22, 2025
2d1bd29
Refacture
hb9cwp May 22, 2025
d2045d3
Refacture
hb9cwp May 22, 2025
27a15d2
Fix comments
hb9cwp May 22, 2025
6568712
go fmt
hb9cwp May 22, 2025
2d79fcf
rename new non-generated files
hb9cwp May 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/machine/machine_rp2_2040.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ const (
numClocks
)

// Additional definitions that are missing in generated src/device/rp/rp2040.go
const (
// from 2040 Datasheet: 2.5.3.1. System DREQ Table
DMA_DREQ_SPI0_TX = 16
DMA_DREQ_SPI0_RX = 17
DMA_DREQ_SPI1_TX = 18
DMA_DREQ_SPI1_RX = 19
Copy link
Contributor

@eliasnaur eliasnaur May 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why export these? They're useful, sure, but do they belong in the machine package from the perspective of importing packages?

Copy link
Contributor Author

@hb9cwp hb9cwp May 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why export these?

Because the generated src/device/rp/rp2040|rp2350.go don't provide them, and they are global and specific for these processors as well.

but do they belong in the machine package

What alternatives do you propose which would be a better fit?

Copy link
Member

Choose a reason for hiding this comment

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

See https://github.com/tinygo-org/tinygo/blob/release/src/device/sam/atsamd51x-bitfields.go for example of how to handle values that are not part of the files generated from the SVDs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why export these?

Because the generated src/device/rp/rp2040|rp2350.go don't provide them, and they are global and specific for these processors as well.

but do they belong in the machine package

What alternatives do you propose which would be a better fit?

For every exported API the question should be: what are the use cases for outside users? If there's no identifiable outside need, it's better to keep the API unexported and deal with isssues such as naming, other platforms etc. when the need arises.

If you or some other maintainer (@soypat?) decide to export anyway, I suggest package device/rp. The machine package is for abstractions across a wide range of hardware, and I know of no restriction that say device/rp is for machine generated code only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, device/rp would be a good place, similar to device/tkey or device/riscv for example.

Unfortunately, it is currently excluded by .gitignore. It would be helpful to have also generated *.go and *.s files in the repo that are versioned. This would facilitate searching and automatic cross-referencing, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the additional global constants for DMA to device/rp/rp2nnn0-plus.go, see refactoring below.
But I did not touch some other lines in src/machine/machine_rp2_2nnn.go that could be refactored as well, such as:

const (
	cpuFreq          = 150 * MHz
	_NUMBANK0_GPIOS  = 48
	_NUMBANK0_IRQS   = 6
	rp2350ExtraReg   = 1
	_NUMIRQ          = 51
	notimpl          = "rp2350: not implemented"
	...

Copy link
Contributor

Choose a reason for hiding this comment

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

Moved the additional global constants for DMA to device/rp/rp2nnn0-plus.go, see refactoring below. But I did not touch some other lines in src/machine/machine_rp2_2nnn.go that could be refactored as well, such as:

const (
	cpuFreq          = 150 * MHz
	_NUMBANK0_GPIOS  = 48
	_NUMBANK0_IRQS   = 6
	rp2350ExtraReg   = 1
	_NUMIRQ          = 51
	notimpl          = "rp2350: not implemented"
	...

These constants are internal implementation details. Why should they be exported?

)

func calcClockDiv(srcFreq, freq uint32) uint32 {
// Div register is 24.8 int.frac divider so multiply by 2^8 (left shift by 8)
return uint32((uint64(srcFreq) << 8) / uint64(freq))
Expand Down
9 changes: 9 additions & 0 deletions src/machine/machine_rp2_2350.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,15 @@ const (
fnNULL pinFunc = 0x1f
)

// Additional definitions that are missing in generated src/device/rp/rp2350.go
const (
// from 2350 Datasheet: 12.6.4.1. System DREQ Table
DMA_DREQ_SPI0_TX = 24
DMA_DREQ_SPI0_RX = 25
DMA_DREQ_SPI1_TX = 26
DMA_DREQ_SPI1_RX = 27
)

// Configure configures the gpio pin as per mode.
func (p Pin) Configure(config PinConfig) {
if p == NoPin {
Expand Down
4 changes: 2 additions & 2 deletions src/machine/machine_rp2_spi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,10 @@ func (spi *SPI) tx(tx []byte) error {
var dreq uint32
if spi.Bus == rp.SPI0 {
ch = &dmaChannels[spi0DMAChannel]
dreq = 16 // DREQ_SPI0_TX
dreq = DMA_DREQ_SPI0_TX
} else { // SPI1
ch = &dmaChannels[spi1DMAChannel]
dreq = 18 // DREQ_SPI1_TX
dreq = DMA_DREQ_SPI1_TX
}

// Configure the DMA peripheral as follows:
Expand Down
Loading