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

atsamd5x: improve SPI #1453

Merged
merged 1 commit into from
Mar 14, 2021
Merged

atsamd5x: improve SPI #1453

merged 1 commit into from
Mar 14, 2021

Conversation

sago35
Copy link
Member

@sago35 sago35 commented Oct 21, 2020

I made it faster by rewriting SPI.Tx().
Originally, there was a gap of about 400ns per byte, but this is a significant improvement.

After this PR is merged, drivers/ili9341 should be able to simplify it.
tinygo-org/drivers#208

old: 6ab0106
image

new txrx():
image

new tx():
image

new rx():
image

@sago35
Copy link
Member Author

sago35 commented Oct 21, 2020

This PR seems to be working fine, but the following problems may remain
So far it seems to work fine except for SERCOM0 on WioTerminal.

@sago35 sago35 marked this pull request as draft October 21, 2020 12:47
Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

The code looks reasonable, but I haven't tested it.

@sago35
Copy link
Member Author

sago35 commented Oct 26, 2020

This PR seems to be working fine, but the following problems may remain
So far it seems to work fine except for SERCOM0 on WioTerminal.

samd51: SPI Data received is irregularly corrupted #1419

I was able to get a few NG examples.
For the reply (137 bytes (r:137:...)) to the command AT+GMR, the result depends on the implementation.

So far, the following rules appear to be in existence.
At the moment I don't know how to do any further research.

  1. Adding the debug port output appears to reduce the number of illegal bytes.
  2. Illegal bytes are inserted, not rewritten.
  3. Occurs only in SERCOM0
  • Not occurring in SERCOM1 and SERCOM5
  1. This also occurs in communications using DMA

request : w:8: "AT+GMR\r\n"
https://github.com/Seeed-Studio/seeed-ambd-sdk

ok-1:
r:137: "AT version:2.2.0.2(0 - Jul 29 2020 07:25:59)\r\nSDK version:v6.2c\r\ncompile time (0):2020/07/29-07:25:30\r\nBin version:1.0.0(RTL8720DM)\r\nOK\r\n"

func (spi SPI) txrx(tx, rx []byte) {
    for i := 0; i < len(rx); i++ {
        rx[i], _ = spi.Transfer(0xFF)
        for j := 0; j < 1000; j++ {
            arm.Asm("nop")
        }
    }
}

ok-2: (If the amount of response is about 1KB, sometimes there are invalid bytes in the response)
r:137: "AT version:2.2.0.2(0 - Jul 29 2020 07:25:59)\r\nSDK version:v6.2c\r\ncompile time (0):2020/07/29-07:25:30\r\nBin version:1.0.0(RTL8720DM)\r\nOK\r\n"

func (spi SPI) txrx(tx, rx []byte) {
    for i := 0; i < len(rx); i++ {
        rx[i], _ = spi.Transfer(0xFF)
    }
}

ng-1:
response: r:137: "AT version:2.2.0.2(0 - Jul 29 2020 07:25:59)\r\nSDK version:v6\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe.2c\r"

func (spi SPI) txrx(tx, rx []byte) {
    spi.Bus.DATA.Set(uint32(tx[0]))
    for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_DRE) {
    }

    for i := 1; i < len(rx); i++ {
        spi.Bus.DATA.Set(uint32(tx[i]))
        for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
        }
        rx[i-1] = byte(spi.Bus.DATA.Get())
    }
    for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
    }
    rx[len(rx)-1] = byte(spi.Bus.DATA.Get())
}

ng-2:
response: r:137: "AT version:2.2.0.2(0 - Jul 29 2020 07:25:59)\r\nSDK version:v6\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe2c\r\ncompile time (0):2020/07/29-07:25:30\r\nBin version"

func (spi SPI) txrx(tx, rx []byte) {
    BCM3.Toggle()
    spi.Bus.DATA.Set(uint32(tx[0]))
    for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_DRE) {
    }

    for i := 1; i < len(rx); i++ {
        spi.Bus.DATA.Set(uint32(tx[i]))
        BCM4.Toggle()
        for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
        }
        BCM4.Toggle()
        rx[i-1] = byte(spi.Bus.DATA.Get())
    }
    BCM27.Toggle()
    for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
    }
    BCM27.Toggle()
    rx[len(rx)-1] = byte(spi.Bus.DATA.Get())
    BCM3.Toggle()
}

