ti driver EMACMSP432E4.c EMACMSP432E4_handleRx() understanding

user4261913 said:

The checksum error part of the code is thus never reached. I am seeing it correct?

Yes, this is true. By default, the h/w drops any RX packet that has a back checksum (CS) and such a packet will never reach the driver.

There is an existing Jira to add an IOCTL to more easily toggle this feature:

    TIDRIVERS-3832 MSP433E4 driver should support HW checksum error drop IOCTL

The fix for this should be out in the next release. If you don't want to wait for that, you could try the following.

You need to ensure that the DT bit of the EMACDMAOPMODE register is set. You should be able to do this with code similar to the following:

            uint32_t dmaMode;

            /* Get current value of the DMA operation mode register */
            dmaMode = HWREG(EMAC0_BASE + EMAC_O_DMAOPMODE);

            /*
             * Disable h/w drops of frames with bad checksums
             * Note the logic here is tricky (seems backwards ...)
             * If the DT bit is currently 0 (i.e. h/w drops are
             * -enabled-), then OR-ing will change the bit to 1 (h/w drops
             * are -disabled-). If h/w drops are already disabled (bit is
             * currently 1), then OR-ing here will have no effect (it will
             * remain disabled).
             */
            dmaMode |= EMAC_DMAOPMODE_DT;

            /* Write the updated value to the DMA operation mode register */
            HWREG(EMAC0_BASE + EMAC_O_DMAOPMODE) = dmaMode;

There is an existing Jira enhancement request to make this functionality available as an IOCTL.

    TIDRIVERS-3832 MSP432E4 driver should support HW checksum error drop IOCTL

user4261913 said:

at line 409, for unvalid frames, pDescList->ulRead is incremented

EMACMSP432E4_private.rxDropped++;                 EMACMSP432E4_primeRx(hPkt,                         &(pDescList->pDescriptors[pDescList->ulRead])); ->             pDescList->ulRead++;                 break;             }

whitout taking care to wrap.

At line 456, for valid frames, pDescList->ulRead is incremented as follow:

/* Move on to the next descriptor in the chain, taking care to wrap. */   pDescList->ulRead++;   if (pDescList->ulRead == pDescList->ulNumDescs) {       pDescList->ulRead = 0;   }

Can someone explain why?

This is a known issue, seen when receiving a bad frame (including those w/ a bad CS) and the fix should be out in the next release. It's tracked by the following Jira:

    TIDRIVERS-4164 EMAC DMA descriptor index overflows causing out of bounds access

If you don't want to wait for that, you could try the following to fix this:

     uint32_t         ui32Mode;
     uint32_t         ui32FrameSz;
     uint32_t         ui32CtrlStatus;
+    bool             noErrors = true;
 
     /* Get a pointer to the receive descriptor list. */
     pDescList = EMACMSP432E4_private.pRxDescList;
@@ -352,7 +353,7 @@ static void EMACMSP432E4_handleRx()
             (pDescList->ulRead - 1) : (pDescList->ulNumDescs - 1);
 
     /* Step through the descriptors that are marked for CPU attention. */
-    while (pDescList->ulRead != ulDescEnd) {
+    while ((pDescList->ulRead != ulDescEnd) && noErrors) {
         descCount++;
 
         /* Does the current descriptor have a buffer attached to it? */
@@ -404,8 +405,7 @@ static void EMACMSP432E4_handleRx()
                 EMACMSP432E4_private.rxDropped++;
                 EMACMSP432E4_primeRx(hPkt,
                         &(pDescList->pDescriptors[pDescList->ulRead]));
-                pDescList->ulRead++;
-                break;
+                noErrors = false;
             }
             else {
                 /* Allocate a new buffer for this descriptor */
-- 

user4261913 said:

my question is why we set two times the linkUp variable during the ISR. First at the beginnig of the ISR (one of the bugfixes) and then in prvProcessPhyInterrupt (because  a bug was never called prvProcessPhyInterrupt).

setting linkup in prvProcessPhyInterrupt alone  would not be a better solution? 

I agree this does seem redundant. I've filed the following to track this:

TIDRIVERS-4702 - MSP432E4 EMAC driver: investigate of 2 calls to signalLinkChange are necessary

Steve