acassis commented on code in PR #7932:
URL: https://github.com/apache/nuttx/pull/7932#discussion_r1052445970
##########
arch/arm/src/s32k3xx/s32k3xx_serial.c:
##########
@@ -1037,6 +1326,23 @@ struct s32k3xx_uart_s
#endif
#ifdef CONFIG_SERIAL_RS485CONTROL
uint8_t rs485mode:1; /* We are in RS485 (RTS on TX) mode */
+#endif
+ /* TX DMA state */
+
+#ifdef SERIAL_HAVE_TXDMA
+ const unsigned int dma_txreqsrc; /* DMAMUX source of TX DMA request */
+ DMACH_HANDLE txdma; /* currently-open trasnmit DMA stream */
Review Comment:
```suggestion
DMACH_HANDLE txdma; /* currently-open transmit DMA stream */
##########
arch/arm/src/s32k3xx/s32k3xx_serial.c:
##########
@@ -1059,12 +1365,49 @@ static int s32k3xx_attach(struct uart_dev_s *dev);
static void s32k3xx_detach(struct uart_dev_s *dev);
static int s32k3xx_interrupt(int irq, void *context, void *arg);
static int s32k3xx_ioctl(struct file *filep, int cmd, unsigned long arg);
+#if !defined(SERIAL_HAVE_ONLY_RXDMA)
static int s32k3xx_receive(struct uart_dev_s *dev, unsigned int *status);
static void s32k3xx_rxint(struct uart_dev_s *dev, bool enable);
static bool s32k3xx_rxavailable(struct uart_dev_s *dev);
-static void s32k3xx_send(struct uart_dev_s *dev, int ch);
+#endif
+#if !defined(SERIAL_HAVE_ONLY_TXDMA)
static void s32k3xx_txint(struct uart_dev_s *dev, bool enable);
+#endif
+
+#ifdef CONFIG_SERIAL_IFLOWCONTROL
+static bool s32k3xx_rxflowcontrol(struct uart_dev_s *dev,
+ unsigned int nbuffered, bool upper);
Review Comment:
Please keep aligned, 'unsigned' should start after '(' from previous line
##########
arch/arm/src/s32k3xx/Kconfig:
##########
@@ -1318,6 +1318,21 @@ config S32K3XX_LPUART_SINGLEWIRE
bool "Signal Wire Support"
default n
+config S32K3XX_SERIAL_RXDMA_BUFFER_SIZE
+ int "Rx DMA buffer size"
Review Comment:
```suggestion
int "RX DMA buffer size"
##########
arch/arm/src/s32k3xx/s32k3xx_serial.c:
##########
@@ -1059,12 +1365,49 @@ static int s32k3xx_attach(struct uart_dev_s *dev);
static void s32k3xx_detach(struct uart_dev_s *dev);
static int s32k3xx_interrupt(int irq, void *context, void *arg);
static int s32k3xx_ioctl(struct file *filep, int cmd, unsigned long arg);
+#if !defined(SERIAL_HAVE_ONLY_RXDMA)
static int s32k3xx_receive(struct uart_dev_s *dev, unsigned int *status);
static void s32k3xx_rxint(struct uart_dev_s *dev, bool enable);
static bool s32k3xx_rxavailable(struct uart_dev_s *dev);
-static void s32k3xx_send(struct uart_dev_s *dev, int ch);
+#endif
+#if !defined(SERIAL_HAVE_ONLY_TXDMA)
static void s32k3xx_txint(struct uart_dev_s *dev, bool enable);
+#endif
+
+#ifdef CONFIG_SERIAL_IFLOWCONTROL
+static bool s32k3xx_rxflowcontrol(struct uart_dev_s *dev,
+ unsigned int nbuffered, bool upper);
+#endif
+static void s32k3xx_send(struct uart_dev_s *dev, int ch);
+
static bool s32k3xx_txready(struct uart_dev_s *dev);
+
+#ifdef SERIAL_HAVE_TXDMA
+static void s32k3xx_dma_send(struct uart_dev_s *dev);
+static void s32k3xx_dma_txint(struct uart_dev_s *dev, bool enable);
+static void s32k3xx_dma_txavailable(struct uart_dev_s *dev);
+static void s32k3xx_dma_txcallback(DMACH_HANDLE handle, void *arg, bool done,
+ int result);
Review Comment:
Ditto
##########
arch/arm/src/s32k3xx/s32k3xx_serial.c:
##########
@@ -1059,12 +1365,49 @@ static int s32k3xx_attach(struct uart_dev_s *dev);
static void s32k3xx_detach(struct uart_dev_s *dev);
static int s32k3xx_interrupt(int irq, void *context, void *arg);
static int s32k3xx_ioctl(struct file *filep, int cmd, unsigned long arg);
+#if !defined(SERIAL_HAVE_ONLY_RXDMA)
static int s32k3xx_receive(struct uart_dev_s *dev, unsigned int *status);
static void s32k3xx_rxint(struct uart_dev_s *dev, bool enable);
static bool s32k3xx_rxavailable(struct uart_dev_s *dev);
-static void s32k3xx_send(struct uart_dev_s *dev, int ch);
+#endif
+#if !defined(SERIAL_HAVE_ONLY_TXDMA)
static void s32k3xx_txint(struct uart_dev_s *dev, bool enable);
+#endif
+
+#ifdef CONFIG_SERIAL_IFLOWCONTROL
+static bool s32k3xx_rxflowcontrol(struct uart_dev_s *dev,
+ unsigned int nbuffered, bool upper);
+#endif
+static void s32k3xx_send(struct uart_dev_s *dev, int ch);
+
static bool s32k3xx_txready(struct uart_dev_s *dev);
+
+#ifdef SERIAL_HAVE_TXDMA
+static void s32k3xx_dma_send(struct uart_dev_s *dev);
+static void s32k3xx_dma_txint(struct uart_dev_s *dev, bool enable);
+static void s32k3xx_dma_txavailable(struct uart_dev_s *dev);
+static void s32k3xx_dma_txcallback(DMACH_HANDLE handle, void *arg, bool done,
+ int result);
+#endif
+
+#if defined(SERIAL_HAVE_RXDMA) || defined(SERIAL_HAVE_TXDMA)
+static int s32k3xx_dma_setup(struct uart_dev_s *dev);
+static void s32k3xx_dma_shutdown(struct uart_dev_s *dev);
+#endif
+
+#ifdef SERIAL_HAVE_RXDMA
+static int s32k3xx_dma_receive(struct uart_dev_s *dev,
+ unsigned int *status);
+#ifdef CONFIG_PM
+static void s32k3xx_dma_reenable(struct s32k3xx_uart_s *priv);
+#endif
+static void s32k3xx_dma_rxint(struct uart_dev_s *dev, bool enable);
+static bool s32k3xx_dma_rxavailable(struct uart_dev_s *dev);
+
+static void s32k3xx_dma_rxcallback(DMACH_HANDLE handle, void *arg, bool done,
+ int result);
Review Comment:
Ditto
##########
arch/arm/src/s32k3xx/s32k3xx_serial.c:
##########
@@ -1091,99 +1435,295 @@ static const struct uart_ops_s g_uart_ops =
.rxint = s32k3xx_rxint,
.rxavailable = s32k3xx_rxavailable,
#ifdef CONFIG_SERIAL_IFLOWCONTROL
- .rxflowcontrol = NULL,
+ .rxflowcontrol = s32k3xx_rxflowcontrol,
#endif
.send = s32k3xx_send,
.txint = s32k3xx_txint,
.txready = s32k3xx_txready,
.txempty = s32k3xx_txempty,
};
+#endif
+
+#if defined(SERIAL_HAVE_RXDMA) && defined(SERIAL_HAVE_TXDMA)
+static const struct uart_ops_s g_lpuart_rxtxdma_ops =
+{
+ .setup = s32k3xx_dma_setup,
+ .shutdown = s32k3xx_dma_shutdown,
+ .attach = s32k3xx_attach,
+ .detach = s32k3xx_detach,
+ .ioctl = s32k3xx_ioctl,
+ .receive = s32k3xx_dma_receive,
+ .rxint = s32k3xx_dma_rxint,
+ .rxavailable = s32k3xx_dma_rxavailable,
+#ifdef CONFIG_SERIAL_IFLOWCONTROL
+ .rxflowcontrol = s32k3xx_rxflowcontrol,
+#endif
+ .send = s32k3xx_send,
+ .txint = s32k3xx_dma_txint,
+ .txready = s32k3xx_txready,
+ .txempty = s32k3xx_txempty,
+ .dmatxavail = s32k3xx_dma_txavailable,
+ .dmasend = s32k3xx_dma_send,
+};
+#endif
+
+#if !defined(SERIAL_HAVE_ONLY_DMA) && defined(SERIAL_HAVE_RXDMA)
+static const struct uart_ops_s g_lpuart_rxdma_ops =
+{
+ .setup = s32k3xx_dma_setup,
+ .shutdown = s32k3xx_dma_shutdown,
+ .attach = s32k3xx_attach,
+ .detach = s32k3xx_detach,
+ .ioctl = s32k3xx_ioctl,
+ .receive = s32k3xx_dma_receive,
+ .rxint = s32k3xx_dma_rxint,
+ .rxavailable = s32k3xx_dma_rxavailable,
+#ifdef CONFIG_SERIAL_IFLOWCONTROL
+ .rxflowcontrol = s32k3xx_rxflowcontrol,
+#endif
+ .send = s32k3xx_send,
+ .txint = s32k3xx_txint,
+ .txready = s32k3xx_txready,
+ .txempty = s32k3xx_txempty,
+};
+#endif
+
+#if !defined(SERIAL_HAVE_ONLY_DMA) && defined(SERIAL_HAVE_TXDMA)
+static const struct uart_ops_s g_lpuart_txdma_ops =
+{
+ .setup = s32k3xx_dma_setup,
+ .shutdown = s32k3xx_dma_shutdown,
+ .attach = s32k3xx_attach,
+ .detach = s32k3xx_detach,
+ .ioctl = s32k3xx_ioctl,
+ .receive = s32k3xx_receive,
+ .rxint = s32k3xx_rxint,
+ .rxavailable = s32k3xx_rxavailable,
+ #ifdef CONFIG_SERIAL_IFLOWCONTROL
+ .rxflowcontrol = s32k3xx_rxflowcontrol,
+ #endif
+ .send = s32k3xx_send,
+ .txint = s32k3xx_dma_txint,
+ .txready = s32k3xx_txready,
+ .txempty = s32k3xx_txempty,
+ .dmatxavail = s32k3xx_dma_txavailable,
+ .dmasend = s32k3xx_dma_send,
+};
+#endif
Review Comment:
Question: since these three variables are basically the same, could it be
possible to use #ifdefs inside the struct initialization to setup ".rxint",
".txint", ".dmatxavail" and ".dmasend" directly?
I agree that for readability doing the way you did is better, I'm thinking
more about code reduction.
--
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
