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 2020/08/29 15:47:52 UTC
[GitHub] [incubator-nuttx] v01d opened a new pull request #1662: nRF52: add support for GPIOTE TASK mode
v01d opened a new pull request #1662:
URL: https://github.com/apache/incubator-nuttx/pull/1662
## Summary
This PR extends GPIOTE peripheral interface to support setting a pin to TASK mode (previously only EVENT mode supported). This also better distinguishes NRF52832/NRF52840 as some definitions do not apply. License header for these two files updated to Apache as authors have ICLA.
## Impact
New feature.
## Testing
Pins can be set correctly from task interface.
----------------------------------------------------------------
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 closed pull request #1662: nRF52: add support for GPIOTE TASK mode
Posted by GitBox <gi...@apache.org>.
v01d closed pull request #1662:
URL: https://github.com/apache/incubator-nuttx/pull/1662
----------------------------------------------------------------
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 #1662: nRF52: add support for GPIOTE TASK mode
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1662:
URL: https://github.com/apache/incubator-nuttx/pull/1662#discussion_r479797314
##########
File path: arch/arm/src/nrf52/nrf52_gpiote.h
##########
@@ -72,6 +57,10 @@
int nrf52_gpiosetevent(uint32_t pinset, bool risingedge, bool fallingedge,
bool event, xcpt_t func, FAR void *arg);
+int nrf52_gpio_taskset(uint32_t pinset, int channel,
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] v01d commented on a change in pull request #1662: nRF52: add support for GPIOTE TASK mode
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1662:
URL: https://github.com/apache/incubator-nuttx/pull/1662#discussion_r479779665
##########
File path: arch/arm/src/nrf52/nrf52_gpiote.c
##########
@@ -183,7 +168,9 @@ int nrf52_gpiosetevent(uint32_t pinset, bool risingedge, bool fallingedge,
int ret = OK;
int i = 0;
int pin = 0;
+#ifdef CONFIG_ARCH_CHIP_NRF52840
Review comment:
I've seen your other PR relating to this. It should be then changed to a HAVE_* ifdef I guess then. I can merge your PR first and then adapt this.
----------------------------------------------------------------
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] raiden00pl commented on a change in pull request #1662: nRF52: add support for GPIOTE TASK mode
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #1662:
URL: https://github.com/apache/incubator-nuttx/pull/1662#discussion_r479788259
##########
File path: arch/arm/src/nrf52/nrf52_gpiote.c
##########
@@ -183,7 +168,9 @@ int nrf52_gpiosetevent(uint32_t pinset, bool risingedge, bool fallingedge,
int ret = OK;
int i = 0;
int pin = 0;
+#ifdef CONFIG_ARCH_CHIP_NRF52840
Review comment:
`NRF52_HAVE_PORT1` is already defined in Kconfig so if you use `#ifdef CONFIG_NRF52_HAVE_PORT1` it should work without my 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] v01d commented on pull request #1662: nRF52: add support for GPIOTE TASK mode
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1662:
URL: https://github.com/apache/incubator-nuttx/pull/1662#issuecomment-683449857
I've performed the suggested changes. I also documented this new function. At the same time, I improved and simplified its interface since it would otherwise allow for invalid configurations for the GPIOTE channel.
----------------------------------------------------------------
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] raiden00pl commented on a change in pull request #1662: nRF52: add support for GPIOTE TASK mode
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #1662:
URL: https://github.com/apache/incubator-nuttx/pull/1662#discussion_r479742876
##########
File path: arch/arm/src/nrf52/nrf52_gpiote.c
##########
@@ -285,3 +275,61 @@ int nrf52_gpiote_init(void)
return OK;
}
+
+int nrf52_gpio_taskset(uint32_t pinset, int channel,
+ bool output_high, bool set_task, bool clear_task,
+ bool toggle_task)
+{
+ /* TODO: find if i conflicts with interrupt */
+
+ uint32_t regval;
+ int pin;
+
+ /* Select GPIOTE pin */
+
+ pin = (pinset & GPIO_PIN_MASK) >> GPIO_PIN_SHIFT;
+ regval = (pin << GPIOTE_CONFIG_PSEL_SHIFT);
+
+#ifdef CONFIG_ARCH_CHIP_NRF52840
Review comment:
Same as above.
##########
File path: arch/arm/src/nrf52/nrf52_gpiote.h
##########
@@ -72,6 +57,10 @@
int nrf52_gpiosetevent(uint32_t pinset, bool risingedge, bool fallingedge,
bool event, xcpt_t func, FAR void *arg);
+int nrf52_gpio_taskset(uint32_t pinset, int channel,
Review comment:
Let's stick to the naming convention that is already in use.
```suggestion
int nrf52_gpiotaskset(uint32_t pinset, int channel,
```
##########
File path: arch/arm/src/nrf52/nrf52_gpiote.c
##########
@@ -183,7 +168,9 @@ int nrf52_gpiosetevent(uint32_t pinset, bool risingedge, bool fallingedge,
int ret = OK;
int i = 0;
int pin = 0;
+#ifdef CONFIG_ARCH_CHIP_NRF52840
Review comment:
It was completely intentional and should have stayed the way it was. There are more chips supporting more than 1 gpio port in the nrf52 family. Adding more chips to the #ifdef conditions will make the code unreadable.
The arch code should be as general as possible and avoid special cases for single chips as much as possible. Otherwise we'll end up with the spaghetti code as in some files in arch/stm32.
----------------------------------------------------------------
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] raiden00pl commented on a change in pull request #1662: nRF52: add support for GPIOTE TASK mode
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #1662:
URL: https://github.com/apache/incubator-nuttx/pull/1662#discussion_r479904588
##########
File path: arch/arm/src/nrf52/nrf52_gpiote.c
##########
@@ -285,3 +290,79 @@ int nrf52_gpiote_init(void)
return OK;
}
+
+/****************************************************************************
+ * Name: nrf52_gpiotaskset
+ *
+ * Description:
+ * Configure GPIO in TASK mode (to be controlled via tasks).
+ * Note that a pin can only be either in TASK or EVENT mode (set by
+ * nrf52_gpiosetevent with event set to true). Also, once set to TASK mode,
+ * pin control is only possible via tasks on the via nrf52_gpio_write and
+ * will automatically set the output mode.
+ * Finally, a given pin should only be assigned to a given channel.
+ *
+ * Input Parameters:
+ * - pinset: gpio pin configuration (only port + pin is important here)
+ * - channel: the GPIOTE channel used to control the given pin
+ * - output_high: set pin initially to output HIGH or LOW.
+ * - outcfg: configure pin behavior one OUT task is triggered
+ *
+ * Returned Value:
+ * Zero (OK) on success; a negated errno value on failure indicating the
+ * nature of the failure.
+ *
+ ****************************************************************************/
+
+int nrf52_gpiotaskset(uint32_t pinset, int channel,
+ bool output_high, enum nrf52_gpiote_outcfg_e outcfg)
+{
+ uint32_t regval;
+ int pin;
+
Review comment:
```suggestion
#ifdef CONFIG_NRF52_HAVE_PORT1
int port = 0;
#endif
```
Just what I noticed :)
----------------------------------------------------------------
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