ESP32: Add driver support to SPI Master and Slave #1422

acassis · 2020-07-18T22:31:08Z

Summary

This driver was implemented by Dong Heng dongheng@espressif.com
and modified to fix coding style by Alan Carvalho de Assis.

Impact

New feature to ESP32

Testing

ESP-WROVER-KIT

arch/xtensa/src/esp32/Make.defs

Ouss4 · 2020-07-18T23:22:54Z

arch/xtensa/src/esp32/Kconfig

@@ -81,25 +81,23 @@ config ESP32_SDMMC
 	---help---
 		No yet implemented

-config ESP32_SPI0


What's the reason to remove SPI0 and SPI1? SPI0 is somewhat special, but the difference with SPI1 is that SPI2 and 3 can be configured as slaves.

Yes, that was the idea, it only support SPI2 and SPI3 because these ports can be master or slave. The code should be modified in the future to support SPI0 and SPI1

Shouldn't then only the slave source code be dependent on CONFIG_ESP32_SPI2 || CONFIG_ESP32_ESP3? The master code is similar, right?

Good point! I agree! You are right, thank you very much Abde!

Ouss4 · 2020-07-18T23:28:23Z

arch/xtensa/src/esp32/Kconfig

+if ESP32_SPI2
+
+config ESP32_SPI2_CSPIN
+	int "SPI2 CS Pin"


Shouldn't these SPI pins be configured by board logic? Similar to other boards.

Since the pins definition for ESP32 UART and other are already in the Kconfig, I think it is better to keep it this way.

Ouss4 · 2020-07-18T23:43:29Z

arch/xtensa/src/esp32/esp32_spi.c

+extern uint8_t esp32_spi2_status(FAR struct spi_dev_s *dev, uint32_t devid);
+extern uint8_t esp32_spi3_status(FAR struct spi_dev_s *dev, uint32_t devid);


For other architectures, these are prototyped in the header file (arch/xtensa/src/esp32/esp32_spi.h in this case) and implemented in the board logic.

Yes, I'm moving it to the header

Ouss4 · 2020-07-18T23:46:17Z

arch/xtensa/src/esp32/esp32_spi.c

