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.

[Pavel A. · Accepted solution]

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

[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

[Amel NASRI]

A good practice @Pavel A.​ ! Thanks for taking the initiative to report the issue there.

So the status of the progress can be followed on Github now.