gustavonihei commented on code in PR #6511:
URL: https://github.com/apache/incubator-nuttx/pull/6511#discussion_r906054724
##########
boards/xtensa/esp32/common/src/esp32_board_spi.c:
##########
@@ -84,6 +84,19 @@ static inline int spi_cmddata(struct spi_dev_s *dev,
uint32_t devid,
}
#endif
+#ifdef CONFIG_LCD_SSD1680
+ if (devid == SPIDEV_DISPLAY(0))
+ {
+ /* This is the Data/Command control pad which determines whether the
+ * data bits are data or a command.
+ */
+
+ esp32_gpiowrite(CONFIG_SSD1680_GPIO_PIN_DTA_CMD, !cmd);
+
+ return OK;
+ }
+#endif
Review Comment:
This block is basically identical to the one above for `LCD_ILI9341`, we
should avoid this code duplication.
Besides, `CONFIG_SSD1680_GPIO_PIN_DTA_CMD` is misplaced
[here](https://github.com/apache/incubator-nuttx/blob/master/boards/xtensa/esp32/common/Kconfig#L63-L90).
It is specific to just one ESP32-based board and, more important, it is an
immutable configuration, so it (and the other GPIOs) should not be a Kconfig
option.
Could you please move them to the board header, like it is being done for
the ESP-WROVER-KIT LCD
[here](https://github.com/apache/incubator-nuttx/blob/master/boards/xtensa/esp32/esp32-wrover-kit/include/board.h#L75-L82)?
This way we can avoid this code duplication and just extend the ifdef to
consider both `CONFIG_LCD_ILI9341` and `CONFIG_LCD_SSD1680`.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
