linguini1 commented on code in PR #18907:
URL: https://github.com/apache/nuttx/pull/18907#discussion_r3275805907


##########
boards/arm64/am62x/beagleplay/src/beagleplay_appinit.c:
##########
@@ -0,0 +1,69 @@
+/****************************************************************************
+ * boards/arm64/am62x/beagleplay/src/beagleplay_appinit.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <sys/types.h>
+#include <nuttx/board.h>
+#include "beagleplay.h"
+
+#ifdef CONFIG_BOARDCTL
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: board_app_initialize
+ *
+ * Description:
+ *   Called indirectly through boardctl(BOARDIOC_INIT) from the NSH start-up
+ *   code.  This is the application-level initialisation hook — it runs in
+ *   task context (not interrupt context) with the scheduler fully running.
+ *
+ *   When CONFIG_BOARD_LATE_INITIALIZE is set, beagleplay_bringup() has
+ *   already been called from board_late_initialize() so we return OK
+ *   immediately.  Otherwise we call it here.
+ *
+ * Input Parameters:
+ *   arg - boardctl() argument (unused for BOARDIOC_INIT)
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno on failure.
+ *
+ ****************************************************************************/
+
+int board_app_initialize(uintptr_t arg)
+{
+  UNUSED(arg);
+
+#ifndef CONFIG_BOARD_LATE_INITIALIZE
+  return beagleplay_bringup();
+#else
+  return OK;
+#endif
+}
+

Review Comment:
   This interface is deprecated; please test this on the latest main using 
`board_late_initialize`.



