esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
15.96k stars 13.34k forks source link

Hadware SPI Slave Select (SS) setup and hold bit states in SPI1U not maintained #5712

Open Bodmer opened 5 years ago

Bodmer commented 5 years ago

I had some issues with lost SPI data in overlap mode. Tracked this down to the Slave Select (SS) setup and hold bits getting cleared by different SPI calls.

The bits are set here by the call here.

But then immediately afterwards cleared here. These bits are also cleared elsewhere in the SPI functions, here and here and here.

Edit (devyte): Ref commit here.

devyte commented 5 years ago

@Bodmer I'm confused. Are you proposing a fix for your issue? If so, please propose your changes in a PR. If not, then please provide more details, because pointers to the set/clear of those bits isn't enough to understand the problem. Please be aware the none of the currently active maintainers are involved with low level SPI, let alone overlap mode, so any investigation and testing would need to be done by a user. I encourage you to continue seeking an answer yourself, or you can wait to see if someone else picks this up.

Bodmer commented 5 years ago

There is a simple bug in the code that will not affect users who do not need longer setup and hold times on the hardware controlled SS line. The setHwCs() function sets the control bits required for extra setup and hold time, but other SPI calls clear them. The links show where this happens.

I can provide a work around for the users code but the esp8266 team need to decide on the best approach to fix the bug.

devyte commented 5 years ago

@Bodmer I suggest you propose your solution in a PR, that would allow a better understanding of what you mean, and someone familiar with SPI could eventually take a look.

devyte commented 5 years ago

@JiriBilek do you know if the changes proposed here have merit? I don't understand the references in the original post, so I'm wondering if you can help out?

NdK73 commented 5 years ago

I think it's related to the problem I'm having to drive a SPI display + SPI touch controller using the "extra" pins on the ESP12E module, the ones usually reserved for the EEPROM chip. "Degrading" flash mode to DIO / DOUT frees GPIO9 and GPIO10, that could be used as CS pins for the display and the touch even if they're still connected to the flash chip (/hold and /wp lines: no ill effects if driven low outside of flash access as long as they're brought back high before accessing the flash again). That would mean having two extra GPIO pins (the ones that would end up used by the two CS lines, usually GPIO0 dor the display and another for the touch). I'm still "a bit" lost in the ESP datasheet, but it seems doable... And that would give a whole new sense to pins() method, that's now quite counter-intuitive: it accepts 4 parameters (appears that any pin combo is OK) but then accepts only two sets...

JiriBilek commented 5 years ago

@devyte sorry, I haven't tried the SPI Master mode on ESP8266. My knowledge ends with the slave mode. I don't understand the refences, too.