+ ****************************************************************************/
+
+static int esp32_spi_lock(FAR struct spi_dev_s *dev, bool lock);
+static void esp32_spi_select(FAR struct spi_dev_s *dev,


This is also prototyped in the header file and implemented in board logic.

I think only spi_select should be in the header, at least on stm32 and nrf52 I only found _select, not _lock.

Yes, I was referring to spi_select not _lock.

acassis · 2020-07-19T12:49:39Z

@xiaoxiang781216 I think these recent modifications on CI broke the xtensa build, it appears that ${{matrix.boards}}.dat wasn't updated:

"cd sources/testing
./cibuild.sh -x testlist/mips-riscv-x86-xtensa.dat
"
ERROR: No readable file exists at testlist/mips-riscv-x86-xtensa.dat

USAGE: /github/workspace/sources/nuttx/tools/testbuild.sh [-l|m|c|u|g|n] [-d] [-e ] [-x] [-j ] [-a ] [-t ] [-p] [-G]
/github/workspace/sources/nuttx/tools/testbuild.sh -h"

Ouss4 · 2020-07-19T13:00:31Z

arch/xtensa/src/esp32/esp32_spi.c

+  FAR struct esp32_spi_priv_s *priv = (FAR struct esp32_spi_priv_s *)dev;
+  const uint32_t duty_cycle = 128;
+
+  if (frequency > ((APB_CLK_FREQ / 4) * 3))


Higher level drivers use setfrequency/setmode/setbits before every SPI transaction to make sure that the bus is in the state that the driver wants it to be. To avoid having to do all of what follows, usually we test if the frequency hasn't changed, if so we just return. Something like:

if (priv->frequency == frequency) { /* We are already at this frequency. Return the actual. */ return priv->actual; }

Good point Abdelatif! Thank you!

Ouss4 · 2020-07-19T13:34:55Z

@xiaoxiang781216 I think these recent modifications on CI broke the xtensa build, it appears that ${{matrix.boards}}.dat wasn't updated:

That's because the config matrix was only updated in testing/. It's trivial, I can provide a fix for that.

#1424 and its sister in apps/ should do.
But both need #1423. Furthermore there are some warnings in the AVR builds, that would also break the build.

acassis · 2020-07-19T14:10:53Z

@xiaoxiang781216 I think these recent modifications on CI broke the xtensa build, it appears that ${{matrix.boards}}.dat wasn't updated:

That's because the config matrix was only updated in testing/. It's trivial, I can provide a fix for that.

#1424 and its sister in apps/ should do.
But both need #1423. Furthermore there are some warnings in the AVR builds, that would also break the build.

It didn't fix/update the issue:

cd sources/testing
./cibuild.sh -x testlist/mips-riscv-x86-xtensa.dat
"
ERROR: No readable file exists at testlist/mips-riscv-x86-xtensa.dat

USAGE: /github/workspace/sources/nuttx/tools/testbuild.sh [-l|m|c|u|g|n] [-d] [-e ] [-x] [-j ] [-a ] [-t ] [-p] [-G]
/github/workspace/sources/nuttx/tools/testbuild.sh -h

Where:
-l|m|c|u|g|n selects Linux (l), macOS (m), Cygwin (c),
Ubuntu under Windows 10 (u), MSYS/MSYS2 (g) or Windows native (n). Default Linux
-d enables script debug output
-e pass extra c/c++ flags such as -Wno-cpp via make command line
-x exit on build failures
-j passed on to make. Default: No -j make option.
-a provides the relative path to the apps/ directory. Default ../apps
-t provides the absolute path to top nuttx/ directory. Default /github/workspace/sources/testing/../nuttx

Ouss4 · 2020-07-19T14:11:58Z

You'll need to rebase. I doubt that the checks would pass however. As I said there is another PR that selects the right toolchain, and then there are some warnings in the AVR build.

xiaoxiang781216 · 2020-07-19T14:27:16Z

@acassis @Ouss4 I just rebase #1423, we need fix the error in the new avr build first.

This driver was implemented by Dong Heng <dongheng@espressif.com> and modified to fix coding style by Alan Carvalho de Assis.

Ouss4 reviewed Jul 18, 2020

View reviewed changes

acassis force-pushed the master branch from 5c5d090 to 2606824 Compare July 19, 2020 12:38

Ouss4 reviewed Jul 19, 2020

View reviewed changes

acassis force-pushed the master branch from dab24f7 to 8aee742 Compare July 19, 2020 14:14

acassis force-pushed the master branch from 8aee742 to 2905542 Compare July 19, 2020 18:50

Ouss4 approved these changes Jul 19, 2020

View reviewed changes

ESP32: Add driver support to SPI Master and Slave

abbd3d0

This driver was implemented by Dong Heng <dongheng@espressif.com> and modified to fix coding style by Alan Carvalho de Assis.

acassis force-pushed the master branch from 2905542 to abbd3d0 Compare July 19, 2020 20:21

Ouss4 merged commit 79a3fd1 into apache:master Jul 19, 2020

btashton added this to To-Add in Release Notes - 10.0.0 Oct 14, 2020

Ouss4 moved this from To-Add to In Progress in Release Notes - 10.0.0 Oct 17, 2020

Ouss4 moved this from In Progress to Added in Release Notes - 10.0.0 Oct 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESP32: Add driver support to SPI Master and Slave #1422

ESP32: Add driver support to SPI Master and Slave #1422

acassis commented Jul 18, 2020

Ouss4 Jul 18, 2020

acassis Jul 18, 2020

Ouss4 Jul 18, 2020

acassis Jul 19, 2020

Ouss4 Jul 18, 2020

acassis Jul 19, 2020

Ouss4 Jul 18, 2020

acassis Jul 19, 2020

Ouss4 Jul 18, 2020

acassis Jul 19, 2020

Ouss4 Jul 19, 2020

acassis commented Jul 19, 2020 •

edited

Ouss4 Jul 19, 2020

acassis Jul 19, 2020

Ouss4 commented Jul 19, 2020 •

edited

acassis commented Jul 19, 2020

Ouss4 commented Jul 19, 2020

xiaoxiang781216 commented Jul 19, 2020 •

edited

		extern uint8_t esp32_spi2_status(FAR struct spi_dev_s *dev, uint32_t devid);
		extern uint8_t esp32_spi3_status(FAR struct spi_dev_s *dev, uint32_t devid);

ESP32: Add driver support to SPI Master and Slave #1422

ESP32: Add driver support to SPI Master and Slave #1422

Conversation

acassis commented Jul 18, 2020

Summary

Impact

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acassis commented Jul 19, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ouss4 commented Jul 19, 2020 • edited

acassis commented Jul 19, 2020

Ouss4 commented Jul 19, 2020

xiaoxiang781216 commented Jul 19, 2020 • edited

acassis commented Jul 19, 2020 •

edited

Ouss4 commented Jul 19, 2020 •

edited

xiaoxiang781216 commented Jul 19, 2020 •

edited