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 2022/08/04 16:21:46 UTC
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6786: IMX.RT EDMA Support
pkarashchenko commented on code in PR #6786:
URL: https://github.com/apache/incubator-nuttx/pull/6786#discussion_r937998801
##########
arch/arm/src/imxrt/imxrt_edma.c:
##########
@@ -86,22 +90,16 @@
*/
#ifdef CONFIG_ARMV7M_DCACHE
-/* Align to the cache line size which we assume is >= 8 */
-
-# define EDMA_ALIGN ARMV7M_DCACHE_LINESIZE
-# define EDMA_ALIGN_MASK (EDMA_ALIGN-1)
-# define EDMA_ALIGN_UP(n) (((n) + EDMA_ALIGN_MASK) & ~EDMA_ALIGN_MASK)
-
-#else
-/* Special alignment is not required in this case,
- * but we will align to 8-bytes
- */
-
-# define EDMA_ALIGN 8
-# define EDMA_ALIGN_MASK 7
-# define EDMA_ALIGN_UP(n) (((n) + 7) & ~7)
+# if EDMA_ALIGN != ARMV7M_DCACHE_LINESIZE
+# warning EDMA_ALIGN != ARMV7M_DCACHE_LINESIZE
+# endif
+# undef EDMA_ALIGN
+# define EDMA_ALIGN ARMV7M_DCACHE_LINESIZE
#endif
+#define EDMA_ALIGN_MASK (EDMA_ALIGN-1)
Review Comment:
```suggestion
#define EDMA_ALIGN_MASK (EDMA_ALIGN - 1)
```
##########
arch/arm/src/imxrt/imxrt_edma.c:
##########
@@ -1018,19 +1003,23 @@ void imxrt_dmach_free(DMACH_HANDLE handle)
*
****************************************************************************/
-int imxrt_dmach_xfrsetup(DMACH_HANDLE *handle,
+int imxrt_dmach_xfrsetup(DMACH_HANDLE handle,
const struct imxrt_edma_xfrconfig_s *config)
{
struct imxrt_dmach_s *dmach = (struct imxrt_dmach_s *)handle;
#if CONFIG_IMXRT_EDMA_NTCD > 0
struct imxrt_edmatcd_s *tcd;
struct imxrt_edmatcd_s *prev;
+ uint16_t mask = config->flags & EDMA_CONFIG_INTMAJOR ? 0 :
+ EDMA_TCD_CSR_INTMAJOR;
#endif
uintptr_t regaddr;
uint16_t regval16;
DEBUGASSERT(dmach != NULL);
- dmainfo("dmach%u: %p config: %p\n", dmach, config);
+ dmainfo("dmach%u: %p config: %p\n", dmach->chan, dmach, config);
+
+ dmach->flags = config->flags;
Review Comment:
```suggestion
dmach->flags = config->flags;
```
##########
arch/arm/src/imxrt/imxrt_edma.c:
##########
@@ -404,13 +398,16 @@ static inline void imxrt_tcd_configure(struct imxrt_edmatcd_s *tcd,
tcd->attr = EDMA_TCD_ATTR_SSIZE(config->ssize) | /* Transfer Attributes */
EDMA_TCD_ATTR_DSIZE(config->dsize);
tcd->nbytes = config->nbytes;
- tcd->slast = tcd->slast;
+ tcd->slast = config->flags & EDMA_CONFIG_LOOPSRC ? -config->iter : 0;
Review Comment:
```suggestion
tcd->slast = config->flags & EDMA_CONFIG_LOOPSRC ? -config->iter : 0;
```
##########
arch/arm/src/imxrt/imxrt_edma.c:
##########
@@ -613,8 +605,18 @@ static void imxrt_dmach_interrupt(struct imxrt_dmach_s *dmach)
/* Terminate the transfer when it is done. */
- imxrt_dmaterminate(dmach, result);
+ if ((dmach->flags & EDMA_CONFIG_LOOP_MASK) == 0)
+ {
+ imxrt_dmaterminate(dmach, result);
+ }
+ else if (dmach->callback != NULL)
+ {
+ dmach->callback((DMACH_HANDLE)dmach, dmach->arg,
+ true, result);
+ }
Review Comment:
```suggestion
{
dmach->callback((DMACH_HANDLE)dmach, dmach->arg,
true, result);
}
```
##########
arch/arm/src/imxrt/imxrt_edma.c:
##########
@@ -1018,19 +1003,23 @@ void imxrt_dmach_free(DMACH_HANDLE handle)
*
****************************************************************************/
-int imxrt_dmach_xfrsetup(DMACH_HANDLE *handle,
+int imxrt_dmach_xfrsetup(DMACH_HANDLE handle,
const struct imxrt_edma_xfrconfig_s *config)
{
struct imxrt_dmach_s *dmach = (struct imxrt_dmach_s *)handle;
#if CONFIG_IMXRT_EDMA_NTCD > 0
struct imxrt_edmatcd_s *tcd;
struct imxrt_edmatcd_s *prev;
+ uint16_t mask = config->flags & EDMA_CONFIG_INTMAJOR ? 0 :
+ EDMA_TCD_CSR_INTMAJOR;
Review Comment:
```suggestion
uint16_t mask = config->flags & EDMA_CONFIG_INTMAJOR ? 0 :
EDMA_TCD_CSR_INTMAJOR;
```
##########
arch/arm/src/imxrt/imxrt_edma.c:
##########
@@ -86,22 +90,16 @@
*/
#ifdef CONFIG_ARMV7M_DCACHE
-/* Align to the cache line size which we assume is >= 8 */
-
-# define EDMA_ALIGN ARMV7M_DCACHE_LINESIZE
-# define EDMA_ALIGN_MASK (EDMA_ALIGN-1)
-# define EDMA_ALIGN_UP(n) (((n) + EDMA_ALIGN_MASK) & ~EDMA_ALIGN_MASK)
-
-#else
-/* Special alignment is not required in this case,
- * but we will align to 8-bytes
- */
-
-# define EDMA_ALIGN 8
-# define EDMA_ALIGN_MASK 7
-# define EDMA_ALIGN_UP(n) (((n) + 7) & ~7)
+# if EDMA_ALIGN != ARMV7M_DCACHE_LINESIZE
+# warning EDMA_ALIGN != ARMV7M_DCACHE_LINESIZE
+# endif
+# undef EDMA_ALIGN
+# define EDMA_ALIGN ARMV7M_DCACHE_LINESIZE
#endif
Review Comment:
Why not just
```
#ifdef CONFIG_ARMV7M_DCACHE
# define EDMA_ALIGN ARMV7M_DCACHE_LINESIZE
#else
# define EDMA_ALIGN 32
#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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org