ESP32 hardware SPI receive broken since 2v06 #1963

gfwilliams · 2020-12-03T08:32:30Z

See: http://forum.espruino.com/conversations/357245/#comment15677900

// Short D12 and D13 with a jumper

var s = new SPI();
s.setup({miso:D13, mosi:D12, sck:D14});
print(JSON.stringify(s.send("Hello World")));
// "Hello World"
print(JSON.stringify(s.send("Hello World")));
// "Hello World"

var s = SPI1;
s.setup({miso:D13, mosi:D12, sck:D14});
print(JSON.stringify(s.send("X")));
// "\xFF"
print(JSON.stringify(s.send("X")));
// "\x00"
print(JSON.stringify(s.send("XY")));
// "XY" <- actually works (seems it works if a multiple of 2 bytes?)
print(JSON.stringify(s.send("Hello World")));
// "Hello Worl\xFF"
print(JSON.stringify(s.send("Hello World")));
// "Hello Worl\x00"

// Trying to use SPI2 at this point causes a full reboot
// E (76511) spi_master: spi_bus_initialize(156): dma channel already in use

// But let's assume we don't use SPI1 so this
// actually works...

//var s = SPI2;
//s.setup({miso:D13, mosi:D12, sck:D14});
// print(JSON.stringify(s.send("XY")));
// "He"
// print(JSON.stringify(s.send("This is a test")));
// "Hell\u0000\u0000\u0000\u0000\u0000\u0000Hell"

// So it seems SPI2 is actually using SPI1's receive buffer?

This seem related to the SPI DMA modifications that went in after 2v06: #1887

@MaBecker @atc1441 could you take a look at this please? This seems totally broken and I'd have thought even the quickest of tests would have shown it wasn't working correctly.

The text was updated successfully, but these errors were encountered:

MaBecker · 2020-12-03T20:19:00Z

Reading and writing at the same time

Looks like this is a knowing issue found here

Half-duplex transactions are not compatible with DMA when both writing and reading phases are used.
If such transactions are required, you have to use one of the alternative solutions:

Use full-duplex transactions instead.

Disable DMA by setting the bus initialization function’s last parameter to 0 as follows:
ret=spi_bus_initialize(VSPI_HOST, &buscfg, 0);

This can prohibit you from transmitting and receiving data longer than 64 bytes. 3. Try using the command and address
fields to replace the write phase.

//    compiled and tested with ret=spi_device_queue_trans(SPIChannels[channelPnt].spi, &spi_trans, 0);
var s = SPI2;
s.setup({miso:D19, mosi:D23, sck:D18});
print(JSON.stringify(s.send("This is a test")));
"This is a test"

So what about a additional spi option like {dma:true} with default value false?
The use of dma transfer to displays is the huge performance kicker - as we all know - so not just a nice to have at all.

Trying to use both hardware SPI at this point causes a full reboot

There is a fixed dma channel with value 1 used in jshSPISetup() that is causing this full reboot.

Espruino/targets/esp32/jshardwareSpi.c

Line 133 in 24ce12a

esp_err_t ret=spi_bus_initialize(SPIChannels[channelPnt].HOST, &buscfg, 1);

Possible fix is to give each SPI it's own dma channel.

void jshSPISetup(
    IOEventFlags device, //!< The identity of the SPI device being initialized.
    JshSPIInfo *inf      //!< Flags for the SPI device.
) {
  int channelPnt = getSPIChannelPnt(device);
  int dma_chan = 0;  
  Pin sck, miso, mosi;
  if(SPIChannels[channelPnt].HOST == HSPI_HOST){
    dma_chan = 1;
    sck = inf->pinSCK != PIN_UNDEFINED ? inf->pinSCK : 14;
    miso = inf->pinMISO != PIN_UNDEFINED ? inf->pinMISO : 12;
    mosi = inf->pinMOSI != PIN_UNDEFINED ? inf->pinMOSI : 13;
  }
  else {
    dma_chan = 2;
    sck = inf->pinSCK != PIN_UNDEFINED ? inf->pinSCK : 5;
    miso = inf->pinMISO != PIN_UNDEFINED ? inf->pinMISO : 19;
    mosi = inf->pinMOSI != PIN_UNDEFINED ? inf->pinMOSI : 23;
  }

........

  esp_err_t ret=spi_bus_initialize(SPIChannels[channelPnt].HOST, &buscfg, dma_chan);

........

test result

>var s = SPI2;
=SPI2
=undefined
>print(JSON.stringify(s.send("This is a test")));
"This is a test"
=undefined

>var s = SPI1;
=SPI1
=undefined
>print(JSON.stringify(s.send("ABC")));
"ABC"
=undefined

What do you think?

gfwilliams · 2020-12-04T09:03:38Z

Thanks for looking into this!

So what about a additional spi option like {dma:true} with default value false?

Honestly that seems like a lot of added complexity. If you add some platform-specific fields like that nobody's really going to use them (or will use them wrong - eg for receiving). If it's some code that only you know about it might as well just be in your own branch :)

