You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "w2016561536 (via GitHub)" <gi...@apache.org> on 2024/01/21 10:45:19 UTC
[PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
w2016561536 opened a new pull request, #11575:
URL: https://github.com/apache/nuttx/pull/11575
## Summary
Fix esp32s3 spi dma transfer.
## Impact
## Testing
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#discussion_r1462226244
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1312,22 +1337,23 @@ void esp32s3_spi_dma_init(struct spi_dev_s *dev)
{
struct esp32s3_spi_priv_s *priv = (struct esp32s3_spi_priv_s *)dev;
- /* Enable GDMA clock for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_CLK_EN0_REG, 0, priv->config->dma_clk_bit);
-
- /* Reset GDMA for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_RST_EN0_REG, priv->config->dma_rst_bit, 0);
-
Review Comment:
Please, revert your changes and keep the overall device initialization.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "w2016561536 (via GitHub)" <gi...@apache.org>.
w2016561536 commented on PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#issuecomment-1904529533
@tmedicci comment has been updated, please review 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "w2016561536 (via GitHub)" <gi...@apache.org>.
w2016561536 commented on code in PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#discussion_r1461942980
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1312,22 +1337,23 @@ void esp32s3_spi_dma_init(struct spi_dev_s *dev)
{
struct esp32s3_spi_priv_s *priv = (struct esp32s3_spi_priv_s *)dev;
- /* Enable GDMA clock for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_CLK_EN0_REG, 0, priv->config->dma_clk_bit);
-
- /* Reset GDMA for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_RST_EN0_REG, priv->config->dma_rst_bit, 0);
-
Review Comment:
In technical reference, There aren't these two registers. So i delete 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#issuecomment-1904322470
@w2016561536 , could you please:
1) Fill the description, including the testing method;
2) Apply the following patch to fix whether to use or not the DMA:
```
diff --git a/arch/xtensa/src/esp32s3/esp32s3_spi.c b/arch/xtensa/src/esp32s3/esp32s3_spi.c
index d700fb7c33..af8681b8cd 100644
--- a/arch/xtensa/src/esp32s3/esp32s3_spi.c
+++ b/arch/xtensa/src/esp32s3/esp32s3_spi.c
@@ -1223,7 +1223,7 @@ static void esp32s3_spi_exchange(struct spi_dev_s *dev,
#ifdef CONFIG_ESP32S3_SPI_DMA
size_t thld = CONFIG_ESP32S3_SPI_DMATHRESHOLD;
- if (nwords > thld)
+ if ((nwords * priv->nbits) / 8 > thld)
{
esp32s3_spi_dma_exchange(priv, txbuffer, rxbuffer, nwords);
}
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "w2016561536 (via GitHub)" <gi...@apache.org>.
w2016561536 commented on code in PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#discussion_r1462235321
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1312,22 +1337,23 @@ void esp32s3_spi_dma_init(struct spi_dev_s *dev)
{
struct esp32s3_spi_priv_s *priv = (struct esp32s3_spi_priv_s *)dev;
- /* Enable GDMA clock for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_CLK_EN0_REG, 0, priv->config->dma_clk_bit);
-
- /* Reset GDMA for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_RST_EN0_REG, priv->config->dma_rst_bit, 0);
-
Review Comment:
> Please, revert your changes and keep the overall device initialization.
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#issuecomment-1902692130
I'll double-check it tomorrow. Please don't merge it yet.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#discussion_r1462077674
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1312,22 +1337,23 @@ void esp32s3_spi_dma_init(struct spi_dev_s *dev)
{
struct esp32s3_spi_priv_s *priv = (struct esp32s3_spi_priv_s *)dev;
- /* Enable GDMA clock for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_CLK_EN0_REG, 0, priv->config->dma_clk_bit);
-
- /* Reset GDMA for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_RST_EN0_REG, priv->config->dma_rst_bit, 0);
-
Review Comment:
It's the overall peripheral initialization. You can keep 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #11575:
URL: https://github.com/apache/nuttx/pull/11575
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#discussion_r1460906784
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -979,6 +988,14 @@ static void esp32s3_spi_dma_exchange(struct esp32s3_spi_priv_s *priv,
static uint32_t esp32s3_spi_poll_send(struct esp32s3_spi_priv_s *priv,
uint32_t wd)
{
+#ifdef CONFIG_ESP32S3_SPI_DMA
+ esp32s3_spi_clr_regbits(SPI_DMA_CONF_REG(priv->config->id),
+ SPI_DMA_TX_ENA_M);
+
+ esp32s3_spi_clr_regbits(SPI_DMA_CONF_REG(priv->config->id),
+ SPI_DMA_RX_ENA_M);
Review Comment:
```suggestion
SPI_DMA_RX_ENA_M);
```
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -979,6 +988,14 @@ static void esp32s3_spi_dma_exchange(struct esp32s3_spi_priv_s *priv,
static uint32_t esp32s3_spi_poll_send(struct esp32s3_spi_priv_s *priv,
uint32_t wd)
{
+#ifdef CONFIG_ESP32S3_SPI_DMA
+ esp32s3_spi_clr_regbits(SPI_DMA_CONF_REG(priv->config->id),
+ SPI_DMA_TX_ENA_M);
Review Comment:
```suggestion
SPI_DMA_TX_ENA_M);
```
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1059,6 +1076,14 @@ static void esp32s3_spi_poll_exchange(struct esp32s3_spi_priv_s *priv,
void *rxbuffer,
size_t nwords)
{
+#ifdef CONFIG_ESP32S3_SPI_DMA
+ esp32s3_spi_clr_regbits(SPI_DMA_CONF_REG(priv->config->id),
+ SPI_DMA_TX_ENA_M);
Review Comment:
ditto
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1059,6 +1076,14 @@ static void esp32s3_spi_poll_exchange(struct esp32s3_spi_priv_s *priv,
void *rxbuffer,
size_t nwords)
{
+#ifdef CONFIG_ESP32S3_SPI_DMA
+ esp32s3_spi_clr_regbits(SPI_DMA_CONF_REG(priv->config->id),
+ SPI_DMA_TX_ENA_M);
+
+ esp32s3_spi_clr_regbits(SPI_DMA_CONF_REG(priv->config->id),
+ SPI_DMA_RX_ENA_M);
Review Comment:
ditto
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#discussion_r1461833762
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1312,22 +1337,23 @@ void esp32s3_spi_dma_init(struct spi_dev_s *dev)
{
struct esp32s3_spi_priv_s *priv = (struct esp32s3_spi_priv_s *)dev;
- /* Enable GDMA clock for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_CLK_EN0_REG, 0, priv->config->dma_clk_bit);
-
- /* Reset GDMA for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_RST_EN0_REG, priv->config->dma_rst_bit, 0);
-
Review Comment:
Why did you remove device initialization? I recommend reverting it...
##########
arch/xtensa/src/esp32s3/esp32s3_dma.h:
##########
@@ -64,7 +64,7 @@ extern "C"
/* DMA channel number */
-#define ESP32S3_DMA_CHAN_MAX (3)
Review Comment:
@acassis , do you recall why didn't you assign all 5 GDMA channels to S3? I looked for uses without using the `esp32s3_dma_request` "API" and I couldn't fin anything using GDMA channels directly.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "w2016561536 (via GitHub)" <gi...@apache.org>.
w2016561536 commented on PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#issuecomment-1905338985
Hello? Is anyone could help me to merge this 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "w2016561536 (via GitHub)" <gi...@apache.org>.
w2016561536 commented on code in PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#discussion_r1462235321
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1312,22 +1337,23 @@ void esp32s3_spi_dma_init(struct spi_dev_s *dev)
{
struct esp32s3_spi_priv_s *priv = (struct esp32s3_spi_priv_s *)dev;
- /* Enable GDMA clock for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_CLK_EN0_REG, 0, priv->config->dma_clk_bit);
-
- /* Reset GDMA for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_RST_EN0_REG, priv->config->dma_rst_bit, 0);
-
Review Comment:
> Please, revert your changes and keep the overall device initialization.
OK. I trust you.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "w2016561536 (via GitHub)" <gi...@apache.org>.
w2016561536 commented on code in PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#discussion_r1462230015
##########
arch/xtensa/src/esp32s3/esp32s3_spi.c:
##########
@@ -1312,22 +1337,23 @@ void esp32s3_spi_dma_init(struct spi_dev_s *dev)
{
struct esp32s3_spi_priv_s *priv = (struct esp32s3_spi_priv_s *)dev;
- /* Enable GDMA clock for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_CLK_EN0_REG, 0, priv->config->dma_clk_bit);
-
- /* Reset GDMA for the SPI peripheral */
-
- modifyreg32(SYSTEM_PERIP_RST_EN0_REG, priv->config->dma_rst_bit, 0);
-
Review Comment:
[https://www.espressif.com.cn/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf#page=810](url)
But, in SYSTEM_PERIP_CLK_EN0_REG, bit 22 and bit 27 are reserved. In SYSTEM_PERIP_CLK_EN1_REG, bit 22 and bit 27 are reserved either. In function ` esp32s3_dma_init() ` , dma clock will be enabled and reset status will be removed.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] esp32s3/spi-dma: Fix spi dma transfer. [nuttx]
Posted by "w2016561536 (via GitHub)" <gi...@apache.org>.
w2016561536 commented on PR #11575:
URL: https://github.com/apache/nuttx/pull/11575#issuecomment-1904480103
I'm working on test now. A detailed description will be added to first conversation soon.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org