Ethernet tail pointer setup is incorrect, and tail pointer management does not work with timestamping enabled

[chris1seto]

Describe the set-up Any Stm32H7

Describe the bug The RX tail pointer is set in both ETH_UpdateDescriptor() (called on init by HAL_ETH_Start_IT()) and ETH_DMARxDescListInit() (called by HAL_ETH_Init()).

The updating of the tail pointer in ETH_UpdateDescriptor() is a runtime update, and the tail pointer will be set to the last descriptor processed by the app. When the descriptors are initially built (from the call in HAL_ETH_Start_IT()), the tail pointer will be set to 1. This is not the correct value for it to be set to on init, and this would mean that only 1 descriptor would be available. ST was clearly trying to init this tail pointer to the correct value in ETH_DMARxDescListInit(), but this function call happens *before the call to build the descriptors via ETH_UpdateDescriptor() from the call to HAL_ETH_Start_IT() so the tail pointer is effectively initialized incorrectly.

IF timestamping is enabled, at least 2 descriptors will be needed by the very first packet which arrives. This will cause an RBU.

How To Reproduce

1. Indicate the global behavior of your application project RBU because only one descriptor is available.

2. The modules that you suspect to be the cause of the problem (Driver, BSP, MW ...) Eth

3. The use case that generates the problem Any use where more than one single descriptor is needed in the first packet

4. How we can reproduce the problem Look at the code, confirm that the tail pointer is being initialized to the incorrect value during init building after it was supposed to have been set to the right value.

[Florian-Fischer-InMach]

Seems i am not the only one stumbling over the tailpointer https://github.com/STMicroelectronics/stm32h7xx_hal_driver/issues/61

[ASEHSTM]

Hello @chris1seto,

Thank you for your detailed report. I would like to clarify that the ETH_DMARxDescListInit() function first initializes the RX descriptors and sets the RX tail pointer (DMACRXDTPR) to the address of the last descriptor. ETH_UpdateDescriptor(), on the other hand, updates the RX tail pointer based on the index of the descriptor being processed.

If I understand correctly, you want us to change the calculation of the tail pointer so that it points to the last built/updated descriptor, rather than the next descriptor. Is this correct?

With Regards,

[chris1seto]

@ASEHSTM I'd like for ST for do the analysis and refer to their internal design information regarding the Ethernet MAC IP block to fix the ethernet driver so it works correctly.

What I posted here is, what I believe to be, an obvious implementation error (ie, that the tail pointer on init, points to the very first descriptor, not the last descriptor).

I believe this should mean that if the first packet to arrive uses up more than one descriptor or if the application doesn't process the descriptor before any more than one descriptor is used, then there will be an RBU thrown. I don't know for sure if I am correct in that description, but I do know there is some kind of issue in the tailpointer code.

I'm at the point where I don't really entirely trust the reference manual section on the ethernet mac. It lacks a lot of plain verbose description of how the tail pointer/dma/timestamping works. I am asking ST to step in and implement the correct fix based on their internal knowledge of how the tail pointer works.

Perhaps you can start by answering a straightforward question:

1. IF there are 10 (0 through 9) descriptors in the ring, at init, which descriptor should the tailpointer point to? If the answer isn't 1, then this code is wrong.
2. During runtime, after processing n descriptors (lets say, starting at and including descriptor 0), where should the tail pointer point?

[ASEHSTM]

ST Internal Reference: 187268

[ASEHSTM]

Duplicate of #61

[chris1seto]

This issue is not entirely a duplicate of the issue which was closed. Please read the full text of my description. The tail pointer is not set correctly as the driver is initialized.

@ASEHSTM

[ASEHSTM]

Hello @chris1seto,

During initialization, the tail pointer must point to the last available descriptor. For example, in the case of four descriptors, it should point to the descriptor at index 3. This is necessary because the DMACRDTPR indicates to the DMA the address of the last receive descriptor. We have verified that the initialization is correct and that the tail pointer indeed points to the last available descriptor. We have modified the calculation of the tail pointer so that it points to the previous descriptor, which ensures that it points to the last available descriptor and allows the DMA to use all available descriptors.

With Regards,

[chris1seto]

@ASEHSTM You mentioned that you verified that the initialization is correct. Did you verify that the initialization of the tail pointer is correct after HAL_ETH_Start_IT() is called when the driver is started? The tail pointer is hard coded to start as the descriptor count - 1 when HAL_ETH_Init is called which is correct, but then HAL_ETH_Start_IT() will modify it as it calls ETH_DMARxDescListInit to rebuild the full descriptor list. After HAL_ETH_Start_IT is called, is the tail pointer value still correctly pointing to the last index - 1?

Thanks,