Skip to content

SAME70 driver xTXDescriptorSemaphore issue #1033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 25, 2023
Merged

SAME70 driver xTXDescriptorSemaphore issue #1033

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
285fbda
Fixes and issue with the SAME70 port where and error in gmac_dev_writ…
Oct 13, 2023
a8d2063
fix formatting
tony-josi-aws Oct 18, 2023
cb12473
Allows release of the network buffer if gmac_dev_write() fails and th…
Oct 18, 2023
7f0cad9
Fix typo
ActoryOu Oct 19, 2023
59b5482
Merge branch 'main' into SAM_Drv_TxSem_PR
ActoryOu Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions source/portable/NetworkInterface/DriverSAM/NetworkInterface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,12 +651,22 @@ static BaseType_t prvSAM_NetworkInterfaceOutput( NetworkInterface_t * pxInterfac
if( ulResult != GMAC_OK )
{
TX_STAT_INCREMENT( tx_write_fail );

/* On a successful write to GMAC, the ownership of the TX descriptor will eventually get returned back to the network
* driver and prvEMACHandlerTask will give back the xTXDescriptorSemaphore counting semaphore. In this case however,
* writing to the GMAC failed, so there will be no EMAC_IF_TX_EVENT sent to prvEMACHandlerTask and the counting
* semaphore will not be given back. Give it back now. */
xSemaphoreGive( xTXDescriptorSemaphore );
}

#if ( ipconfigZERO_COPY_TX_DRIVER != 0 )
Copy link
Contributor

@htibosch htibosch Oct 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, one remark: when sending was not possible, the ownership of the packet has not been transferred to the interface, hence the packet must be released "manually":

 ulResult = gmac_dev_write( &gs_gmac_dev, ( void * ) pxDescriptor->pucEthernetBuffer, ulTransmitSize );
 
 if( ulResult != GMAC_OK )
 {
     TX_STAT_INCREMENT( tx_write_fail );
 
     xSemaphoreGive( xTXDescriptorSemaphore );
 }
 
 #if ( ipconfigZERO_COPY_TX_DRIVER != 0 )
 {
-    /* Confirm that the pxDescriptor may be kept by the driver. */
+    if( ulResult == GMAC_OK )
+    {
+        /* The message was send in a zero-copy way.
+         * It will be released after a succeful transmission. 
+         */
         bReleaseAfterSend = pdFALSE;
+    }
 }
 #endif /* ipconfigZERO_COPY_TX_DRIVER */

Copy link
Contributor Author

@evpopov evpopov Oct 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see.... On an error, The network buffer was never handed over to hardware so we allow for it to be released.
Thanks, I'll add that.

{
/* Confirm that the pxDescriptor may be kept by the driver. */
bReleaseAfterSend = pdFALSE;
if( ulResult == GMAC_OK )
{
/* The message was send in a zero-copy way.
* It will be released after a successful transmission. */
bReleaseAfterSend = pdFALSE;
}
}
#endif /* ipconfigZERO_COPY_TX_DRIVER */
/* Not interested in a call-back after TX. */
Expand Down