[GitHub] [mynewt-core] kasjer commented on a change in pull request #2610: nxp: kinetis: decrease LPUART RAM usage

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

kasjer commented on a change in pull request #2610:
URL: https://github.com/apache/mynewt-core/pull/2610#discussion_r642848845



##########
File path: hw/mcu/nxp/kinetis/src/hal_lpuart.c
##########
@@ -345,12 +433,15 @@ hal_uart_config(int port,
 {
     struct hal_uart *u;
     lpuart_config_t uconfig;
+    void (*uart_irq_fn)(void);
 
-    if (port >= FSL_FEATURE_SOC_LPUART_COUNT) {
+    u = uart_by_port(port);
+    if (!u || !u->u_configured || u->u_open) {
         return -1;
     }
-    u = &uarts[port];
-    if (!u->u_configured || u->u_open) {
+
+    uart_irq_fn = s_uartirqs[port];
+    if (!uart_irq_fn) {

Review comment:
       this check will never fail, it seems

##########
File path: hw/mcu/nxp/kinetis/src/hal_lpuart.c
##########
@@ -410,7 +501,8 @@ hal_uart_config(int port,
     uconfig.enableTx = true;
     uconfig.enableRx = true;
 
-    NVIC_SetVector(u->u_irq, (uint32_t)s_uartirqs[port]);
+

Review comment:
       extra empty line

##########
File path: hw/mcu/nxp/kinetis/src/hal_lpuart.c
##########
@@ -70,25 +70,112 @@ struct hal_uart {
 };
 
 /* UART configurations */
-static struct hal_uart uarts[FSL_FEATURE_SOC_LPUART_COUNT];
+#define UART_CNT (((uint8_t)(MYNEWT_VAL(UART_0) != 0)) + \
+                  ((uint8_t)(MYNEWT_VAL(UART_1) != 0)) + \
+                  ((uint8_t)(MYNEWT_VAL(UART_2) != 0)) + \
+                  ((uint8_t)(MYNEWT_VAL(UART_3) != 0)) + \
+                  ((uint8_t)(MYNEWT_VAL(UART_4) != 0)))
+
+static struct hal_uart uarts[UART_CNT];
+static struct hal_uart *
+uart_by_port(int port)
+{
+    int index;
+
+    (void)index;
+    (void)uarts;
+
+    index = 0;
+#if MYNEWT_VAL(UART_0)
+    if (port == 0) {
+        return &uarts[index];
+    }
+    index++;
+#endif
+#if MYNEWT_VAL(UART_1)
+    if (port == 1) {
+        return &uarts[index];
+    }
+    index++;
+#endif
+#if MYNEWT_VAL(UART_2)
+#   if FSL_FEATURE_SOC_LPUART_COUNT < 3
+#   error "UART_2 is not supported on this MCU"
+#   endif
+    if (port == 2) {
+        return &uarts[index];
+    }
+    index++;
+#endif
+#if MYNEWT_VAL(UART_3)
+#   if FSL_FEATURE_SOC_LPUART_COUNT < 4
+#   error "UART_3 is not supported on this MCU"
+#   endif
+    if (port == 3) {
+        return &uarts[index];
+    }
+    index++;
+#endif
+#if MYNEWT_VAL(UART_4)
+#   if FSL_FEATURE_SOC_LPUART_COUNT < 5
+#   error "UART_4 is not supported on this MCU"
+#   endif
+    if (port == 4) {
+        return &uarts[index];
+    }
+#endif
+    return NULL;
+};
 
-static uint8_t const s_uartExists[] = NXP_UART_EXISTS;
-static uint8_t const s_uartEnabled[] = NXP_UART_ENABLED;
 static LPUART_Type *const s_uartBases[] = LPUART_BASE_PTRS;
-static clock_name_t s_uartClocks[] = NXP_UART_CLOCKS;
 static uint8_t const s_uartIRQ[] = LPUART_RX_TX_IRQS;
 static PORT_Type *const s_uartPort[] = NXP_UART_PORTS;
 static clock_ip_name_t const s_uartPortClocks[] = NXP_UART_PORT_CLOCKS;
 static uint8_t const s_uartPIN_RX[] = NXP_UART_PIN_RX;
 static uint8_t const s_uartPIN_TX[] = NXP_UART_PIN_TX;
 
+#if MYNEWT_VAL(UART_0)
 static void uart_irq0(void);
+#endif
+#if MYNEWT_VAL(UART_1)
 static void uart_irq1(void);
+#endif
+#if MYNEWT_VAL(UART_2)
 static void uart_irq2(void);
+#endif
+#if MYNEWT_VAL(UART_3)
 static void uart_irq3(void);
+#endif
+#if MYNEWT_VAL(UART_4)
 static void uart_irq4(void);
+#endif
+
 static void (*s_uartirqs[])(void) = {

Review comment:
       While it's not part of a change s_uartirqs could be const

##########
File path: hw/mcu/nxp/kinetis/src/hal_lpuart.c
##########
@@ -70,25 +70,112 @@ struct hal_uart {
 };
 
 /* UART configurations */
-static struct hal_uart uarts[FSL_FEATURE_SOC_LPUART_COUNT];
+#define UART_CNT (((uint8_t)(MYNEWT_VAL(UART_0) != 0)) + \
+                  ((uint8_t)(MYNEWT_VAL(UART_1) != 0)) + \
+                  ((uint8_t)(MYNEWT_VAL(UART_2) != 0)) + \
+                  ((uint8_t)(MYNEWT_VAL(UART_3) != 0)) + \
+                  ((uint8_t)(MYNEWT_VAL(UART_4) != 0)))
+
+static struct hal_uart uarts[UART_CNT];
+static struct hal_uart *
+uart_by_port(int port)
+{
+    int index;
+
+    (void)index;
+    (void)uarts;
+
+    index = 0;
+#if MYNEWT_VAL(UART_0)
+    if (port == 0) {
+        return &uarts[index];
+    }
+    index++;
+#endif
+#if MYNEWT_VAL(UART_1)
+    if (port == 1) {
+        return &uarts[index];
+    }
+    index++;
+#endif
+#if MYNEWT_VAL(UART_2)
+#   if FSL_FEATURE_SOC_LPUART_COUNT < 3
+#   error "UART_2 is not supported on this MCU"
+#   endif
+    if (port == 2) {
+        return &uarts[index];
+    }
+    index++;
+#endif
+#if MYNEWT_VAL(UART_3)
+#   if FSL_FEATURE_SOC_LPUART_COUNT < 4
+#   error "UART_3 is not supported on this MCU"

Review comment:
       Slight inconsistency here, if `#if` is indented why not indent `#error` as well




-- 
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