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 17:21:17 UTC

[GitHub] [incubator-nuttx] davids5 opened a new pull request, #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

davids5 opened a new pull request, #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787

   ## Summary
   
   - Added Support for LAN8742A PHY
   - All boards ARCH_PHY_INTERRUPT is a board property
   - add IMXRT_PHY_POLLING
   - Support either ETH1 or ETH2
   - pinmux Add ENET2 and correct ALT for GPIO_ENET2_REF_CLK2
   - Support WB dcache:  ensure proper dcache operation in Writeback mode
   -  Better interrupt state Handling: Proper order of operations, atomic operations 
   
   ## Impact
   
   The driver now works in dcache WB mode  on ETH 1 or 2 (defaults to 1 to avoind breakage.
   
   ## Testing
   
   PX4 nxp_fmurt160-v2 and QGC 
   


-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#issuecomment-1206117525

   `Error: chip/imxrt_clockconfig.c:276:12: error: 'CCM_ANALOG_PLL_ENET_ENABLE' undeclared (first use in this function); did you mean 'CCM_ANALOG_PLL_USB2_ENABLE'?`


-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#issuecomment-1206503208

   CI still complains on
   ```
   Error: chip/imxrt_enet.c:120:37: error: 'GPR_GPR1_ENET1_TX_DIR_OUT' undeclared (first use in this function); did you mean 'GPR_GPR1_ENET_TX_DIR'?
   ```


-- 
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


[GitHub] [incubator-nuttx] davids5 commented on pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
davids5 commented on PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#issuecomment-1206271555

   @pkarashchenko @xiaoxiang781216 - Thank you for the review. I will incorporate all the changes and fix CI - then force push.


-- 
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


[GitHub] [nuttx] davids5 commented on pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by "davids5 (via GitHub)" <gi...@apache.org>.
davids5 commented on PR #6787:
URL: https://github.com/apache/nuttx/pull/6787#issuecomment-1425319361

   @
   
   > @davids5 Sorry for adding my comments to this old PR. In this PR, it seems that you fixed write-back cache issues in imxrt_enet.c. So I removed the write-through configuration from imxrt1060-evk:netnsh like
   > 
   > ```
   > --- a/boards/arm/imxrt/imxrt1060-evk/configs/netnsh/defconfig
   > +++ b/boards/arm/imxrt/imxrt1060-evk/configs/netnsh/defconfig
   > @@ -14,7 +14,6 @@ CONFIG_ARCH_CHIP_IMXRT=y
   >  CONFIG_ARCH_CHIP_MIMXRT1062DVL6A=y
   >  CONFIG_ARCH_STACKDUMP=y
   >  CONFIG_ARMV7M_DCACHE=y
   > -CONFIG_ARMV7M_DCACHE_WRITETHROUGH=y
   >  CONFIG_ARMV7M_ICACHE=y
   >  CONFIG_ARMV7M_USEBASEPRI=y
   >  CONFIG_BOARD_LOOPSPERMSEC=104926
   > ```
   > 
   > And tried to see how the networking with imxrt1060-evk is stable. However, it stopped after several minutes. Do you think d-cache write-back mode works with imxrt1060-evk?
   
   Yes. I have not tested on current master but it was working here https://github.com/PX4/PX4-Autopilot/tree/pr-fmu1062/boards/nxp/fmurt1062-v2 with NuttX 10.3+


-- 
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


[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#discussion_r938958338


##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -125,6 +125,12 @@
 #  error Write back D-Cache not yet supported
 #endif
 
+/* TX poll delay = 1 seconds. CLK_TCK is the number of clock ticks per
+ * second.
+ */
+
+#define IMXRT_WDDELAY     (1 * CLK_TCK)

Review Comment:
   Ahh Thanks It was the merge from the cherry-pick



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#discussion_r938868074


##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -125,6 +125,12 @@
 #  error Write back D-Cache not yet supported
 #endif
 
+/* TX poll delay = 1 seconds. CLK_TCK is the number of clock ticks per
+ * second.
+ */
+
+#define IMXRT_WDDELAY     (1 * CLK_TCK)

Review Comment:
   you add in patch:
   https://github.com/apache/incubator-nuttx/pull/6787/commits/d56cbc7d296b7f535c563e6cc67644f8b29bda09#diff-17de76e9fae5c5fa9177a5c7c101870dab6729a68d2cd9650491b91926d5fa61R132
   and remove here:
   https://github.com/apache/incubator-nuttx/pull/6787/commits/b4dddc245d87a657bcd95afbac68a0c53ede1215#diff-17de76e9fae5c5fa9177a5c7c101870dab6729a68d2cd9650491b91926d5fa61L132



-- 
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


[GitHub] [incubator-nuttx] davids5 commented on pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
davids5 commented on PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#issuecomment-1206506468

   > CI still complains on
   > 
   > ```
   > Error: chip/imxrt_enet.c:120:37: error: 'GPR_GPR1_ENET1_TX_DIR_OUT' undeclared (first use in this function); did you mean 'GPR_GPR1_ENET_TX_DIR'?
   > ```
   Yes - I am tracking it down and have had 3 levels of interrupt.....reti now
   


-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787


-- 
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


[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#discussion_r938751569


##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -125,6 +125,12 @@
 #  error Write back D-Cache not yet supported
 #endif
 
+/* TX poll delay = 1 seconds. CLK_TCK is the number of clock ticks per
+ * second.
+ */
+
+#define IMXRT_WDDELAY     (1 * CLK_TCK)

Review Comment:
   This is Odd. Do you see this is the file?



-- 
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


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#discussion_r938489101


##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -269,34 +314,48 @@ struct imxrt_driver_s
   struct net_driver_s dev;     /* Interface understood by the network */
 };
 
+/* This union type forces the allocated size of TX&RX descriptors to be
+ * padded to a exact multiple of the Cortex-M7 D-Cache line size.
+ */
+
+union enet_desc_u
+{
+  uint8_t             pad[DESC_PADSIZE];
+  struct enet_desc_s  desc;
+};
+
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
 static struct imxrt_driver_s g_enet[CONFIG_IMXRT_ENET_NETHIFS];
 
-/* The DMA descriptors.  A unaligned uint8_t is used to allocate the
- * memory; 16 is added to assure that we can meet the descriptor alignment
- * requirements.
- */
+/* The DMA descriptors */
 
-static uint8_t g_desc_pool[NENET_NBUFFERS * sizeof(struct enet_desc_s)]
-               aligned_data(ENET_ALIGN);
+static union enet_desc_u g_desc_pool[NENET_NBUFFERS]
+                                     aligned_data(ENET_ALIGN);
 
-/* The DMA buffers.  Again, A unaligned uint8_t is used to allocate the
- * memory; 16 is added to assure that we can meet the descriptor alignment
- * requirements.
- */
+/* The DMA buffers */
 
-static uint8_t g_buffer_pool[NENET_NBUFFERS * IMXRT_BUF_SIZE]
-               aligned_data(ENET_ALIGN);
+static uint8_t g_buffer_pool[NENET_NBUFFERS][ALIGNED_BUFSIZE]
+                             aligned_data(ENET_ALIGN);
 
 /****************************************************************************
  * Private Function Prototypes
  ****************************************************************************/
 
 /* Utility functions */
 
+static inline uint32_t imxrt_enet_getreg32(FAR struct imxrt_driver_s *priv,
+                                           uint32_t offset);
+static inline void imxrt_enet_putreg32(FAR struct imxrt_driver_s *priv,

Review Comment:
   ```suggestion
   static inline void imxrt_enet_putreg32(struct imxrt_driver_s *priv,
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -858,6 +989,9 @@ static void imxrt_receive(struct imxrt_driver_s *priv)
             imxrt_swap32((uint32_t)priv->txdesc[priv->txhead].data);
           rxdesc->status1 |= RXDESC_E;
 
+          up_clean_dcache((uintptr_t) rxdesc,
+                          (uintptr_t) rxdesc + sizeof(struct enet_desc_s));

Review Comment:
   ```suggestion
             up_clean_dcache((uintptr_t)rxdesc,
                             (uintptr_t)rxdesc + sizeof(struct enet_desc_s));
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -269,34 +314,48 @@ struct imxrt_driver_s
   struct net_driver_s dev;     /* Interface understood by the network */
 };
 
+/* This union type forces the allocated size of TX&RX descriptors to be
+ * padded to a exact multiple of the Cortex-M7 D-Cache line size.
+ */
+
+union enet_desc_u
+{
+  uint8_t             pad[DESC_PADSIZE];
+  struct enet_desc_s  desc;
+};
+
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
 static struct imxrt_driver_s g_enet[CONFIG_IMXRT_ENET_NETHIFS];
 
-/* The DMA descriptors.  A unaligned uint8_t is used to allocate the
- * memory; 16 is added to assure that we can meet the descriptor alignment
- * requirements.
- */
+/* The DMA descriptors */
 
-static uint8_t g_desc_pool[NENET_NBUFFERS * sizeof(struct enet_desc_s)]
-               aligned_data(ENET_ALIGN);
+static union enet_desc_u g_desc_pool[NENET_NBUFFERS]
+                                     aligned_data(ENET_ALIGN);
 
-/* The DMA buffers.  Again, A unaligned uint8_t is used to allocate the
- * memory; 16 is added to assure that we can meet the descriptor alignment
- * requirements.
- */
+/* The DMA buffers */
 
-static uint8_t g_buffer_pool[NENET_NBUFFERS * IMXRT_BUF_SIZE]
-               aligned_data(ENET_ALIGN);
+static uint8_t g_buffer_pool[NENET_NBUFFERS][ALIGNED_BUFSIZE]
+                             aligned_data(ENET_ALIGN);
 
 /****************************************************************************
  * Private Function Prototypes
  ****************************************************************************/
 
 /* Utility functions */
 
+static inline uint32_t imxrt_enet_getreg32(FAR struct imxrt_driver_s *priv,

Review Comment:
   ```suggestion
   static inline uint32_t imxrt_enet_getreg32(struct imxrt_driver_s *priv,
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -374,6 +433,74 @@ static void imxrt_reset(struct imxrt_driver_s *priv);
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: imxrt_enet_getreg32
+ *
+ * Description:
+ *   Get the contents of the ENET register at offset
+ *
+ * Input Parameters:
+ *   priv   - private ENET device structure
+ *   offset - offset to the register of interest
+ *
+ * Returned Value:
+ *   The contents of the 32-bit register
+ *
+ ****************************************************************************/
+
+static inline uint32_t
+imxrt_enet_getreg32(FAR struct imxrt_driver_s *priv, uint32_t offset)
+{
+  return getreg32(priv->base + offset);
+}
+
+/****************************************************************************
+ * Name: imxrt_enet_putreg32
+ *
+ * Description:
+ *   Atomically modify the specified bits in a memory mapped register
+ *
+ * Input Parameters:
+ *   priv      - private SPI device structure
+ *   offset    - offset to the register of interest
+ *   clearbits - the 32-bit value to be written as 0s
+ *   setbits   - the 32-bit value to be written as 1s
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+static inline void imxrt_enet_modifyreg32(FAR struct imxrt_driver_s *priv,

Review Comment:
   ```suggestion
   static inline void imxrt_enet_modifyreg32(struct imxrt_driver_s *priv,
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -527,41 +652,47 @@ static int imxrt_transmit(struct imxrt_driver_s *priv)
   txdesc->status1 |= (TXDESC_R | TXDESC_L | TXDESC_TC);
 
   buf = (uint8_t *)imxrt_swap32((uint32_t)priv->dev.d_buf);
-  if (priv->rxdesc[priv->rxtail].data == buf)
-    {
-      struct enet_desc_s *rxdesc = &priv->rxdesc[priv->rxtail];
 
+  struct enet_desc_s *rxdesc = &priv->rxdesc[priv->rxtail];
+
+  up_invalidate_dcache((uintptr_t)rxdesc,
+                       (uintptr_t)rxdesc + sizeof(struct enet_desc_s));
+
+  if (rxdesc->data == buf)
+    {
       /* Data was written into the RX buffer, so swap the TX and RX buffers */
 
       DEBUGASSERT((rxdesc->status1 & RXDESC_E) == 0);
       rxdesc->data = txdesc->data;
       txdesc->data = buf;
+      up_clean_dcache((uintptr_t) rxdesc,
+                      (uintptr_t) rxdesc + sizeof(struct enet_desc_s));
     }
   else
     {
       DEBUGASSERT(txdesc->data == buf);
     }
 
-  /* Make the following operations atomic */
+  up_clean_dcache((uintptr_t) txdesc,
+                  (uintptr_t) txdesc + sizeof(struct enet_desc_s));

Review Comment:
   ```suggestion
     up_clean_dcache((uintptr_t)txdesc,
                     (uintptr_t)txdesc + sizeof(struct enet_desc_s));
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -374,6 +433,74 @@ static void imxrt_reset(struct imxrt_driver_s *priv);
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: imxrt_enet_getreg32
+ *
+ * Description:
+ *   Get the contents of the ENET register at offset
+ *
+ * Input Parameters:
+ *   priv   - private ENET device structure
+ *   offset - offset to the register of interest
+ *
+ * Returned Value:
+ *   The contents of the 32-bit register
+ *
+ ****************************************************************************/
+
+static inline uint32_t
+imxrt_enet_getreg32(FAR struct imxrt_driver_s *priv, uint32_t offset)
+{
+  return getreg32(priv->base + offset);
+}
+
+/****************************************************************************
+ * Name: imxrt_enet_putreg32
+ *
+ * Description:
+ *   Atomically modify the specified bits in a memory mapped register
+ *
+ * Input Parameters:
+ *   priv      - private SPI device structure
+ *   offset    - offset to the register of interest
+ *   clearbits - the 32-bit value to be written as 0s
+ *   setbits   - the 32-bit value to be written as 1s
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+static inline void imxrt_enet_modifyreg32(FAR struct imxrt_driver_s *priv,
+                                          unsigned int offset,
+                                          uint32_t clearbits,
+                                          uint32_t setbits)
+{
+  modifyreg32(priv->base + offset, clearbits, setbits);
+}
+
+/****************************************************************************
+ * Name: imxrt_enet_putreg32
+ *
+ * Description:
+ *   Write a 16-bit value to the ENET register at offset
+ *
+ * Input Parameters:
+ *   priv   - private SPI device structure
+ *   value  - the 32-bit value to be written
+ *   offset - offset to the register of interest
+ *
+ * Returned Value:
+ *   The contents of the 32-bit register
+ *
+ ****************************************************************************/
+
+static inline void imxrt_enet_putreg32(FAR struct imxrt_driver_s *priv,

Review Comment:
   ```suggestion
   static inline void imxrt_enet_putreg32(struct imxrt_driver_s *priv,
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -269,34 +314,48 @@ struct imxrt_driver_s
   struct net_driver_s dev;     /* Interface understood by the network */
 };
 
+/* This union type forces the allocated size of TX&RX descriptors to be
+ * padded to a exact multiple of the Cortex-M7 D-Cache line size.
+ */
+
+union enet_desc_u
+{
+  uint8_t             pad[DESC_PADSIZE];
+  struct enet_desc_s  desc;
+};
+
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
 static struct imxrt_driver_s g_enet[CONFIG_IMXRT_ENET_NETHIFS];
 
-/* The DMA descriptors.  A unaligned uint8_t is used to allocate the
- * memory; 16 is added to assure that we can meet the descriptor alignment
- * requirements.
- */
+/* The DMA descriptors */
 
-static uint8_t g_desc_pool[NENET_NBUFFERS * sizeof(struct enet_desc_s)]
-               aligned_data(ENET_ALIGN);
+static union enet_desc_u g_desc_pool[NENET_NBUFFERS]
+                                     aligned_data(ENET_ALIGN);
 
-/* The DMA buffers.  Again, A unaligned uint8_t is used to allocate the
- * memory; 16 is added to assure that we can meet the descriptor alignment
- * requirements.
- */
+/* The DMA buffers */
 
-static uint8_t g_buffer_pool[NENET_NBUFFERS * IMXRT_BUF_SIZE]
-               aligned_data(ENET_ALIGN);
+static uint8_t g_buffer_pool[NENET_NBUFFERS][ALIGNED_BUFSIZE]
+                             aligned_data(ENET_ALIGN);
 
 /****************************************************************************
  * Private Function Prototypes
  ****************************************************************************/
 
 /* Utility functions */
 
+static inline uint32_t imxrt_enet_getreg32(FAR struct imxrt_driver_s *priv,
+                                           uint32_t offset);
+static inline void imxrt_enet_putreg32(FAR struct imxrt_driver_s *priv,
+                                       uint32_t value, uint32_t offset);
+
+static inline void imxrt_enet_modifyreg32(FAR struct imxrt_driver_s *priv,

Review Comment:
   ```suggestion
   static inline void imxrt_enet_modifyreg32(struct imxrt_driver_s *priv,
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -374,6 +433,74 @@ static void imxrt_reset(struct imxrt_driver_s *priv);
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: imxrt_enet_getreg32
+ *
+ * Description:
+ *   Get the contents of the ENET register at offset
+ *
+ * Input Parameters:
+ *   priv   - private ENET device structure
+ *   offset - offset to the register of interest
+ *
+ * Returned Value:
+ *   The contents of the 32-bit register
+ *
+ ****************************************************************************/
+
+static inline uint32_t
+imxrt_enet_getreg32(FAR struct imxrt_driver_s *priv, uint32_t offset)

Review Comment:
   ```suggestion
   static inline uint32_t imxrt_enet_getreg32(struct imxrt_driver_s *priv, 
                                              uint32_t offset)
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -1097,14 +1226,15 @@ static void imxrt_enet_interrupt_work(void *arg)
 
 static int imxrt_enet_interrupt(int irq, void *context, void *arg)
 {
-  register struct imxrt_driver_s *priv = &g_enet[0];
+  register FAR struct imxrt_driver_s *priv =
+                                          (FAR struct imxrt_driver_s *) arg;

Review Comment:
   ```suggestion
     register struct imxrt_driver_s *priv =
       (struct imxrt_driver_s *)arg;
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -2320,14 +2454,17 @@ static void imxrt_initbuffers(struct imxrt_driver_s *priv)
       priv->rxdesc[i].bdu     = 0;
       priv->rxdesc[i].status2 = RXDESC_INT;
 #endif
-      addr                   += IMXRT_BUF_SIZE;
+      addr                   += ALIGNED_BUFSIZE;
     }
 
   /* Set the wrap bit in the last descriptors to form a ring */
 
   priv->txdesc[CONFIG_IMXRT_ENET_NTXBUFFERS - 1].status1 |= TXDESC_W;
   priv->rxdesc[CONFIG_IMXRT_ENET_NRXBUFFERS - 1].status1 |= RXDESC_W;
 
+  up_clean_dcache((uintptr_t) g_desc_pool,
+                  (uintptr_t) g_desc_pool + sizeof(g_desc_pool));

Review Comment:
   ```suggestion
     up_clean_dcache((uintptr_t)g_desc_pool,
                     (uintptr_t)g_desc_pool + sizeof(g_desc_pool));
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -527,41 +652,47 @@ static int imxrt_transmit(struct imxrt_driver_s *priv)
   txdesc->status1 |= (TXDESC_R | TXDESC_L | TXDESC_TC);
 
   buf = (uint8_t *)imxrt_swap32((uint32_t)priv->dev.d_buf);
-  if (priv->rxdesc[priv->rxtail].data == buf)
-    {
-      struct enet_desc_s *rxdesc = &priv->rxdesc[priv->rxtail];
 
+  struct enet_desc_s *rxdesc = &priv->rxdesc[priv->rxtail];
+
+  up_invalidate_dcache((uintptr_t)rxdesc,
+                       (uintptr_t)rxdesc + sizeof(struct enet_desc_s));
+
+  if (rxdesc->data == buf)
+    {
       /* Data was written into the RX buffer, so swap the TX and RX buffers */
 
       DEBUGASSERT((rxdesc->status1 & RXDESC_E) == 0);
       rxdesc->data = txdesc->data;
       txdesc->data = buf;
+      up_clean_dcache((uintptr_t) rxdesc,
+                      (uintptr_t) rxdesc + sizeof(struct enet_desc_s));

Review Comment:
   ```suggestion
         up_clean_dcache((uintptr_t)rxdesc,
                         (uintptr_t)rxdesc + sizeof(struct enet_desc_s));
   ```



##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -527,41 +652,47 @@ static int imxrt_transmit(struct imxrt_driver_s *priv)
   txdesc->status1 |= (TXDESC_R | TXDESC_L | TXDESC_TC);
 
   buf = (uint8_t *)imxrt_swap32((uint32_t)priv->dev.d_buf);
-  if (priv->rxdesc[priv->rxtail].data == buf)
-    {
-      struct enet_desc_s *rxdesc = &priv->rxdesc[priv->rxtail];
 
+  struct enet_desc_s *rxdesc = &priv->rxdesc[priv->rxtail];
+
+  up_invalidate_dcache((uintptr_t)rxdesc,
+                       (uintptr_t)rxdesc + sizeof(struct enet_desc_s));
+
+  if (rxdesc->data == buf)
+    {
       /* Data was written into the RX buffer, so swap the TX and RX buffers */
 
       DEBUGASSERT((rxdesc->status1 & RXDESC_E) == 0);
       rxdesc->data = txdesc->data;
       txdesc->data = buf;
+      up_clean_dcache((uintptr_t) rxdesc,
+                      (uintptr_t) rxdesc + sizeof(struct enet_desc_s));
     }
   else
     {
       DEBUGASSERT(txdesc->data == buf);
     }
 
-  /* Make the following operations atomic */
+  up_clean_dcache((uintptr_t) txdesc,
+                  (uintptr_t) txdesc + sizeof(struct enet_desc_s));
 
-  flags = spin_lock_irqsave(NULL);
+  up_clean_dcache((uintptr_t) priv->dev.d_buf,
+                  (uintptr_t) priv->dev.d_buf + priv->dev.d_len);

Review Comment:
   ```suggestion
     up_clean_dcache((uintptr_t)priv->dev.d_buf,
                     (uintptr_t)priv->dev.d_buf + priv->dev.d_len);
   ```



-- 
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


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6787:
URL: https://github.com/apache/incubator-nuttx/pull/6787#discussion_r938398751


##########
arch/arm/src/imxrt/imxrt_enet.c:
##########
@@ -125,6 +125,12 @@
 #  error Write back D-Cache not yet supported
 #endif
 
+/* TX poll delay = 1 seconds. CLK_TCK is the number of clock ticks per
+ * second.
+ */
+
+#define IMXRT_WDDELAY     (1 * CLK_TCK)

Review Comment:
   don't need anymore



-- 
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


[GitHub] [nuttx] masayuki2009 commented on pull request #6787: imxrt: Ethernet Cleanup and extend support for ETH1 or ETH2 and LAN8742A

Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #6787:
URL: https://github.com/apache/nuttx/pull/6787#issuecomment-1425134435

   @davids5 
   Sorry for adding my comments to this old PR.
   In this PR, it seems that you fixed write-back cache issues in imxrt_enet.c. 
   So I removed the write-through configuration from imxrt1060-evk:netnsh like
   
   ```
   --- a/boards/arm/imxrt/imxrt1060-evk/configs/netnsh/defconfig
   +++ b/boards/arm/imxrt/imxrt1060-evk/configs/netnsh/defconfig
   @@ -14,7 +14,6 @@ CONFIG_ARCH_CHIP_IMXRT=y
    CONFIG_ARCH_CHIP_MIMXRT1062DVL6A=y
    CONFIG_ARCH_STACKDUMP=y
    CONFIG_ARMV7M_DCACHE=y
   -CONFIG_ARMV7M_DCACHE_WRITETHROUGH=y
    CONFIG_ARMV7M_ICACHE=y
    CONFIG_ARMV7M_USEBASEPRI=y
    CONFIG_BOARD_LOOPSPERMSEC=104926
   ```
   
   And tried to see how the networking with imxrt1060-evk is stable.
   However, it stopped after several minutes.
   Do you think d-cache write-back mode works with imxrt1060-evk?
   


-- 
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