yamt commented on a change in pull request #1958:
URL: https://github.com/apache/incubator-nuttx/pull/1958#discussion_r503727146



##########
File path: arch/xtensa/Kconfig
##########
@@ -81,6 +81,21 @@ config XTENSA_CP_INITSET
                is provided by CONFIG_XTENSA_CP_INITSET.  Each bit corresponds 
to one
                coprocessor with the same bit layout as for the CPENABLE 
register.
 
+config XTENSA_USE_SEPERATE_IMEM
+       bool "Use a seperate heap for internal memory"
+       default n
+
+config XTENSA_IMEM_REGION_SIZE
+       hex "DRAM region size for internal use"
+       depends on XTENSA_USE_SEPERATE_IMEM
+       range 0x2000 0x28000
+       default 0x28000
+
+config XTENSA_IMEM_PROCFS

Review comment:
       i feel it's more convenient for users to have this info in /proc/meminfo 
than the dedicated file. how do you think?

##########
File path: arch/xtensa/Kconfig
##########
@@ -81,6 +81,21 @@ config XTENSA_CP_INITSET
                is provided by CONFIG_XTENSA_CP_INITSET.  Each bit corresponds 
to one
                coprocessor with the same bit layout as for the CPENABLE 
register.
 
+config XTENSA_USE_SEPERATE_IMEM
+       bool "Use a seperate heap for internal memory"

Review comment:
       i guess the description should at least explain:
   * what's internal memory
   * what uses the memory if this config is enabled
   

##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -924,6 +955,21 @@ static void esp32_spi_dma_exchange(FAR struct 
esp32_spi_priv_s *priv,
       tp += n;
       rp += n;
     }
+
+#ifdef CONFIG_XTENSA_USE_SEPERATE_IMEM
+  if (esp32_ptr_extram(rxbuffer))
+    {
+      memcpy(rxbuffer, rp, bytes);
+      up_imm_free(rp);

Review comment:
       tp and rp are modified above. (tp += n)
   i guess you should free the original pointer.
   

##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -924,6 +955,21 @@ static void esp32_spi_dma_exchange(FAR struct 
esp32_spi_priv_s *priv,
       tp += n;
       rp += n;
     }
+
+#ifdef CONFIG_XTENSA_USE_SEPERATE_IMEM
+  if (esp32_ptr_extram(rxbuffer))
+    {
+      memcpy(rxbuffer, rp, bytes);
+      up_imm_free(rp);
+    }
+#endif
+
+#ifdef CONFIG_XTENSA_USE_SEPERATE_IMEM
+  if (esp32_ptr_extram(txbuffer))
+    {
+      up_imm_free(tp);

Review comment:
       ditto

##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -861,11 +866,37 @@ static void esp32_spi_dma_exchange(FAR struct 
esp32_spi_priv_s *priv,
                                    uint32_t nwords)
 {
   uint32_t bytes = nwords * (priv->nbits / 8);
-  uint8_t *tp = (uint8_t *)txbuffer;
-  uint8_t *rp = (uint8_t *)rxbuffer;
+  uint8_t *tp;
+  uint8_t *rp;
   uint32_t n;
   uint32_t regval;
 
+  /* If the buffer comes from PSRAM, allocate a new one from DRAM */
+
+#ifdef CONFIG_XTENSA_USE_SEPERATE_IMEM
+  if (esp32_ptr_extram(txbuffer))
+    {
+      tp = up_imm_malloc(bytes);
+      DEBUGASSERT(tp != NULL);

Review comment:
       don't you need memcpy here?
   (i don't understand what this function is doing. just a wild guess.)




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to