##########
boards/arm64/am62x/beagleplay/src/beagleplay_autoleds.c:
##########
@@ -0,0 +1,134 @@
+/****************************************************************************
+ * boards/arm64/am62x/beagleplay/src/beagleplay_autoleds.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* LED state encoding (see board.h):
+ *
+ *   SYMBOL                     USR0  USR1  USR2
+ *   ----------------------     ----  ----  ----
+ *   LED_STARTED                 ON    OFF   OFF
+ *   LED_HEAPALLOCATE            ON    ON    OFF
+ *   LED_IRQSENABLED             ON    ON    ON
+ *   LED_STACKCREATED            OFF   ON    OFF
+ *   LED_PANIC                   ---  blink  ---
+ *
+ * NOTE: GPIO driver not yet implemented.  The LED functions are stubs that
+ * will be filled in when the AM62x GPIO driver is added in a later phase.
+ * The architecture LED calls compile and link now so the overall board
+ * configuration is valid.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/board.h>
+#include <arch/board/board.h>
+#include "beagleplay.h"
+
+#ifdef CONFIG_ARCH_LEDS
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Tracks whether we are in a panic so board_autoled_on/off know to blink */
+
+static bool g_panic;
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: beagleplay_led_initialize
+ *
+ * Description:
+ *   Configure LED GPIO pins as outputs, all OFF initially.
+ *   Stubbed until the GPIO driver is available.
+ *
+ ****************************************************************************/
+
+void beagleplay_led_initialize(void)
+{
+  /* TODO: configure GPIO pins for USR0/USR1/USR2 LEDs once GPIO driver
+   * is implemented.
+   */
+}
+
+/****************************************************************************
+ * Name: board_autoled_on
+ *
+ * Description:
+ *   Turn ON the LED(s) associated with the OS event 'led'.
+ *
+ ****************************************************************************/
+
+void board_autoled_on(int led)
+{
+  switch (led)
+    {
+      case LED_STARTED:
+        break;
+
+      case LED_HEAPALLOCATE:
+        break;
+
+      case LED_IRQSENABLED:
+        break;
+
+      case LED_STACKCREATED:
+        break;
+
+      case LED_PANIC:
+        g_panic = true;
+        break;
+
+      default:
+        break;
+    }
+}
+
+/****************************************************************************
+ * Name: board_autoled_off
+ *
+ * Description:
+ *   Turn OFF the LED(s) associated with the OS event 'led'.
+ *
+ ****************************************************************************/
+
+void board_autoled_off(int led)
+{
+  switch (led)
+    {
+      case LED_PANIC:
+        if (g_panic)
+          {
+          }

Review Comment:
   This condition does nothing.



##########
Documentation/platforms/arm64/am62x/boards/beagleplay/index.rst:
##########


Review Comment:
   Please follow the template in 
`Documentation/contributing/doc_templates/board.rst`



##########
boards/arm64/am62x/beagleplay/src/beagleplay_autoleds.c:
##########
@@ -0,0 +1,134 @@
+/****************************************************************************
+ * boards/arm64/am62x/beagleplay/src/beagleplay_autoleds.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* LED state encoding (see board.h):
+ *
+ *   SYMBOL                     USR0  USR1  USR2
+ *   ----------------------     ----  ----  ----
+ *   LED_STARTED                 ON    OFF   OFF
+ *   LED_HEAPALLOCATE            ON    ON    OFF
+ *   LED_IRQSENABLED             ON    ON    ON
+ *   LED_STACKCREATED            OFF   ON    OFF
+ *   LED_PANIC                   ---  blink  ---
+ *
+ * NOTE: GPIO driver not yet implemented.  The LED functions are stubs that
+ * will be filled in when the AM62x GPIO driver is added in a later phase.
+ * The architecture LED calls compile and link now so the overall board
+ * configuration is valid.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/board.h>
+#include <arch/board/board.h>
+#include "beagleplay.h"
+
+#ifdef CONFIG_ARCH_LEDS
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Tracks whether we are in a panic so board_autoled_on/off know to blink */
+
+static bool g_panic;
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: beagleplay_led_initialize
+ *
+ * Description:
+ *   Configure LED GPIO pins as outputs, all OFF initially.
+ *   Stubbed until the GPIO driver is available.
+ *
+ ****************************************************************************/
+
+void beagleplay_led_initialize(void)
+{
+  /* TODO: configure GPIO pins for USR0/USR1/USR2 LEDs once GPIO driver
+   * is implemented.
+   */
+}
+
+/****************************************************************************
+ * Name: board_autoled_on
+ *
+ * Description:
+ *   Turn ON the LED(s) associated with the OS event 'led'.
+ *
+ ****************************************************************************/
+
+void board_autoled_on(int led)
+{
+  switch (led)
+    {
+      case LED_STARTED:
+        break;
+
+      case LED_HEAPALLOCATE:
+        break;
+
+      case LED_IRQSENABLED:
+        break;
+
+      case LED_STACKCREATED:
+        break;
+
+      case LED_PANIC:
+        g_panic = true;

Review Comment:
   You don't need global variables for this.



##########
boards/arm64/am62x/pocketbeagle2/include/board.h:
##########
@@ -0,0 +1,100 @@
+/****************************************************************************
+ * boards/arm64/am62x/pocketbeagle2/include/board.h
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* PocketBeagle 2 board definitions */
+
+#ifndef __BOARDS_ARM64_AM62X_POCKETBEAGLE2_INCLUDE_BOARD_H
+#define __BOARDS_ARM64_AM62X_POCKETBEAGLE2_INCLUDE_BOARD_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Clocking *****************************************************************/
+
+/* UART functional clock, sourced from HSDIV4_CLKOUT1 as configured by the
+ * R5 SYSFW.  Matches AM62X_UART_SCLK in hardware/am62x_uart.h.
+ */
+
+#define BOARD_UART_SCLK         48000000ul  /* 48 MHz */
+
+/* ARM Generic Timer (CNTPCT_EL0) reference clock.
+ * On AM62x this is driven by the 200 MHz SYSCLK0 / 4 path.
+ * Configured in defconfig via CONFIG_ARCH_TIMER_FREQ.
+ */
+
+#define BOARD_ARCH_TIMER_FREQ   200000000ul /* 200 MHz */
+
+/* LED definitions *********************************************************
+ *
+ * PocketBeagle 2 user LEDs are connected to GPIO pins on the AM6254.
+ * The exact GPIO numbers will be finalised when the GPIO driver is added;
+ * these defines are placeholders so board code can compile now.
+ *
+ * LED index   Colour   Net name
+ *   0          USR0     Green
+ *   1          USR1     Yellow/Amber
+ *   2          USR2     Red
+ */
+
+typedef enum
+{
+  BOARD_LED_USR0 = 0,
+  BOARD_LED_USR1 = 1,
+  BOARD_LED_USR2 = 2,
+  BOARD_NLEDS,
+} led_typedef_enum;

Review Comment:
   Why a typedef enum?



##########
drivers/serial/uart_16550.c:
##########
@@ -807,11 +807,6 @@ static int u16550_setup(FAR struct uart_dev_s *dev)
       return -EPERM;
     }
 
-  /* Clear fifos */

Review Comment:
   Why is it necessary to delete this? Could you explain what the issues were 
with the 16550 driver?



##########
boards/arm64/am62x/pocketbeagle2/src/pocketbeagle2_autoleds.c:
##########
@@ -0,0 +1,134 @@
+/****************************************************************************
+ * boards/arm64/am62x/pocketbeagle2/src/pocketbeagle2_autoleds.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* LED state encoding (see board.h):
+ *
+ *   SYMBOL                     USR0  USR1  USR2
+ *   ----------------------     ----  ----  ----
+ *   LED_STARTED                 ON    OFF   OFF
+ *   LED_HEAPALLOCATE            ON    ON    OFF
+ *   LED_IRQSENABLED             ON    ON    ON
+ *   LED_STACKCREATED            OFF   ON    OFF
+ *   LED_PANIC                   ---  blink  ---
+ *
+ * NOTE: GPIO driver not yet implemented.  The LED functions are stubs that
+ * will be filled in when the AM62x GPIO driver is added in a later phase.
+ * The architecture LED calls compile and link now so the overall board
+ * configuration is valid.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/board.h>
+#include <arch/board/board.h>
+#include "pocketbeagle2.h"
+
+#ifdef CONFIG_ARCH_LEDS
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Tracks whether we are in a panic so board_autoled_on/off know to blink */
+
+static bool g_panic;
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pocketbeagle2_led_initialize
+ *
+ * Description:
+ *   Configure LED GPIO pins as outputs, all OFF initially.
+ *   Stubbed until the GPIO driver is available.
+ *
+ ****************************************************************************/
+
+void pocketbeagle2_led_initialize(void)
+{
+  /* TODO: configure GPIO pins for USR0/USR1/USR2 LEDs once GPIO driver
+   * is implemented.
+   */
+}
+
+/****************************************************************************
+ * Name: board_autoled_on
+ *
+ * Description:
+ *   Turn ON the LED(s) associated with the OS event 'led'.
+ *
+ ****************************************************************************/
+
+void board_autoled_on(int led)
+{
+  switch (led)
+    {
+      case LED_STARTED:
+        break;
+
+      case LED_HEAPALLOCATE:
+        break;
+
+      case LED_IRQSENABLED:
+        break;
+
+      case LED_STACKCREATED:
+        break;
+
+      case LED_PANIC:
+        g_panic = true;
+        break;
+
+      default:
+        break;
+    }
+}
+
+/****************************************************************************
+ * Name: board_autoled_off
+ *
+ * Description:
+ *   Turn OFF the LED(s) associated with the OS event 'led'.
+ *
+ ****************************************************************************/
+
+void board_autoled_off(int led)
+{
+  switch (led)
+    {
+      case LED_PANIC:
+        if (g_panic)
+          {
+          }
+        break;

Review Comment:
   Ditto



##########
Documentation/platforms/arm64/am62x/boards/pocketbeagle2/index.rst:
##########


Review Comment:
   Please follow the template in 
`Documentation/contributing/doc_templates/board.rst`



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to