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/11/09 21:58:10 UTC
[GitHub] [incubator-nuttx] v01d opened a new pull request #2270: nRF52 SPI improvements
v01d opened a new pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270
## Summary
This PR does:
* A rework of #2189 which left SPI non-functional (thus the revert of that commit), including the same feature of not having to define MISO/MOSI but with a couple of required fixes found when inspecting the issue
* Support for sending more than 256 bytes of data by using DMA list feature which basically makes the SPI peripheral auto-increment the DMA pointer after each transfer. This means arbitrarily long transfers can be done in one go
* Support power management operations: disable SPI peripheral and re-enable when going into/coming back from SLEEP/STANDBY
* There is an errata (89) of nRF52 (which applies to all chip revisions) which adds 400uA static current consumption when using SPIM and GPIOTE (which is actually needed due to another workaround for SPI but can be active if used to process pin interrupts). To reduce impact of this problem, power managment can now be used to disable SPI when going to sleep. The workaround for errata 89 is to put the SPIM peripheral into a specific state once it is disabled and is thus done here.
## Impact
As above, various fixes and improvements.
## Testing
Monitored correct state change in and out of sleep and SPI functions correctly.
----------------------------------------------------------------
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 merged pull request #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl merged pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r521876964
##########
File path: arch/arm/src/nrf52/hardware/nrf52_spi.h
##########
@@ -153,13 +154,13 @@
#define SPIM_ENABLE_DIS (0) /* Disable SPIM */
#define SPIM_ENABLE_EN (0x7 << 0) /* Enable SPIM */
-/* PSEL(MOSI/MISO/SCK/CSN) Register */
+/* PSEL* Registers */
-#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: pin number */
-#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSEL_PIN_SHIFT)
-#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: port number */
-#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSEL_PORT_SHIFT)
-#define SPIM_PSEL_CONNECTED (1 << 31) /* Bit 31: Connection */
+#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: SCK pin number */
+#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSELSCK_PIN_SHIFT)
+#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: SCK port number */
+#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSELSCK_PORT_SHIFT)
+#define SPIM_PSEL_DISCONNECTED (1 << 31) /* Bit 31: Disconnect */
Review comment:
Yes, that is a good idea. We can also move all the functionality related to the PSEL configuration to one place. Kind of like `nrf52_spi_pselinit()` but common to all peripherals.
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r520761468
##########
File path: arch/arm/src/nrf52/hardware/nrf52_spi.h
##########
@@ -153,13 +154,13 @@
#define SPIM_ENABLE_DIS (0) /* Disable SPIM */
#define SPIM_ENABLE_EN (0x7 << 0) /* Enable SPIM */
-/* PSEL(MOSI/MISO/SCK/CSN) Register */
+/* PSEL* Registers */
-#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: pin number */
-#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSEL_PIN_SHIFT)
-#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: port number */
-#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSEL_PORT_SHIFT)
-#define SPIM_PSEL_CONNECTED (1 << 31) /* Bit 31: Connection */
+#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: SCK pin number */
+#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSELSCK_PIN_SHIFT)
+#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: SCK port number */
+#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSELSCK_PORT_SHIFT)
+#define SPIM_PSEL_DISCONNECTED (1 << 31) /* Bit 31: Disconnect */
Review comment:
Yes I agree it is confusing. But all other peripherals use CONNECT in PSEL definitions.
So we can decide to update all other headers or leave it unchanged.
If we decide to change, I think it should be 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] v01d commented on a change in pull request #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r520627992
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -404,6 +425,71 @@ static int nrf52_spi_isr(int irq, FAR void *context, FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: nrf52_spi_init
+ *
+ * Description:
+ * Configure SPI
+ *
+ ****************************************************************************/
+
+static int nrf52_spi_init(FAR struct nrf52_spidev_s *priv)
+{
+ /* Disable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_DIS);
+
+ /* Configure SPI pins */
+
+ nrf52_spi_gpioinit(priv);
+
+ /* NOTE: Chip select pin must be configured by board-specific logic */
+
+#ifdef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
+ /* Enable interrupts for RX and TX done */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_INTENSET_OFFSET, SPIM_INT_END);
+#endif
+
+ /* Enable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_EN);
+
+ return OK;
+}
+
+#ifdef CONFIG_PM
+/****************************************************************************
+ * Name: nrf52_spi_deinit
+ *
+ * Description:
+ * Configure SPI
+ *
+ ****************************************************************************/
+
+static int nrf52_spi_deinit(FAR struct nrf52_spidev_s *priv)
+{
+ /* Disable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_DIS);
+
+ /* Apply workaround for errata 89 (replace dummy read by barrier to avoid
Review comment:
Right, thanks, I tend to forget about other chips sometimes.
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r520336052
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -983,51 +1089,86 @@ static void nrf52_spi_exchange(FAR struct spi_dev_s *dev,
regval = (uint32_t)txbuffer;
nrf52_spi_putreg(priv, NRF52_SPIM_TXDPTR_OFFSET, regval);
-
- /* Write number of bytes in TXD buffer */
-
- regval = nwords;
- nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, regval);
}
else
{
nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, 0);
}
- /* SPI start */
+ /* If more than 255 bytes, enable list mode to send data
+ * in batches
+ */
+
+ if (nwords > 0xff)
+ {
+ if (rxbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDLIST_OFFSET, 1);
+ }
+
+ if (txbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDLIST_OFFSET, 1);
+ }
+ }
+
+ while (nwords_left > 0)
+ {
+ size_t transfer_size = (nwords_left > 255 ? 255 : nwords_left);
- nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
+ if (rxbuffer != NULL)
+ {
+ /* Write number of bytes in RXD buffer */
-#ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
- /* Wait for RX done and TX done */
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDMAXCNT_OFFSET, transfer_size);
+ }
- while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+ if (txbuffer != NULL)
+ {
+ /* Write number of bytes in TXD buffer */
- /* Clear event */
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, transfer_size);
+ }
- nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
-#else
- /* Wait for transfer complete */
+ /* SPI start */
- nxsem_wait(&priv->sem_isr);
-#endif
+ nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
- if (nrf52_spi_getreg(priv, NRF52_SPIM_TXDAMOUNT_OFFSET) != nwords)
- {
- spierr("Incomplete transfer wrote %d expected %d\n", regval, nwords);
- }
+ #ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
+ /* Wait for RX done and TX done */
+
+ while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+
+ /* Clear event */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
+ #else
Review comment:
```suggestion
#else
```
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -983,51 +1089,86 @@ static void nrf52_spi_exchange(FAR struct spi_dev_s *dev,
regval = (uint32_t)txbuffer;
nrf52_spi_putreg(priv, NRF52_SPIM_TXDPTR_OFFSET, regval);
-
- /* Write number of bytes in TXD buffer */
-
- regval = nwords;
- nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, regval);
}
else
{
nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, 0);
}
- /* SPI start */
+ /* If more than 255 bytes, enable list mode to send data
+ * in batches
+ */
+
+ if (nwords > 0xff)
+ {
+ if (rxbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDLIST_OFFSET, 1);
+ }
+
+ if (txbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDLIST_OFFSET, 1);
+ }
+ }
+
+ while (nwords_left > 0)
+ {
+ size_t transfer_size = (nwords_left > 255 ? 255 : nwords_left);
- nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
+ if (rxbuffer != NULL)
+ {
+ /* Write number of bytes in RXD buffer */
-#ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
- /* Wait for RX done and TX done */
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDMAXCNT_OFFSET, transfer_size);
+ }
- while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+ if (txbuffer != NULL)
+ {
+ /* Write number of bytes in TXD buffer */
- /* Clear event */
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, transfer_size);
+ }
- nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
-#else
- /* Wait for transfer complete */
+ /* SPI start */
- nxsem_wait(&priv->sem_isr);
-#endif
+ nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
- if (nrf52_spi_getreg(priv, NRF52_SPIM_TXDAMOUNT_OFFSET) != nwords)
- {
- spierr("Incomplete transfer wrote %d expected %d\n", regval, nwords);
- }
+ #ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
+ /* Wait for RX done and TX done */
+
+ while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+
+ /* Clear event */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
+ #else
+ /* Wait for transfer complete */
- /* SPI stop */
+ nxsem_wait_uninterruptible(&priv->sem_isr);
+ #endif
Review comment:
```suggestion
#endif
```
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r520331956
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -404,6 +425,71 @@ static int nrf52_spi_isr(int irq, FAR void *context, FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: nrf52_spi_init
+ *
+ * Description:
+ * Configure SPI
+ *
+ ****************************************************************************/
+
+static int nrf52_spi_init(FAR struct nrf52_spidev_s *priv)
+{
+ /* Disable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_DIS);
+
+ /* Configure SPI pins */
+
+ nrf52_spi_gpioinit(priv);
+
+ /* NOTE: Chip select pin must be configured by board-specific logic */
+
+#ifdef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
+ /* Enable interrupts for RX and TX done */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_INTENSET_OFFSET, SPIM_INT_END);
+#endif
+
+ /* Enable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_EN);
+
+ return OK;
+}
+
+#ifdef CONFIG_PM
+/****************************************************************************
+ * Name: nrf52_spi_deinit
+ *
+ * Description:
+ * Configure SPI
+ *
+ ****************************************************************************/
+
+static int nrf52_spi_deinit(FAR struct nrf52_spidev_s *priv)
+{
+ /* Disable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_DIS);
+
+ /* Apply workaround for errata 89 (replace dummy read by barrier to avoid
Review comment:
As I can see in errata, this only applies to nrf52832. We should not use a workaround for other chips if it is not needed.
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r520334006
##########
File path: arch/arm/src/nrf52/hardware/nrf52_spi.h
##########
@@ -153,13 +154,13 @@
#define SPIM_ENABLE_DIS (0) /* Disable SPIM */
#define SPIM_ENABLE_EN (0x7 << 0) /* Enable SPIM */
-/* PSEL(MOSI/MISO/SCK/CSN) Register */
+/* PSEL* Registers */
-#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: pin number */
-#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSEL_PIN_SHIFT)
-#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: port number */
-#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSEL_PORT_SHIFT)
-#define SPIM_PSEL_CONNECTED (1 << 31) /* Bit 31: Connection */
+#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: SCK pin number */
+#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSELSCK_PIN_SHIFT)
+#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: SCK port number */
+#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSELSCK_PORT_SHIFT)
+#define SPIM_PSEL_DISCONNECTED (1 << 31) /* Bit 31: Disconnect */
Review comment:
This bit is called CONNECT in the manual, not DISCONNECT. Shouldn't we stick to the manufacture's name?
----------------------------------------------------------------
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 pull request #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#issuecomment-725882237
LGTM
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r521387798
##########
File path: arch/arm/src/nrf52/hardware/nrf52_spi.h
##########
@@ -153,13 +154,13 @@
#define SPIM_ENABLE_DIS (0) /* Disable SPIM */
#define SPIM_ENABLE_EN (0x7 << 0) /* Enable SPIM */
-/* PSEL(MOSI/MISO/SCK/CSN) Register */
+/* PSEL* Registers */
-#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: pin number */
-#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSEL_PIN_SHIFT)
-#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: port number */
-#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSEL_PORT_SHIFT)
-#define SPIM_PSEL_CONNECTED (1 << 31) /* Bit 31: Connection */
+#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: SCK pin number */
+#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSELSCK_PIN_SHIFT)
+#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: SCK port number */
+#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSELSCK_PORT_SHIFT)
+#define SPIM_PSEL_DISCONNECTED (1 << 31) /* Bit 31: Disconnect */
Review comment:
Ok, I'll remove the change.
These PSEL definitions are the always same so we could actually do it in a single header and include it where needed.
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r520335981
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -983,51 +1089,86 @@ static void nrf52_spi_exchange(FAR struct spi_dev_s *dev,
regval = (uint32_t)txbuffer;
nrf52_spi_putreg(priv, NRF52_SPIM_TXDPTR_OFFSET, regval);
-
- /* Write number of bytes in TXD buffer */
-
- regval = nwords;
- nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, regval);
}
else
{
nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, 0);
}
- /* SPI start */
+ /* If more than 255 bytes, enable list mode to send data
+ * in batches
+ */
+
+ if (nwords > 0xff)
+ {
+ if (rxbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDLIST_OFFSET, 1);
+ }
+
+ if (txbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDLIST_OFFSET, 1);
+ }
+ }
+
+ while (nwords_left > 0)
+ {
+ size_t transfer_size = (nwords_left > 255 ? 255 : nwords_left);
- nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
+ if (rxbuffer != NULL)
+ {
+ /* Write number of bytes in RXD buffer */
-#ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
- /* Wait for RX done and TX done */
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDMAXCNT_OFFSET, transfer_size);
+ }
- while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+ if (txbuffer != NULL)
+ {
+ /* Write number of bytes in TXD buffer */
- /* Clear event */
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, transfer_size);
+ }
- nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
-#else
- /* Wait for transfer complete */
+ /* SPI start */
- nxsem_wait(&priv->sem_isr);
-#endif
+ nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
- if (nrf52_spi_getreg(priv, NRF52_SPIM_TXDAMOUNT_OFFSET) != nwords)
- {
- spierr("Incomplete transfer wrote %d expected %d\n", regval, nwords);
- }
+ #ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
Review comment:
```suggestion
#ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
```
----------------------------------------------------------------
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] acassis commented on pull request #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#issuecomment-725494691
Please @raiden00pl could you confirm all modifications are fine? LGTM?
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r520627545
##########
File path: arch/arm/src/nrf52/hardware/nrf52_spi.h
##########
@@ -153,13 +154,13 @@
#define SPIM_ENABLE_DIS (0) /* Disable SPIM */
#define SPIM_ENABLE_EN (0x7 << 0) /* Enable SPIM */
-/* PSEL(MOSI/MISO/SCK/CSN) Register */
+/* PSEL* Registers */
-#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: pin number */
-#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSEL_PIN_SHIFT)
-#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: port number */
-#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSEL_PORT_SHIFT)
-#define SPIM_PSEL_CONNECTED (1 << 31) /* Bit 31: Connection */
+#define SPIM_PSEL_PIN_SHIFT (0) /* Bits 0-4: SCK pin number */
+#define SPIM_PSEL_PIN_MASK (0x1f << SPIM_PSELSCK_PIN_SHIFT)
+#define SPIM_PSEL_PORT_SHIFT (5) /* Bit 5: SCK port number */
+#define SPIM_PSEL_PORT_MASK (0x1 << SPIM_PSELSCK_PORT_SHIFT)
+#define SPIM_PSEL_DISCONNECTED (1 << 31) /* Bit 31: Disconnect */
Review comment:
The problem is that it was "connect" with value 1, which is opposite of what it does (0 means connected), so to actually provide the expected meaning the definition should be for bit 1, which applies for "disconnect". I can change the comment if you prefer.
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r521390671
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -983,51 +1089,86 @@ static void nrf52_spi_exchange(FAR struct spi_dev_s *dev,
regval = (uint32_t)txbuffer;
nrf52_spi_putreg(priv, NRF52_SPIM_TXDPTR_OFFSET, regval);
-
- /* Write number of bytes in TXD buffer */
-
- regval = nwords;
- nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, regval);
}
else
{
nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, 0);
}
- /* SPI start */
+ /* If more than 255 bytes, enable list mode to send data
+ * in batches
+ */
+
+ if (nwords > 0xff)
+ {
+ if (rxbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDLIST_OFFSET, 1);
+ }
+
+ if (txbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDLIST_OFFSET, 1);
+ }
+ }
+
+ while (nwords_left > 0)
+ {
+ size_t transfer_size = (nwords_left > 255 ? 255 : nwords_left);
- nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
+ if (rxbuffer != NULL)
+ {
+ /* Write number of bytes in RXD buffer */
-#ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
- /* Wait for RX done and TX done */
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDMAXCNT_OFFSET, transfer_size);
+ }
- while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+ if (txbuffer != NULL)
+ {
+ /* Write number of bytes in TXD buffer */
- /* Clear event */
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, transfer_size);
+ }
- nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
-#else
- /* Wait for transfer complete */
+ /* SPI start */
- nxsem_wait(&priv->sem_isr);
-#endif
+ nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
- if (nrf52_spi_getreg(priv, NRF52_SPIM_TXDAMOUNT_OFFSET) != nwords)
- {
- spierr("Incomplete transfer wrote %d expected %d\n", regval, nwords);
- }
+ #ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
Review comment:
Done
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -983,51 +1089,86 @@ static void nrf52_spi_exchange(FAR struct spi_dev_s *dev,
regval = (uint32_t)txbuffer;
nrf52_spi_putreg(priv, NRF52_SPIM_TXDPTR_OFFSET, regval);
-
- /* Write number of bytes in TXD buffer */
-
- regval = nwords;
- nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, regval);
}
else
{
nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, 0);
}
- /* SPI start */
+ /* If more than 255 bytes, enable list mode to send data
+ * in batches
+ */
+
+ if (nwords > 0xff)
+ {
+ if (rxbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDLIST_OFFSET, 1);
+ }
+
+ if (txbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDLIST_OFFSET, 1);
+ }
+ }
+
+ while (nwords_left > 0)
+ {
+ size_t transfer_size = (nwords_left > 255 ? 255 : nwords_left);
- nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
+ if (rxbuffer != NULL)
+ {
+ /* Write number of bytes in RXD buffer */
-#ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
- /* Wait for RX done and TX done */
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDMAXCNT_OFFSET, transfer_size);
+ }
- while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+ if (txbuffer != NULL)
+ {
+ /* Write number of bytes in TXD buffer */
- /* Clear event */
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, transfer_size);
+ }
- nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
-#else
- /* Wait for transfer complete */
+ /* SPI start */
- nxsem_wait(&priv->sem_isr);
-#endif
+ nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
- if (nrf52_spi_getreg(priv, NRF52_SPIM_TXDAMOUNT_OFFSET) != nwords)
- {
- spierr("Incomplete transfer wrote %d expected %d\n", regval, nwords);
- }
+ #ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
+ /* Wait for RX done and TX done */
+
+ while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+
+ /* Clear event */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
+ #else
Review comment:
Done
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -983,51 +1089,86 @@ static void nrf52_spi_exchange(FAR struct spi_dev_s *dev,
regval = (uint32_t)txbuffer;
nrf52_spi_putreg(priv, NRF52_SPIM_TXDPTR_OFFSET, regval);
-
- /* Write number of bytes in TXD buffer */
-
- regval = nwords;
- nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, regval);
}
else
{
nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, 0);
}
- /* SPI start */
+ /* If more than 255 bytes, enable list mode to send data
+ * in batches
+ */
+
+ if (nwords > 0xff)
+ {
+ if (rxbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDLIST_OFFSET, 1);
+ }
+
+ if (txbuffer != NULL)
+ {
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDLIST_OFFSET, 1);
+ }
+ }
+
+ while (nwords_left > 0)
+ {
+ size_t transfer_size = (nwords_left > 255 ? 255 : nwords_left);
- nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
+ if (rxbuffer != NULL)
+ {
+ /* Write number of bytes in RXD buffer */
-#ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
- /* Wait for RX done and TX done */
+ nrf52_spi_putreg(priv, NRF52_SPIM_RXDMAXCNT_OFFSET, transfer_size);
+ }
- while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+ if (txbuffer != NULL)
+ {
+ /* Write number of bytes in TXD buffer */
- /* Clear event */
+ nrf52_spi_putreg(priv, NRF52_SPIM_TXDMAXCNT_OFFSET, transfer_size);
+ }
- nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
-#else
- /* Wait for transfer complete */
+ /* SPI start */
- nxsem_wait(&priv->sem_isr);
-#endif
+ nrf52_spi_putreg(priv, NRF52_SPIM_TASK_START_OFFSET, SPIM_TASKS_START);
- if (nrf52_spi_getreg(priv, NRF52_SPIM_TXDAMOUNT_OFFSET) != nwords)
- {
- spierr("Incomplete transfer wrote %d expected %d\n", regval, nwords);
- }
+ #ifndef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
+ /* Wait for RX done and TX done */
+
+ while (nrf52_spi_getreg(priv, NRF52_SPIM_EVENTS_END_OFFSET) != 1);
+
+ /* Clear event */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_EVENTS_END_OFFSET, 0);
+ #else
+ /* Wait for transfer complete */
- /* SPI stop */
+ nxsem_wait_uninterruptible(&priv->sem_isr);
+ #endif
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] raiden00pl commented on a change in pull request #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r520766478
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -404,6 +425,71 @@ static int nrf52_spi_isr(int irq, FAR void *context, FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: nrf52_spi_init
+ *
+ * Description:
+ * Configure SPI
+ *
+ ****************************************************************************/
+
+static int nrf52_spi_init(FAR struct nrf52_spidev_s *priv)
+{
+ /* Disable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_DIS);
+
+ /* Configure SPI pins */
+
+ nrf52_spi_gpioinit(priv);
+
+ /* NOTE: Chip select pin must be configured by board-specific logic */
+
+#ifdef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
+ /* Enable interrupts for RX and TX done */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_INTENSET_OFFSET, SPIM_INT_END);
+#endif
+
+ /* Enable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_EN);
+
+ return OK;
+}
+
+#ifdef CONFIG_PM
+/****************************************************************************
+ * Name: nrf52_spi_deinit
+ *
+ * Description:
+ * Configure SPI
+ *
+ ****************************************************************************/
+
+static int nrf52_spi_deinit(FAR struct nrf52_spidev_s *priv)
+{
+ /* Disable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_DIS);
+
+ /* Apply workaround for errata 89 (replace dummy read by barrier to avoid
Review comment:
Fortunately, there are no significant differences between the nrf52 chips. Usually we find them in errata, where bugs from old chips are often fixed but new bugs are introduced :P
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#issuecomment-725450300
All changes applied, I force pushed.
----------------------------------------------------------------
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 #2270: nRF52 SPI improvements
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2270:
URL: https://github.com/apache/incubator-nuttx/pull/2270#discussion_r521388471
##########
File path: arch/arm/src/nrf52/nrf52_spi.c
##########
@@ -404,6 +425,71 @@ static int nrf52_spi_isr(int irq, FAR void *context, FAR void *arg)
}
#endif
+/****************************************************************************
+ * Name: nrf52_spi_init
+ *
+ * Description:
+ * Configure SPI
+ *
+ ****************************************************************************/
+
+static int nrf52_spi_init(FAR struct nrf52_spidev_s *priv)
+{
+ /* Disable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_DIS);
+
+ /* Configure SPI pins */
+
+ nrf52_spi_gpioinit(priv);
+
+ /* NOTE: Chip select pin must be configured by board-specific logic */
+
+#ifdef CONFIG_NRF52_SPI_MASTER_INTERRUPTS
+ /* Enable interrupts for RX and TX done */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_INTENSET_OFFSET, SPIM_INT_END);
+#endif
+
+ /* Enable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_EN);
+
+ return OK;
+}
+
+#ifdef CONFIG_PM
+/****************************************************************************
+ * Name: nrf52_spi_deinit
+ *
+ * Description:
+ * Configure SPI
+ *
+ ****************************************************************************/
+
+static int nrf52_spi_deinit(FAR struct nrf52_spidev_s *priv)
+{
+ /* Disable SPI */
+
+ nrf52_spi_putreg(priv, NRF52_SPIM_ENABLE_OFFSET, SPIM_ENABLE_DIS);
+
+ /* Apply workaround for errata 89 (replace dummy read by barrier to avoid
Review comment:
I made this conditional to nrf52832
----------------------------------------------------------------
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