ng-3:
response: r:137: "AT version:2.2.0.2(0 - Jul 29 2020 07:25:59)\r\nSDK version:v6\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe.2c\r\ncompile time (0):2020/07/29-07:25:30\r\nBin version:1.0.0"

func (spi SPI) txrx(tx, rx []byte) {
    BCM3.Toggle()
    spi.Bus.DATA.Set(uint32(tx[0]))
    for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_DRE) {
    }

    for i := 1; i < len(rx); i++ {
        spi.Bus.DATA.Set(uint32(tx[i]))
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
        }
        BCM4.Toggle()
        rx[i-1] = byte(spi.Bus.DATA.Get())
    }
    BCM27.Toggle()
    for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
    }
    BCM27.Toggle()
    rx[len(rx)-1] = byte(spi.Bus.DATA.Get())
    BCM3.Toggle()
}

ng-4:
response: r:137: "AT version:2.2.0.2(0 - Jul 29 2020 07:25:59)\r\nSDK version:v6\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xbe\xae2c\r\ncompile time (0):2020/07/29-07"

func (spi SPI) txrx(tx, rx []byte) {
    BCM3.Toggle()
    spi.Bus.DATA.Set(uint32(tx[0]))
    for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_DRE) {
    }

    for i := 1; i < len(rx); i++ {
        spi.Bus.DATA.Set(uint32(tx[i]))
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        BCM4.Toggle()
        for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
        }
        BCM4.Toggle()
        rx[i-1] = byte(spi.Bus.DATA.Get())
    }
    BCM27.Toggle()
    for !spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
    }
    BCM27.Toggle()
    rx[len(rx)-1] = byte(spi.Bus.DATA.Get())
    BCM3.Toggle()
}

@sago35
Copy link
Member Author

sago35 commented Oct 26, 2020

This PR seems to be working fine, but the following problems may remain
So far it seems to work fine except for SERCOM0 on WioTerminal.

samd51: SPI Data received is irregularly corrupted #1419

It seems that my SPI settings were incorrect.
The RTL8720DN is set at 6MHz on the arduino, and it seems to me that it should be that clock.
When I changed Freq, the problem does not occur at present.

Comment on lines +1499 to +1528
// read to clear RXC register
for spi.Bus.INTFLAG.HasBits(sam.SERCOM_SPIM_INTFLAG_RXC) {
spi.Bus.DATA.Get()
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This process is necessary because other functions are looking at RXC.

For example, if RXC is set to 1 at the time of the SPI.Transfer() call, the wait will fail.

@sago35
Copy link
Member Author

sago35 commented Mar 11, 2021

It seems that my SPI settings were incorrect.
The RTL8720DN is set at 6MHz on the arduino, and it seems to me that it should be that clock.
When I changed Freq, the problem does not occur at present.

The SPI speed was too fast and was not working properly.
MISO is the data from the RTL8720DN.

image

@sago35
Copy link
Member Author

sago35 commented Mar 11, 2021

I rebase the SPI source code because I think it is OK.
Please review it.

old : 42088f9
image

new txrx : 15d8ef3
image

new tx : 15d8ef3
image

new rx : 15d8ef3
image

test code:

package main

// This is the most minimal blinky example and should run almost everywhere.

import (
	"machine"
	"time"
)

func main() {
	led := machine.LED
	led.Configure(machine.PinConfig{Mode: machine.PinOutput})

	spi := machine.SPI0
	spi.Configure(machine.SPIConfig{
		SCK:       machine.SPI0_SCK_PIN,
		SDO:       machine.SPI0_SDO_PIN,
		SDI:       machine.SPI0_SDI_PIN,
		Frequency: 48000000,
	})

	txbuf := make([]byte, 64)
	rxbuf := make([]byte, 64)

	for i := range txbuf {
		txbuf[i] = byte(i)
		rxbuf[i] = byte(i)
	}

	for {
		led.Toggle()
		time.Sleep(time.Millisecond * 500)

		if true {
			spi.Tx(txbuf, rxbuf)
		}
		if false {
			//spi.Tx(txbuf, nil)
			spi.Tx(nil, rxbuf)
		}
	}
}

@sago35 sago35 requested a review from aykevl March 11, 2021 12:15
@sago35
Copy link
Member Author

sago35 commented Mar 11, 2021

I made a mistake in rebasing, so I rebased it again.

@sago35 sago35 marked this pull request as ready for review March 11, 2021 12:19
@deadprogram
Copy link
Member

Thank you very much again for this exceptionally well documented PR.

Now merging.

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