stm32h7xx_hal_driver
stm32h7xx_hal_driver copied to clipboard
STMicroelectronics
ETH Rx Tailpointer calculation wrong
Caution The Issues are strictly limited for the reporting of problem encountered with the software provided in this project. For any other problem related to the STM32 product, the performance, the hardware characteristics and boards, the tools the environment in general, please post a topic in the ST Community/STM32 MCUs forum
Describe the set-up Custom Board with STM32H743XIHX
Describe the bug In v1.11.3 in the function ETH_UpdateDescriptor the calculation of the tailidx/DMACRDTPR is wrong. Becaus of this the HAL_ETH_ErrorCallback is reached in my application
The tail pointer should point to the last built/updated descriptor.
v1.11.0 points to NULL -- completly wrong v1.11.3 tried to fix it but points to 2 after the last updated descriptor
Hello @Florian-Fischer-InMach,
To assist you more effectively with the DMA pointer issue you're experiencing, could you please provide us with the following configuration details of your custom board?
- The exact configuration of the MPU settings for the DMA descriptors.
- Any custom settings or modifications you have made to the DMA controller.
With Regards,
Hello I use this code to configure the MPU. I use some linker script magic to place dma stuff at begin of each ram section(d1/d2/d3/external) and extended to statisfy the power of 2 requierment of mpu region size The DMA descriptors end up in uncachable dma region at begin of d2 ram.
What do you mean by custom setting to DMA controller? For ETH stuff i use the HAL functions. No custom settings.
For adc, spi and uart DMA1 is used
/**
* Set the whole memory to cacheable and set overlay region for dma to uncacheable
*/
static void set_region ( const uint8_t region_number, const uint32_t base_adress, const uint32_t memeory_size, const uint32_t dma_size )
{
MPU_Region_InitTypeDef MPU_InitStruct = {0};
/*
* Initializes Ram as Write-back, write and read allocate -> tex 1 + c + b + no s
*/
MPU_InitStruct.Enable = MPU_REGION_ENABLE;
MPU_InitStruct.Number = region_number;
MPU_InitStruct.BaseAddress = base_adress;
MPU_InitStruct.Size = calc_mpu_region_size ( memeory_size );
MPU_InitStruct.SubRegionDisable = 0x0;
MPU_InitStruct.TypeExtField = MPU_TEX_LEVEL1;
MPU_InitStruct.AccessPermission = MPU_REGION_FULL_ACCESS;
MPU_InitStruct.DisableExec = MPU_INSTRUCTION_ACCESS_DISABLE;
MPU_InitStruct.IsShareable = MPU_ACCESS_NOT_SHAREABLE;
MPU_InitStruct.IsCacheable = MPU_ACCESS_CACHEABLE;
MPU_InitStruct.IsBufferable = MPU_ACCESS_BUFFERABLE;
HAL_MPU_ConfigRegion(&MPU_InitStruct);
`
/*
* Initializes overlay of dma part as shareable and not cachable -> tex 1 + no c + no b + s
*/
volatile uint32_t volatile_dma_size = dma_size;//workaround compiler bug. It assumes a adress is never 0 and optimises the if away
if ( volatile_dma_size > 0 )
{
MPU_InitStruct.Number = region_number + 1;
MPU_InitStruct.BaseAddress = base_adress;
MPU_InitStruct.Size = calc_mpu_region_size ( dma_size );
MPU_InitStruct.IsShareable = MPU_ACCESS_SHAREABLE;
MPU_InitStruct.IsCacheable = MPU_ACCESS_NOT_CACHEABLE;
MPU_InitStruct.IsBufferable = MPU_ACCESS_NOT_BUFFERABLE;
HAL_MPU_ConfigRegion(&MPU_InitStruct);
}
}
My solution to fix the tail pointer: replace
tailidx = (descidx + 1U) % ETH_RX_DESC_CNT;
with
tailidx = (ETH_RX_DESC_CNT + descidx - 1U) % ETH_RX_DESC_CNT;
Indeed, that fixes it for me too. Can you explain the reasoning behind adding ETH_RX_DESC_CNT the sum to be modulod? It should be the mathematically the same without, no? I think I understand the - 1... you want the match to happen, and the index was incremented before this, so you're just "undoing" that increment so it matches to the actual last descriptor to be processed. Is that correct?
For some reason i assumed modulo does not work for -1 in a unsigned variable. Do not remember if proofed this assumption.
Yes the idea is to get the idx of the last touched descriptor
Did the proof now. Simplified to uint8_t -1 results 255 in the unsigned variable.
Assume amount of descriptors to be 7. 255 % 7 = 3 Expected is 6 --> addition is needed.
Yes I also just did that. Good thinking, you are right that it's needed.
Hello @Florian-Fischer-InMach and @chris1seto,
During initialization in the ETH_DMARxDescListInit() function, the tail pointer is set to the address of the last descriptor, which is at index 3 when ETH_RX_DESC_CNT=4. In the ETH_UpdateDescriptor() function, the tail pointer is updated to point to the next descriptor (descidx + 1). It starts at 1, since the initial descriptor is at index 0, and then increments by 1 until it reaches the last descriptor at index 3.
Could you please explain why you want to change the pointer so that it points to the previous descriptor instead of the next descriptor?
With Regards,
Please think about what the idea of interrupts and dma is. Then think about what your code is doing.
If you want to shourcut the thinking read the answer i wrote also in my online support ticket #00208305
To avoid packet loss you always want maximum amount of descriptors assigned to the dma. If i understand this corret ETH_UpdateDescriptor is intended to doing exactly that.
Description of what happens during first call of ETH_UpdateDescriptor. descidx is initialised with heth->RxDescList.RxBuildDescIdx because of first call this is 0.
in the while loop all descriptors are assigned to the dma and descidx is increased
After the loop descidx got incremented ETH_RX_DESC_CNT times in the first call. Due to wrap around this results in 0 Then you set the tail pointer to descidx + 1 Thus the dma is allowed to use 1 descriptor but all descriptors are ready to be used by dma
Thank you for your contribution and for bringing this to our attention. You are right, this issue will be fixed soon.
ST Internal Reference: 187268
With Regards,
The stm32h5xx_hal_driver has the same issue with tailpointer (tested on STM32H563VI).
Please include the fix in the next release as well. Current (v1.3.0) implementation:
https://github.com/STMicroelectronics/stm32h5xx_hal_driver/blob/0de1dbf8ad03e6e466ed55d1c174656561e3240d/Src/stm32h5xx_hal_eth.c#L1212
