force input buffer for CRC to be aligned 32 bits

[Bassem]

Dear ST community,

I noticed that the CRC API is forcing the input buffer to be aligned 32 bits, which can be understood in order to be generic, but in the majority of the case we have a byte buffer which could be stored in unaligned in the memory or it's taken from frame where storage address is unaligned, till now it's not a problem we can cast (uint32_t *) and everything should work, except the case where the core of STM32 is Cortex M0 or M0+ so unaligned access is not supported and will lead to hard fault if the InputDataFormat is not selected to be CRC_INPUTDATA_FORMAT_BYTES .

In my opinion the exposed API with forced aligned buffer can be an error prone, there is for sure a lot of workaround that can be applied but I could prefer to add to the API CRC_Handle_8 and CRC_Handle_16 to the exposed API, with that it will solve immediately the problem and remove any usage ambiguity.

Maybe I'm wrong and I miss something.

Best regards

Bassem Taamallah

[TDK]

It's only forced to be 32-bit aligned if you're calculating on 32-bit values. If you're calculating over bytes, alignment is not needed or forced. Where are you seeing that it is?

[unsigned_char_array]

Please share your code and the MCU you are using. I've used hardware CRC in the past and don't recall having this issue.

[Bassem]

Maybe I was not describing the problem correctly, so let's do it by giving example.

 1) initialization we choose wrongly and here insist in wrongly InputDataFormat :

hcrc.Instance = CRC;
hcrc.Init.DefaultPolynomialUse = DEFAULT_POLYNOMIAL_DISABLE;
hcrc.Init.GeneratingPolynomial = 0x8024;
hcrc.Init.CRCLength = CRC_POLYLENGTH_16B;
hcrc.Init.DefaultInitValueUse = DEFAULT_INIT_VALUE_DISABLE;
hcrc.Init.InitValue = 0x0000;
hcrc.Init.InputDataInversionMode = CRC_INPUTDATA_INVERSION_BYTE;
hcrc.Init.OutputDataInversionMode = CRC_OUTPUTDATA_INVERSION_ENABLED;
hcrc.InputDataFormat = CRC_INPUTDATA_FORMAT_WORDS;  ===> this field it was misleading for me

2) now we start the CRC calculation :

uwCRCValue = HAL_CRC_Calculate(&hcrc, (uint32_t *)(input_buffer), sizeof(input_buffer));

input_buffer it's frame of bytes located at the address 0x200000041 (unaligned address)

by applying this example we will get immediately Hard fault because unaligned access to input_buffer.

so we need to declare  InputDataFormat as format byte to use the correct alignment.

So when selecting the buffer we need to take care to select the correct value to the InputDataFormat, which I find that such way of configuring is error prone because two parameters are depending from each other.

Another point usually selecting a wrong parameter should lead to HAL_ERROR but API must not lead to Hard Fault.

As conclusion, I suggest to avoid changing the API is to add a check inside the API and return HAL_ERROR if the address of the buffer didn't correspond to the input format

Best regards

Bassem 

[unsigned_char_array]

---

@Bassem wrote:

hcrc.InputDataFormat = CRC_INPUTDATA_FORMAT_WORDS;  ===> this field it was misleading for me

I see. Yes the CRC peripheral requires 32-bit data to be 4 byte aligned and 16-bit data to be 2-byte aligned.
The DMA also has alignment requirements.Peripherals that deal with memory often require some type of alignment. This is written in the user manual and in the API docs. Sometimes library functions such as memcpy have alignment requirements and don't always work with members of packed structs.

---

@Bassem wrote:

Another point usually selecting a wrong parameter should lead to HAL_ERROR but API must not lead to Hard Fault.

As conclusion, I suggest to avoid changing the API is to add a check inside the API and return HAL_ERROR if the address of the buffer didn't correspond to the input format

---

Excellent suggestion. The problem is that the current CRC HAL implementation doesn't have an return code for an error. So it would mean an API breaking change. I suggest you implement your own wrapper function around the HAL CRC or just use the correct configuration from now on. Maybe using c++ would make it cleaner as you can overload the function or use a template to check for proper alignment.
If you want to report this as a bug you should add the bug-report tag. But I see it more as a feature request.