You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/01/16 22:40:10 UTC
[GitHub] [incubator-nuttx] v01d opened a new pull request #2700: nRF52 various minor fixes
v01d opened a new pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700
## Summary
- nrf52: fix build without serial
- nrf52: fix build for PWM without multichan enabled
- nrf52: fix SPI3 irq macro naming
- nrf52: enable and fix build for SPI BITORDER
- nrf52 spi: build fixes and a missing register setting (polarity)
- nrf52 GPIO: set GPIO drive setting and missing input buffer configuration
- nrf52 i2c: disable peripheral while configuring
## Impact
Minor fixes (functional and build)
## Testing
nRF52840 on custom board/config
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2700: nRF52 various minor fixes
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700#discussion_r559069637
##########
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:
done
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] btashton merged pull request #2700: nRF52 various minor fixes
Posted by GitBox <gi...@apache.org>.
btashton merged pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2700: nRF52 various minor fixes
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700#discussion_r559070817
##########
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:
ok, I followed your suggestion, let me know if it looks OK
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] btashton commented on a change in pull request #2700: nRF52 various minor fixes
Posted by 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
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2700: nRF52 various minor fixes
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700#discussion_r559069376
##########
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:
since this would then require changing various other parts of this code do you mind if I do it in a separate PR?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] btashton commented on a change in pull request #2700: nRF52 various minor fixes
Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700#discussion_r559070282
##########
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:
Sure, its probably fine to just take the lock on the public functions that need it since there is not waiting going on in the private ones. It looks like that is only `nrf52_gpio_config` so very minor change just calling `spin_lock_irqsave` at the top and `spin_unlock_irqrestore(flags)` at the end. Do you want to do that change or should I?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] btashton commented on a change in pull request #2700: nRF52 various minor fixes
Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700#discussion_r559070282
##########
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:
Sure, its probably fine to just take the lock on the public functions that need it since there is not waiting going on in the private ones. It looks like that is only `nrf52_gpio_config` so very minor change just calling `spin_lock_irqsave` at the top. Do you want to do that change or should I?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] btashton commented on a change in pull request #2700: nRF52 various minor fixes
Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #2700:
URL: https://github.com/apache/incubator-nuttx/pull/2700#discussion_r559071007
##########
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:
Looks good. Thanks!
----------------------------------------------------------------
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