How about just automatically disabling DMA (maybe even using Espruino's fallback jshSPISendMany code) in the case that 'rx' is non-null (eg you're supposed to receive data)?

Possible fix is to give each SPI it's own dma channel.

That sounds perfect - as long as it doesn't break anything else?

gfwilliams · 2020-12-04T09:04:34Z

Also just wanted to check, some similar changes went in for ESP8266 - does this mean that is broken too?

MaBecker · 2020-12-04T11:37:16Z

Also just wanted to check, some similar changes went in for ESP8266 - does this mean that is broken too?

Yes I tried but it was never merged. As I am mot using SPI on ESP8266 devices I have no clue if is is working

MaBecker · 2020-12-04T11:40:03Z

Possible fix is to give each SPI it's own dma channel.

That sounds perfect - as long as it doesn't break anything else?

Who know's, as the main ESP32 developer are not longer active .....

MaBecker · 2020-12-04T12:03:58Z

Ok, let's do some LCD testing with out DMA .....

* disabling DMA just if rx is specified

MaBecker · 2020-12-04T15:46:57Z

>var s = SPI2;
=SPI2
>s.setup({miso:D19, mosi:D23, sck:D18}); 
>print(JSON.stringify(s.send("This is a test")));
"This is a test"

Fast display updates work too.

gfwilliams · 2020-12-04T16:05:47Z

Great - thanks for this!

