[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #459: stm32h7: support SDRAM via FMC peripherial

2020-03-25 Thread GitBox
davids5 commented on a change in pull request #459: stm32h7: support SDRAM via 
FMC peripherial
URL: https://github.com/apache/incubator-nuttx/pull/459#discussion_r397833141
 
 

 ##
 File path: arch/arm/src/stm32h7/stm32_allocateheap.c
 ##
 @@ -60,33 +60,57 @@
 #include "hardware/stm32_memorymap.h"
 #include "stm32_mpuinit.h"
 #include "stm32_dtcm.h"
+#include "stm32_fmc.h"
 
 /
  * Pre-processor Definitions
  /
 
-/* Internal SRAM is available in all members of the STM32 family. The
- * following definitions must be provided to specify the size and
- * location of internal(system) SRAM:
+/* At startup the kernel will invoke up_addregion() so that platform code
+ * may register available memories for use as part of system heap.
+ * The global configuration option CONFIG_MM_REGIONS defines the maximal
+ * number of non-contiguous memory ranges that may be registered with the
+ * system heap. You must make sure it is large enough to hold all memory
+ * regions you intend to use.
  *
- * In addition to internal SRAM, external RAM may also be available through
- * the FMC.  In order to use FMC RAM, the following additional things need
- * to be present in the NuttX configuration file:
+ * The following memory types can be used for heap on STM32H7 platform:
  *
- * CONFIG_STM32H7_FMC=y : Enables the FMC
- * CONFIG_STM32H7_FMC_S[D]RAM=y : SRAM and/or SDRAM is available via the FMC.
- *Either of these autoselects
- *CONFIG_ARCH_HAVE_HEAP2 which is what we
- *are interested in here.
- * CONFIG_HEAP2_BASE: The base address of the external RAM in
- *the FMC address space
- * CONFIG_HEAP2_SIZE: The size of the external RAM in the FMC
- *address space
- * CONFIG_MM_REGIONS: Must be set to a large enough value to
- *include the FMC external RAM (as determined
- *by the rules provided below)
+ * - AXI SRAM is a 512kb memory area. This will be automatically registered
+ *  with the system heap in up_allocate_heap, all the other memory
+ *  regions will be registered in up_addregion().
+ *  So, CONFIG_MM_REGIONS must be at least 1 to use AXI SRAM.
  *
- *  CONFIG_STM32H7_DTCMEXCLUDE  : Set to exclude the DTCM from heap
+ * - Internal SRAM is available in all members of the STM32 family.
+ *  This is always registered with system heap.
+ *  There are two contiguous regions of internal SRAM:
+ *  SRAM1+SRAM2+SRAM3 and SRAM4 at a separate address.
+ *  So, add 2 more to CONFIG_MM_REGIONS.
+ *
+ * - Tightly Coupled Memory (TCM RAM), we can use Data TCM (DTCM) for system
+ *  heap. Note that DTCM has a number of limitations, for example DMA
+ *  transfers to/from DTCM are impossible.
 
 Review comment:
   @anpaza was this copied from the another SoC? The H7 has a multiple DMA 
engines and DTCM is on the cross bar to this statment is not true.


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


With regards,
Apache Git Services


[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #459: stm32h7: support SDRAM via FMC peripherial

2020-03-25 Thread GitBox
davids5 commented on a change in pull request #459: stm32h7: support SDRAM via 
FMC peripherial
URL: https://github.com/apache/incubator-nuttx/pull/459#discussion_r397833561
 
 

 ##
 File path: arch/arm/src/stm32h7/stm32_allocateheap.c
 ##
 @@ -60,33 +60,57 @@
 #include "hardware/stm32_memorymap.h"
 #include "stm32_mpuinit.h"
 #include "stm32_dtcm.h"
+#include "stm32_fmc.h"
 
 /
  * Pre-processor Definitions
  /
 
-/* Internal SRAM is available in all members of the STM32 family. The
- * following definitions must be provided to specify the size and
- * location of internal(system) SRAM:
+/* At startup the kernel will invoke up_addregion() so that platform code
+ * may register available memories for use as part of system heap.
+ * The global configuration option CONFIG_MM_REGIONS defines the maximal
+ * number of non-contiguous memory ranges that may be registered with the
+ * system heap. You must make sure it is large enough to hold all memory
+ * regions you intend to use.
  *
- * In addition to internal SRAM, external RAM may also be available through
- * the FMC.  In order to use FMC RAM, the following additional things need
- * to be present in the NuttX configuration file:
+ * The following memory types can be used for heap on STM32H7 platform:
  *
- * CONFIG_STM32H7_FMC=y : Enables the FMC
- * CONFIG_STM32H7_FMC_S[D]RAM=y : SRAM and/or SDRAM is available via the FMC.
- *Either of these autoselects
- *CONFIG_ARCH_HAVE_HEAP2 which is what we
- *are interested in here.
- * CONFIG_HEAP2_BASE: The base address of the external RAM in
- *the FMC address space
- * CONFIG_HEAP2_SIZE: The size of the external RAM in the FMC
- *address space
- * CONFIG_MM_REGIONS: Must be set to a large enough value to
- *include the FMC external RAM (as determined
- *by the rules provided below)
+ * - AXI SRAM is a 512kb memory area. This will be automatically registered
+ *  with the system heap in up_allocate_heap, all the other memory
+ *  regions will be registered in up_addregion().
+ *  So, CONFIG_MM_REGIONS must be at least 1 to use AXI SRAM.
  *
- *  CONFIG_STM32H7_DTCMEXCLUDE  : Set to exclude the DTCM from heap
+ * - Internal SRAM is available in all members of the STM32 family.
+ *  This is always registered with system heap.
+ *  There are two contiguous regions of internal SRAM:
+ *  SRAM1+SRAM2+SRAM3 and SRAM4 at a separate address.
+ *  So, add 2 more to CONFIG_MM_REGIONS.
+ *
+ * - Tightly Coupled Memory (TCM RAM), we can use Data TCM (DTCM) for system
+ *  heap. Note that DTCM has a number of limitations, for example DMA
+ *  transfers to/from DTCM are impossible.
 
 Review comment:
   ```suggestion
*  transfers to/from DTCM are limited.
   ```


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


With regards,
Apache Git Services


[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #459: stm32h7: support SDRAM via FMC peripherial

2020-03-10 Thread GitBox
davids5 commented on a change in pull request #459: stm32h7: support SDRAM via 
FMC peripherial
URL: https://github.com/apache/incubator-nuttx/pull/459#discussion_r390278568
 
 

 ##
 File path: arch/arm/src/stm32h7/stm32_allocateheap.c
 ##
 @@ -246,6 +249,14 @@ void up_allocate_heap(FAR void **heap_start, size_t 
*heap_size)
 
   up_heap_color(*heap_start, *heap_size);
 #endif
+
+#if defined(CONFIG_DEBUG_FEATURES)
+
+  /* Display memory ranges to help debugging */
+
+  _info("%uKb of SRAM at %p\n", *heap_size / 1024, *heap_start);
 
 Review comment:
   So the `correct and best` thing to do is to look and see if the HW has an 
SDRAM in it. Then add a config and update board.h for thoses ones.  It if does 
not then there is no action needed.
   
   The issue is that, if you do not have the HW you can not verify it. But you 
are the `expert` on this at this point in time and can share that expertise in 
a board config so that when some does try to use it, they are closer to making 
it work that staring from scratch. 


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


With regards,
Apache Git Services


[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #459: stm32h7: support SDRAM via FMC peripherial

2020-03-08 Thread GitBox
davids5 commented on a change in pull request #459: stm32h7: support SDRAM via 
FMC peripherial
URL: https://github.com/apache/incubator-nuttx/pull/459#discussion_r389355817
 
 

 ##
 File path: arch/arm/src/stm32h7/stm32_allocateheap.c
 ##
 @@ -246,6 +249,14 @@ void up_allocate_heap(FAR void **heap_start, size_t 
*heap_size)
 
   up_heap_color(*heap_start, *heap_size);
 #endif
+
+#if defined(CONFIG_DEBUG_FEATURES)
+
+  /* Display memory ranges to help debugging */
+
+  _info("%uKb of SRAM at %p\n", *heap_size / 1024, *heap_start);
 
 Review comment:
   @anpaza let me know if you are OK with this change and I will commit and 
merge 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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #459: stm32h7: support SDRAM via FMC peripherial

2020-03-06 Thread GitBox
davids5 commented on a change in pull request #459: stm32h7: support SDRAM via 
FMC peripherial
URL: https://github.com/apache/incubator-nuttx/pull/459#discussion_r389018630
 
 

 ##
 File path: arch/arm/src/stm32h7/stm32_allocateheap.c
 ##
 @@ -246,6 +249,14 @@ void up_allocate_heap(FAR void **heap_start, size_t 
*heap_size)
 
   up_heap_color(*heap_start, *heap_size);
 #endif
+
+#if defined(CONFIG_DEBUG_FEATURES)
+
+  /* Display memory ranges to help debugging */
+
+  _info("%uKb of SRAM at %p\n", *heap_size / 1024, *heap_start);
 
 Review comment:
   In practice the underscore version is not use directly. see debug.h minfo() 
would be used here.
   ```suggestion
 minfo("%uKb of SRAM at %p\n", *heap_size / 1024, *heap_start);
   ```


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


With regards,
Apache Git Services