[GitHub] [incubator-nuttx] btashton commented on a change in pull request #2700: nRF52 various minor fixes

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

btashton commented on a change in pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700#discussion_r559068794



##########
File path: arch/arm/src/nrf52/nrf52_gpio.c
##########
@@ -192,6 +202,59 @@ static inline void nrf52_gpio_sense(nrf52_pinset_t cfgset,
   putreg32(regval, offset);
 }
 
+/****************************************************************************
+ * Name: nrf52_gpio_drive
+ *
+ * Description:
+ *   Set DRIVE configuration for a pin
+ *
+ ****************************************************************************/
+
+static inline void nrf52_gpio_drive(nrf52_pinset_t cfgset,

Review comment:
       This needs to take the lock or use `modifyreg32` which does it for you.  I notice that a lot of the functions here to do not do this and should probably also be updated.

##########
File path: arch/arm/src/nrf52/nrf52_lowputc.c
##########
@@ -270,7 +270,7 @@ static void nrf52_setparity(uintptr_t base,
  * Name: nrf52_setstops
  ****************************************************************************/
 
-#ifdef HAVE_UART_STOPBITS
+#if defined(HAVE_UART_DEVICE) && defined(HAVE_UART_STOPBITS)

Review comment:
       Why not just wrap all of these in `HAVE_UART_DEVICE` instead of putting it around every function?  I think that how most of the other arch are done.  Then its really easy to see the couple required functions that are not conditioned on it.




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