Skip to main content
AAshc
AAshc
Associate
November 20, 2019
Solved

Hello! May be it is wrong ethernet DMA tail calculation?

  • November 20, 2019
  • 6 replies
  • 3557 views

In ethernet HAL driver stm32h7xx_hal_eth.c function void ETH_DMARxDescListInit include row: " WRITE_REG(heth->Instance->DMACRDTPR, ((uint32_t)(heth->Init.RxDesc + (((uint32_t)(ETH_RX_DESC_CNT - 1))*sizeof(ETH_DMADescTypeDef)))));"

This code adding value to heth->Init.RxDesc. But, RxDesc is pointer to 32bit and added shift is multiplicated for 4.

Correct row must be:

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

This topic has been closed for replies.
Best answer by Pavel A.

Created an issue on github:

https://github.com/STMicroelectronics/STM32CubeH7/issues/7

Yep, this is why "better" languages like Go don't have pointer arithmetic. Too confusing. Causes more trouble than utility.

-- pa

6 replies

Pavel A.
Pavel A.
November 20, 2019

RxDesc is pointer to ETH_DMADescTypeDef. 6*4 bytes,

-- pa

Piranha
Piranha
Principal III
November 20, 2019

Pavel is right - RxDesc is a pointer to a structure. Therefore neither ST's, nor your version is correct because of C pointer arithmetic, but the correct version is even simpler:

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

ST, this is a bug report and we're talking about this line:

https://github.com/STMicroelectronics/STM32CubeH7/blob/e261d820b398846a94f22f4aeb32d86c29546efb/Drivers/STM32H7xx_HAL_Driver/Src/stm32h7xx_hal_eth.c#L2704

P.S. ST's driver developers, who don't know C pointer arithmetic? Fabulous! Do they know arithmetic at all?

waclawek.jan
waclawek.jan
Super User
November 20, 2019

@Piranha ,

@AAshc​ 's version is correct, too, as he casts the pointer to integer before applying the + operator, and then uses sizeof() of the target type.

I personally am not fan of pointer arithmetics, as it's bit easily perceivable when fast reading code; and I recommend to explicitly comment all instances where it's used, or, maybe better, use it in the &a[b] form, which is more easily recognizable when reading the code.

(Strictly speaking, pointer cast to integer is not entirely correct, but then all three versions do some form of that).

JW

Piranha
Piranha
Principal III
November 20, 2019

Indeed, I stand corrected! I overlooked the braces in OP's version. Therefore only ST's version is flawed.

waclawek.jan
waclawek.jan
Super User
November 20, 2019

I don't really care as I don't Cube, but is this limited to H7?

JW

Piranha
Piranha
Principal III
November 21, 2019

Yes, H7 only. Other STM32 series doesn't even have that or similar register. H7 has a different ETH peripheral than all other STM32 MCUs.

waclawek.jan
waclawek.jan
Super User
November 21, 2019

Oh, I see. Thanks for the explanation.

JW

Piranha
Piranha
Principal III
February 2, 2020

By the way, here is the respective code line from H7 HAL v1.3.0:

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

It's essentially the same as the OP's version. It relied on C operator precedence, but it was correct! Until some brainless code monkey "fixed" it by adding braces in wrong places...

LCE
LCE
Principal II
January 10, 2023

I'm just changing my non-HAL ethernet driver from F7 to H7 - oh my, it is so different.

At start, why not set the tail pointer directly to the last descriptor's address?

ETH->DMACRDTPR = (uint32_t)&DMARxDscrTab[ETH_RX_DESC_CNT - 1];

But maybe I have not yet fully understood the tail pointer.

Thankfully I found this thread with @alister​ 's explanation.

https://community.st.com/s/question/0D53W00000EGsnUSAT/how-does-ethernet-rx-descriptor-wrapping-work-on-stm32h7

Terms & ConditionsAccessibility statement