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

[AAshc]

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)));"

[Pavel A.]

RxDesc is pointer to ETH_DMADescTypeDef. 6*4 bytes,

-- pa

[Piranha]

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]

@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]

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

[waclawek.jan]

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

JW

[Piranha]

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]

Oh, I see. Thanks for the explanation.

JW

[Piranha]

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]

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