[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1422: ESP32: Add driver support to SPI Master and Slave

[GitBox <gi...@apache.org>]

Ouss4 commented on a change in pull request #1422:
URL: https://github.com/apache/incubator-nuttx/pull/1422#discussion_r456837563



##########
File path: arch/xtensa/src/esp32/Make.defs
##########
@@ -99,6 +99,7 @@ CHIP_CSRCS  = esp32_allocateheap.c esp32_clockconfig.c esp32_cpuint.c
 CHIP_CSRCS += esp32_gpio.c esp32_intdecode.c esp32_irq.c esp32_region.c
 CHIP_CSRCS += esp32_timerisr.c
 CHIP_CSRCS += esp32_user.c
+CHIP_CSRCS += esp32_spi.c esp32_spi_slave.c

Review comment:
       ```suggestion
   ifeq ($(CONFIG_ESP32_SPI),y)
   CHIP_CSRCS += esp32_spi.c esp32_spi_slave.c
   endif
   ```

##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -81,25 +81,23 @@ config ESP32_SDMMC
 	---help---
 		No yet implemented
 
-config ESP32_SPI0

Review comment:
       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.

##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -309,4 +307,58 @@ endif # SERIAL_IFLOWCONTROL || SERIAL_OFLOWCONTROL
 endif # ESP32_UART2
 
 endmenu # UART configuration
+
+menu "SPI configuration"
+	depends on ESP32_SPI
+
+if ESP32_SPI2
+
+config ESP32_SPI2_CSPIN
+	int "SPI2 CS Pin"

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




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org