stm32f4xx_hal_spi.c, HAL_SPI_TransmitReceive - conversion of HW register pointer

[Marcin Wolcendorf]

Posted on June 04, 2018 at 16:25

Hi Everyone,

In HAL_SPI_TransmitReceive, around line 937, there is a gem like that:

 *((__IO uint8_t*)&hspi->Instance->DR) = (*pTxData);

Isn't it a bit dangerous to convert the HW register, which always have a specific size and  usually also access restrictions (in case of DR - reading it will delete the received data, for example), instead of converting the data read from the buffer? Wouldn't it be better to have something like that instead:

hspi->Instance->DR = (uint32_t)(*pTxData);

BR,

M.W.

#hw-register-access #spi #hal_spi_transmitreceive #data-integrity

[Tesla DeLorean]

Posted on June 04, 2018 at 17:21

You don't understand the purpose of the code.

The code defines how wide the write occurs at a bus level. You can do this in assembler more simply. It does not read the register.

The register behaves differently depending if you write 8, 16 or 32-bit wide values.

Not going to help writing 4 bytes when you only have 3

[Marcin Wolcendorf]

Posted on June 04, 2018 at 18:26

You don't understand the purpose of the code.

Apparently I do not. 

And I am quite certain I am not the only one. Yet - there is no comment in the code explaining the strange construct. 

The register behaves differently depending if you write 8, 16 or 32-bit wide values.

Really? Could you point me to the right passage in the documentation?

Is there a difference in the execution time between 8-bit, 16-bit and 32-bit data transmission to the APB bus? Considering the DR is 16-bit wide (and the structure for register mapping - SPI_TypeDef - makes it 32-bit),  the CPU is 32-bit, the busses are 32-bit, and these 2 passages from the RM0090 rev 16,

p. 923:

For an 8-bit data frame, the buffers are 8-bit and only the LSB of the register

(SPI_DR[7:0]) is used for transmission/reception. When in reception mode, the MSB of

the register (SPI_DR[15:8]) is forced to 0.

For a 16-bit data frame, the buffers are 16-bit and the entire register, SPI_DR[15:0] is

used for transmission/reception.

(so - I can write 16 bits and only 8 will be used in 8-bit mode, when I read 16 bits in 8-bit mode, the upper 8 bits will be 0)

and p. 64:

Note:

When a 16- or an 8-bit access is performed on an APB register, the access is transformed

into a 32-bit access: the bridge duplicates the 16- or 8-bit data to feed the 32-bit vector.

(The SPI is on the APB bus, see the 

Table 1. STM32F4xx register boundary addresses

) - so all the accesses are converted to 32-bit anyway, with an added danger, that the values written are repeated.

Again - why would there be strange, unreadable, undocumented conversion, if any access to SPI is converted to 32-bit anyway, and there is no execution time difference?

[AvaTar]

Posted on June 05, 2018 at 07:51

Really? Could you point me to the right passage in the documentation?

The SPI peripheral section, and the DR register in particular.

The same applies to other peripherals, like the UART, I2C and ADC. Access as 8 bit and 16 bit is mentioned explicitly.

The ST coder could have added a comment, to explain what he wanted to achieve with this cumbersome-looking code.

It seems documentation doesn't pay for free software ...

[Marcin Wolcendorf]

Posted on June 05, 2018 at 09:25

Really? Could you point me to the right passage in the documentation?

The SPI peripheral section, and the DR register in particular.

The same applies to other peripherals, like the UART, I2C and ADC. Access as 8 bit and 16 bit is mentioned explicitly.

I've read that. I have even mentioned it in the post you're replying to. 

There is no difference in the register operation, regardless of the width of the access by CPU (and the WTF code in question does exactly that - changes the width of the data transfer to/from the CPU), as the bus bridge will make all the accesses to the APB peripherals 32-bit by duplicating the data (again - see my previous post). There is a difference in DR operation depending on the setup of the SPI with some consequence if (and only if) you put 1 byte to DR in 16-bit mode (in which case you will have it sent twice because of the APB bus bridge duplication). To avoid this it is enough to convert all the *input data* (not the register pointer!) to 16- or 32-bit. 

Which brings me back to the original question - why converting the HW register pointer and not the data put into the register? So far I cannot see any reason for register pointer conversion.

AvaTar wrote:

The ST coder could have added a comment, to explain what he wanted to achieve with this cumbersome-looking code.

I'd argue that *fixing* this code would be better. 

[T J]

Posted on June 05, 2018 at 14:12

All of you are way more advanced.

I am never sure which is little indian or the other one...

So of course we use:

(__IO uint8_t*)&hspi->Instance->DR // to force an 8 bit transaction.

it takes the same time

Turvey.Clive.002

‌ is suggesting that from ARM to ARM from Brand to Brand you can trust its going to work:

(__IO uint8_t*)&hspi->Instance->DR // it will always work on any ARM system.

and of course

it takes the same time

[AvaTar]

Posted on June 05, 2018 at 14:41

Turvey.Clive.002

is suggesting that from ARM to ARM from Brand to Brand you can trust its going to work:

(__IO uint8_t*)&hspi->Instance->DR // it will always work on any ARM system.

On the bus itself, yes.

What happens inside the peripheral (SPI) is totally up to the vendor - ST in this case.

Some other vendors considered it a good idea to transplant their 8-bit peripherals directly on Cortex M devices. Not even sure if all the registers (8-bit, of course) were 32-bit aligned. They are bought up in the meantime.