Bug in ETH_DMARxDescListInit - reported in the forum about stm32cubeh7 HOT 18 CLOSED

Hi Pavel,

The fix of this issue will not be part of the v1.7.0 pacakge which will be published soon but part of a later one.

In the meanwhile, here is how the erroneous line will be updated:

-WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc + (((uint32_t)(ETH_RX_DESC_CNT - 1))*sizeof(ETH_DMADescTypeDef)))));
+WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc) + ((uint32_t)(ETH_RX_DESC_CNT - 1))));

We hope this helps and we thank you once more for your contribution.

With regards,

from stm32cubeh7.

Hi Pavel,

Thank you for having reported this point. It will be transmitted to our development teams. We will be back to you as soon as they provide us with a feedback. Thank you once more.

With regards,

from stm32cubeh7.

Thank you Ali. Kudos for finding this issue go to ashchepkov581 (on the forum).

from stm32cubeh7.

Hi Pavel,

Our development teams confirmed this issue. A fix will be implemented and published in the frame of a future release. Thank you again for your contribution.

With regards,

from stm32cubeh7.

ST Internal Reference: 76913

from stm32cubeh7.

Still not fixed in v. 1.6.0

from stm32cubeh7.

Hi Pavel,

This bug is intended to be fixed in the frame of package v1.7.0 release (or a later one).

With regards,

from stm32cubeh7.

Ali, could they review it again, please? Per the docum (RM0433 page 2889) ETH_DMACRXDTPR indicated location of the last RX descriptor. I understand this as it contains address of the last descriptor, that is (base addr) + sizeof (one descr) * (number of desc. -1)
But in their variant: (base addr) + (number of desc. -1).
-- pa

from stm32cubeh7.

Hi Pavel,

I had a look to the ETH_DMADescTypeDef structure type and I think I understand better your point. Would you confirm please before I loop back with our development teams? Thanks.

Below is the definition of the ETH_DMADescTypeDef structre type:

Now, sizeof(ETH_DMADescTypeDef) would return 24 (6 members x 4 bytes per member).

And here is the point:

• On the one hand, removing the sizeof(ETH_DMADescTypeDef) from the expression would not yield the correct result, as you are saying (resulting address less than the targeted one).
• On the other hand, leaving the sizeof(ETH_DMADescTypeDef) as it is would not yield the correct result neither (resulting address greater than the targeted one).

Rules of pointer arithmetic require rather the following:

• sizeof(ETH_DMADescTypeDef) / sizeof(uint32_t)

Correct implementation would then be:

-WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc + (((uint32_t)(ETH_RX_DESC_CNT - 1))*sizeof(ETH_DMADescTypeDef)))));
+WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc) + (((uint32_t)(ETH_RX_DESC_CNT - 1)) * (uint32_t)(sizeof(ETH_DMADescTypeDef) / sizeof(uint32_t)))));

With regards,

from stm32cubeh7.

Ali, sorry for delay. So now there are two "correct" variants. (I assume that red line with - is wrong and green line with + is proposed correction).

I'm confused.... where is pointer arithmetic in (uint32_t)(heth->Init.RxDesc) ? If it were (uint32_t*)(heth->Init.RxDesc) then maybe... Lets' ask help from the ethernet experts on the forum.

Regards,
Pavel

from stm32cubeh7.

I think @ALABSTM is right. The correct calculation should be (uint32_t)(heth->Init.RxDesc) + offset_bytes

I meet this issue and debug for half of day. I can't believe this bug is not fixed for this long time.

from stm32cubeh7.

Hi Pavel,

Pointer arithmetic arrives on the scene because structure member RxDesc is actually a pointer to the first DMA Rx descriptor.

To conclude, first fix suggestion is the correct one as confirmed by @kele14x.

-WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc + (((uint32_t)(ETH_RX_DESC_CNT - 1))*sizeof(ETH_DMADescTypeDef)))));
+WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc) + ((uint32_t)(ETH_RX_DESC_CNT - 1))));

The above expression (in green) will be automatically reevaluated @firstRxDesc + (ETH_RX_DESC_CNT - 1) * sizeof(ETH_DMADescTypeDef).

I hope this makes things clearer. Please do not hesitate if you still have questions about the suggested fix.

With regards,

from stm32cubeh7.

Ali, @kele14x, with all respect it seems there's some misunderstanding.
I put a little example here to explain: rextester.com/LRKA74918
As soon as you cast a pointer to a number (uint32_t)(heth->Init.RxDesc), it becomes a number and pointer stuff no longer applies to it.

However, as @alister commented on the forum, it looks like there are multiple variants of setting this register, depending on the overall design of the driver. If the person who proposed the correction will provide also a thorough fix of the ETH driver, he/she likely knows better. If such fix won't be provided, this issue alone is not important.

Regards,
Pavel A.

from stm32cubeh7.

Hi Pavel,

I confirm there is a misunderstanding and a typo (from my side) too. In the expression I reported of the suggested fix, parenthesis are not correctly placed. With such an expression, pointer arithmetic applies no more. You are absolutely right about it.

The fix would rather be:

-WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc + (((uint32_t)(ETH_RX_DESC_CNT - 1))*sizeof(ETH_DMADescTypeDef)))));
+WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc + (uint32_t)(ETH_RX_DESC_CNT - 1))));

I hope you agree on what is written here. I do apologize for the confusion caused.

With regards,

from stm32cubeh7.

Ali, no problem, thanks for checking. The bigger question now is, how about a deeper revision of the eth driver? When this could be expected?

• P.

from stm32cubeh7.

Hello Guys
I have question if we are changing the RX descriptor tail pointer address.
Do you think the TX descriptors should also be change from

/* Set Transmit Descriptor Tail pointer /
(Line 2663) WRITE_REG(heth->Instance->DMACTDTPR, (uint32_t) heth->Init.TxDesc);
to
/ Set Transmit Descriptor Tail pointer */
(Line 2663) WRITE_REG(heth->Instance->DMACTDTPR, ((uint32_t)(heth->Init.TxDesc + (uint32_t)(ETH_TX_DESC_CNT - 1))));

Please let me know if there is any difference in TX and RX descriptors. I am not expert on this Ethernet HAL handling on stm32h7.

Thanks
Tushar

from stm32cubeh7.

@tushartp You're welcome to ask this in the forum.

from stm32cubeh7.

To make the code's intention clearer, I would suggest

WRITE_REG(heth->Instance->DMACRDTPR, (uint32_t)(&heth->Init.RxDesc[ETH_RX_DESC_CNT - 1]));

Much clearer, and doesn't require knowledge of pointer arithmetic rules.

from stm32cubeh7.