Update SPI (fix #1963)

gfwilliams · 2020-12-04T16:40:37Z

SPI2 now works as well as SPI1 - thanks!

But they're BOTH still broken! If you try the code from the test right at the start of this issue it still fails in exactly the same way:

var s = SPI1;
s.setup({miso:D13, mosi:D12, sck:D14});
print(JSON.stringify(s.send("X")));
// "\xFF"
print(JSON.stringify(s.send("X")));
// "\x00"
print(JSON.stringify(s.send("XY")));
// "XY" <- actually works (seems it works if a multiple of 2 bytes?)
print(JSON.stringify(s.send("Hello World")));
// "Hello Worl\xFF"
print(JSON.stringify(s.send("Hello World")));
// "Hello Worl\x00"

I think your "This is a test" only works because as mentioned before it's a multiple of 2 bytes.

MaBecker · 2020-12-04T16:51:39Z

thanks for testing is again with a single char.

This will not fix it but you mixed pin numbers for miso and mosi; s.setup({miso:D12, mosi:D13, sck:D14});

Hmm, just tested on SPI2 and 1 char is broken on my side too.

this is the section:

Espruino/targets/esp32/jshardwareSpi.c

Lines 186 to 190 in 3604727

    
           if (count==1) { 
        
             int r = jshSPISend(device, tx?*tx:-1); 
        
             if (rx) *rx = r; 
        
             return true; 
        
           }

MaBecker · 2020-12-04T16:54:46Z

I think your "This is a test" only works because as mentioned before it's a multiple of 2 bytes.

it work's with 3 chars too

 >print(JSON.stringify(s.send("ABC")));
"ABC"

gfwilliams · 2020-12-04T17:01:09Z

What about print(JSON.stringify(s.send("Hello World"))); ?

MaBecker · 2020-12-04T17:17:47Z

Wow, what is wrong here .......

    if (count==1) {
      jsiConsolePrintf("%d\n",tx[0]);  // added this line to my local coding 
      int r = jshSPISend(device, tx?*tx:-1);
      if (rx) *rx = r;
      return true;
    }

>print(JSON.stringify(s.send("Hello World")));
100   <- (count == 1)
"Hello Worl\u0000"

>print(JSON.stringify(s.send("ABC")));
"ABC"

>print(JSON.stringify(s.send("ABCE")));
"ABCE"

>print(JSON.stringify(s.send("ABCEF")));
"ABCEF"

>print(JSON.stringify(s.send("ABCEFG")));
"ABCEFG"

>print(JSON.stringify(s.send("ABCEFGI")));
"ABCEFGI"

>print(JSON.stringify(s.send("ABCEFGHI")));
"ABCEFGHI"

>print(JSON.stringify(s.send("ABCEFGHIJ")));
"ABCEFGHIJ"

>print(JSON.stringify(s.send("ABCEFGHIJK")));
"ABCEFGHIJK"

>print(JSON.stringify(s.send("ABCEFGHIJKL")));
76   <- (count == 1)
"ABCEFGHIJK\u0000"

>print(JSON.stringify(s.send("ABCEFGHIJKLM")));
"ABCEFGHIJKLM"

>print(JSON.stringify(s.send("ABCEFGHIJKLMN")));
"ABCEFGHIJKLMN"

rujorgensen · 2020-12-04T17:39:50Z

I seem to have an issue with using arrays (sorry for throwing another issue in here, I know you've been working hard on this today 😊).

Perhaps I don't understand SPI.send, but assuming these are equivalent: print(JSON.stringify(s.send("AB"))); print(JSON.stringify(s.send([0x65, 0x66])));:

The same output is not generated with:

const PIN_SCK = D18;
const PIN_MISO = D19;
const PIN_MOSI = D23;

let s = SPI2;
s.setup({
    sck: PIN_SCK,
    miso: PIN_MISO,
    mosi: PIN_MOSI,
    baud: 600000,
});

print(JSON.stringify(s.send("AB")));
// Outputs "AB"
print(JSON.stringify(s.send([0x65, 0x66])));
// Outputs "[255, 0]"

MaBecker · 2020-12-04T17:43:22Z

look's like jshSPISend()need some attention too.

MaBecker · 2020-12-05T07:09:45Z

New test code to get a clue what's goin on

var s = SPI2;
s.setup({miso:D19, mosi:D23, sck:D18});
for (var i =1; i< 50; i++) {
  var d = "".padEnd(i,"0");
  print((i.toString(10)).padStart(2,"0"),JSON.stringify(s.send(d)));
}

output

>count: 1, rx: 1
01 "\u00FF"
count: 2, rx: 1
02 "00"
count: 3, rx: 1
03 "000"
count: 4, rx: 1
04 "0000"
count: 5, rx: 1
05 "00000"
count: 6, rx: 1
06 "000000"
count: 7, rx: 1
07 "0000000"
count: 8, rx: 1
08 "00000000"
count: 9, rx: 1
09 "000000000"
count: 10, rx: 1
10 "0000000000"
count: 10, rx: 1
count: 1, rx: 1
11 "0000000000\u0000"
count: 10, rx: 1
count: 2, rx: 1
12 "000000000000"
count: 10, rx: 1
count: 3, rx: 1
13 "0000000000000"
count: 10, rx: 1
count: 4, rx: 1
14 "00000000000000"
count: 10, rx: 1
count: 5, rx: 1
15 "000000000000000"
count: 10, rx: 1
count: 6, rx: 1
16 "0000000000000000"
count: 10, rx: 1
count: 7, rx: 1
17 "00000000000000000"
count: 10, rx: 1
count: 8, rx: 1
18 "000000000000000000"
count: 10, rx: 1
count: 9, rx: 1
19 "0000000000000000000"
count: 10, rx: 1
count: 10, rx: 1
20 "00000000000000000000"
count: 10, rx: 1
count: 11, rx: 1
21 "000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
22 "0000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 1, rx: 1
23 "0000000000000000000000\u0000"
count: 10, rx: 1
count: 12, rx: 1
count: 2, rx: 1
24 "000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 3, rx: 1
25 "0000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 4, rx: 1
26 "00000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 5, rx: 1
27 "000000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 6, rx: 1
28 "0000000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 7, rx: 1
29 "00000000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 8, rx: 1
30 "000000000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 9, rx: 1
31 "0000000000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 10, rx: 1
32 "00000000000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 11, rx: 1
33 "000000000000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 12, rx: 1
34 "0000000000000000000000000000000000"
count: 10, rx: 1
count: 12, rx: 1
count: 12, rx: 1
count: 1, rx: 1
35 "0000000000000000000000000000000000\u0000"
>

There is a logic that splits the send buffer in chunks of 10,12,12,... bytes to pass to jshSPISendMany()
Every time jshSPISendMany() is called with count==1 the wrong data is returned.
Fix jshSPISend() to fix the issue.

MaBecker · 2020-12-05T07:32:47Z

Ok restoring jshSPISend() and placing the read a the correct position which is at the end and not at the beginning fixed that issue on my side.

MaBecker · 2020-12-05T07:56:28Z

>count: 1, rx: 1
01 "A"
count: 2, rx: 1
02 "AA"
count: 3, rx: 1
03 "AAA"
count: 4, rx: 1
04 "AAAA"
count: 5, rx: 1
05 "AAAAA"
count: 6, rx: 1
06 "AAAAAA"
count: 7, rx: 1
07 "AAAAAAA"
count: 8, rx: 1
08 "AAAAAAAA"
count: 9, rx: 1
09 "AAAAAAAAA"
count: 10, rx: 1
10 "AAAAAAAAAA"
count: 10, rx: 1
count: 1, rx: 1
11 "AAAAAAAAAAA"
count: 10, rx: 1
count: 2, rx: 1
12 "AAAAAAAAAAAA"
count: 10, rx: 1
count: 3, rx: 1
13 "AAAAAAAAAAAAA"
count: 10, rx: 1
count: 4, rx: 1
14 "AAAAAAAAAAAAAA"
count: 10, rx: 1
count: 5, rx: 1
15 "AAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 6, rx: 1
16 "AAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 7, rx: 1
17 "AAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 8, rx: 1
18 "AAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 9, rx: 1
19 "AAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 10, rx: 1
20 "AAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 11, rx: 1
21 "AAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
22 "AAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 1, rx: 1
23 "AAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 2, rx: 1
24 "AAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 3, rx: 1
25 "AAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 4, rx: 1
26 "AAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 5, rx: 1
27 "AAAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 6, rx: 1
28 "AAAAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 7, rx: 1
29 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 8, rx: 1
30 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 9, rx: 1
31 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 10, rx: 1
32 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 11, rx: 1
33 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 12, rx: 1
34 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
count: 10, rx: 1
count: 12, rx: 1
count: 12, rx: 1
count: 1, rx: 1
35 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
> 
```

MaBecker · 2020-12-05T08:02:51Z

Perhaps I don't understand SPI.send, but assuming these are equivalent: print(JSON.stringify(s.send("AB"))); print(JSON.stringify(s.send([0x65, 0x66])));:

close, values are int and yes this works no too with the latest pr.

print(JSON.stringify(s.send("AB")));
// Outputs "AB"
print(JSON.stringify(s.send([0x65, 0x66])));
// Outputs "[101,102]"

rujorgensen · 2020-12-05T11:39:18Z

will let you know when it is time to merge, like to spend some more hours on testing.

Don't know if you still want feedback, but I'm getting a new, rather nasty, error 🙂:

ASSERT(jsvGetLocks(var) < 15) FAILED AT src/jsvar.c:721
  #1[r1,l2] Object {
    #2[r1,l2] ASSERT(jsvGetLocks(var) < 15) FAILED AT src/jsvar.c:701
HALTING.

with

const PIN_SCK = D18;
const PIN_MISO = D19;  // D19
const PIN_MOSI = D23;

const PIN_CS = D5;                               // NSS
const PIN_RESET = D21;
const IRQ_PIN = D22;

// *** Setup SPI

let spi = SPI2;
spi.setup({
    sck: PIN_SCK,
    miso: PIN_MISO,
    mosi: PIN_MOSI,
    baud: 600000,
});

// *** Setup SX127x

// Configuration
const txConfig = {
    bandwidth: 0,
    freq: 434330000,
    power: 17,
    forcePaBoost: true,
};

const sx = require("SX127x").connect({ spi: SPI2, cs: PIN_CS, rst: PIN_RESET });

// Poll DIO0 for interrupts
setInterval(function () { sx.onIRQ(); }, 100);
sx.setTxConfig(txConfig);
// Transmit
sx.send("Hello", function () {
    console.log("TX done");
});

I don't know if it would also have been there before, since HW SPI was not working then.

MaBecker · 2020-12-05T16:00:24Z

Try this build http://www.espruino.com/binaries/travis/master/?C=M;O=D

Hmm just checked, there is no fw for ESP32.

rujorgensen · 2020-12-05T16:30:56Z

Try this build http://www.espruino.com/binaries/travis/master/?C=M;O=D

Hmm just checked, there is no fw for ESP32.

And aren't they all built before my comment too 🙂? I could pull something into my local repo, but I don't see anything new pushed.

* add callback for cout == 1

* fix build error

MaBecker · 2020-12-06T07:53:53Z

Ok, fixed the build error, now there is a ESP32 firmware created by travis.

Update SPI (fix #1963)

gfwilliams added the ESP32 This is only a problem on ESP32-based devices label Dec 3, 2020

MaBecker self-assigned this Dec 3, 2020

MaBecker mentioned this issue Dec 4, 2020

Improve ESP32 implementation #1777

Open

33 tasks

MaBecker added a commit that referenced this issue Dec 4, 2020

Update SPI (fix #1963)

1245529

* disabling DMA just if rx is specified

gfwilliams closed this as completed in a133b57 Dec 4, 2020

gfwilliams added a commit that referenced this issue Dec 4, 2020

Merge pull request #1964 from espruino/esp32_improvement

3604727

Update SPI (fix #1963)

gfwilliams reopened this Dec 4, 2020

MaBecker added a commit that referenced this issue Dec 6, 2020

Update SPI (fix #1963)

6c18ec8

* add callback for cout == 1

MaBecker added a commit that referenced this issue Dec 6, 2020

Update SPI (fix #1963)

689a48d

* fix build error

gfwilliams closed this as completed in 64c4a0f Dec 7, 2020

gfwilliams added a commit that referenced this issue Dec 7, 2020

Merge pull request #1965 from espruino/esp32_improvement

6b19a02

Update SPI (fix #1963)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESP32 hardware SPI receive broken since 2v06 #1963

ESP32 hardware SPI receive broken since 2v06 #1963

gfwilliams commented Dec 3, 2020

MaBecker commented Dec 3, 2020

gfwilliams commented Dec 4, 2020 •

edited

Loading

gfwilliams commented Dec 4, 2020

MaBecker commented Dec 4, 2020

MaBecker commented Dec 4, 2020

MaBecker commented Dec 4, 2020

MaBecker commented Dec 4, 2020

gfwilliams commented Dec 4, 2020

gfwilliams commented Dec 4, 2020

MaBecker commented Dec 4, 2020

MaBecker commented Dec 4, 2020

gfwilliams commented Dec 4, 2020

MaBecker commented Dec 4, 2020

rujorgensen commented Dec 4, 2020 •

edited

Loading

MaBecker commented Dec 4, 2020

MaBecker commented Dec 5, 2020

MaBecker commented Dec 5, 2020

MaBecker commented Dec 5, 2020

MaBecker commented Dec 5, 2020 •

edited

Loading

rujorgensen commented Dec 5, 2020

MaBecker commented Dec 5, 2020 •

edited

Loading

rujorgensen commented Dec 5, 2020

MaBecker commented Dec 6, 2020

ESP32 hardware SPI receive broken since 2v06 #1963

ESP32 hardware SPI receive broken since 2v06 #1963

Comments

gfwilliams commented Dec 3, 2020

MaBecker commented Dec 3, 2020

gfwilliams commented Dec 4, 2020 • edited Loading

gfwilliams commented Dec 4, 2020

MaBecker commented Dec 4, 2020

MaBecker commented Dec 4, 2020

MaBecker commented Dec 4, 2020

MaBecker commented Dec 4, 2020

gfwilliams commented Dec 4, 2020

gfwilliams commented Dec 4, 2020

MaBecker commented Dec 4, 2020

MaBecker commented Dec 4, 2020

gfwilliams commented Dec 4, 2020

MaBecker commented Dec 4, 2020

rujorgensen commented Dec 4, 2020 • edited Loading

MaBecker commented Dec 4, 2020

MaBecker commented Dec 5, 2020

MaBecker commented Dec 5, 2020

MaBecker commented Dec 5, 2020

MaBecker commented Dec 5, 2020 • edited Loading

rujorgensen commented Dec 5, 2020

MaBecker commented Dec 5, 2020 • edited Loading

rujorgensen commented Dec 5, 2020

MaBecker commented Dec 6, 2020

gfwilliams commented Dec 4, 2020 •

edited

Loading

rujorgensen commented Dec 4, 2020 •

edited

Loading

MaBecker commented Dec 5, 2020 •

edited

Loading

MaBecker commented Dec 5, 2020 •

edited

Loading