feat: Add 32-bit timer support for waveform function

[CNLHC]

Currently waveform_chx function in SimplePwm assume that all timer are 16-bit, which will cause some unexpected issue ( e.g. #2522)

This PR add 32-bit timer support for waveform function by changing the input data from &[u16] to more general &[u8], and use unsafe transmute to prepare &[u16] or &[u32] based on the timer size for DMA.

Approach in this PR is far from perfect. I think constant generic type may be a better solution, but will introduce significant changes to current timer HAL in embassy-stm32

[Dirbaio]

I'm not sure if changing the function's behavior (receiving a 16bit vs 32bit waveform data) depending on the timer's bitness is the best solution.

In general the 32bit timers are designed so you can treat them as if they were 16bit and they mostly just work. This is useful if for example you need 4 16bit timers, and your chip has 3 16bit timers and 1 32bit timer. You can just treat them all as 16bit timers. With the solution in this PR you'd have to special-case the 32bit timer to generate a 32bit waveform for one timer and 16bit waveforms for the others.

It'd be nicer if gen_waveform() takes a 16bit waveform always. We could add another function gen_waveform_32bit() that takes a 32bit waveform, and has a where clause so you can only call it on 32bit timers.

Do you know why the 16bit DMA doesn't work for the 32bit timers? Perhaps there's a way to make it work.

[CNLHC]

I'm not sure if changing the function's behavior (receiving a 16bit vs 32bit waveform data) depending on the timer's bitness is the best solution.

In general the 32bit timers are designed so you can treat them as if they were 16bit and they mostly just work. This is useful if for example you need 4 16bit timers, and your chip has 3 16bit timers and 1 32bit timer. You can just treat them all as 16bit timers. With the solution in this PR you'd have to special-case the 32bit timer to generate a 32bit waveform for one timer and 16bit waveforms for the others.

It'd be nicer if gen_waveform() takes a 16bit waveform always. We could add another function gen_waveform_32bit() that takes a 32bit waveform, and has a where clause so you can only call it on 32bit timers.

Do you know why the 16bit DMA doesn't work for the 32bit timers? Perhaps there's a way to make it work.

As I said in the first, changing parameters of the function is not the best solution. I think the best solution is using constant generic type to distinguish different timers and check the data width in the compile time.

You are right 16-bit and 32-bit timer are used with an interchangeable way, but the problem here actually is the DMA request.

Currently, the DMA will infer data size from the type of source buffer, which means if you use &[u16] then it will assume the source address and dest address both have two-bytes length. For example, if you send let data: [u16;4] = [0x12,0x34,0x56,0x78] to a 32-bit timer via DMA, actually you will send [0x0012,0x3400,0x0056,0x7800] ( or [0x0012,0x3412,0x3456,0x7856] if the timer CCRx will not reset after an interrupt, sorry I am not familar with this part but i hope this example can explain the problem)

I aggree with you to make waveform_xx funtion only taks a 16bit waveform with an extra where clause, but I have not found an easy way to do that. I will check how to add type constraint to associate const ( BITS in CoreInstance trait) today.

[CNLHC]

Here are some more details

The problem here is we are trying to dma copy a &[u16] buffer to the 16-bit or 32-bit TIMx_CCRx channel, while MINC set to true, PINC set to false , MSIZE set to 16bit and PSIZE set to 16bit(both for 16-bit and 32-bit timers)

MAR: &[u16]   // for user data
PAR: BASE+0x34  // for TIMx_CCRx adderss
MINC: true // increment memory address
PINC: false // do not increment periperal address
MSIZE: 16bit // read 16bit data in each dma transfer
PSIZE: 16bit // assume the peripheral is 16-bit length, no matter the real size.

For 16-bit Timer, everything works fine, each time the timer send DMA requests, DMA controller will copy a 16-bit data to TIMx_CCRx and increment the memory address to prepare for the next transfer.

For 32-bit Timer, things become a little bit weird.

With PINC=false, the periperial address is expected to be constant during different transfers, which means 32-bit timer are actually equivalent to the 16-bit ones because of every DMA transfer will only write 16 bits to the TIMx_CCRx[0:16]. Meanwhile, TIMx_CCRx[16:31] can never be touched since the peripheral address can not be incremented automatically due to PINC and PSIZE setting.

But the experiments shows that:

1. if we use current settings, stm32 chip seems to fill both TIMx_CCRx[0:16] and TIMx_CCRx[16:31] , which means the peripheral address do increment and reset automatically to make DMA controller fill the 32-bit TIMx_CCRx, even if PSIZE=16 and PINC=false.
2. if we set PSIZE=32, then everything works fine. the 16bit data is copied to TIMx_CCRx[0:16] without changing TIMx_CCRx[16:31].

So my hypotheses is stm32 has some mechanism to detect mismatch between PSIZE and real register width. If PSIZE is smaller than the width, the hardware will try to fix error by implictly increment and reset peripheral address to make DMA transfer to cover all 32bit of register.

[Dirbaio]

So my hypotheses is stm32 has some mechanism to detect mismatch between PSIZE and real register width. If PSIZE is smaller than the width, the hardware will try to fix error by implictly increment and reset peripheral address to make DMA transfer to cover all 32bit of register.

I don't think this is the case. The DMA just blindly fires write requests on the bus, it has no way to "know" the size of a register.

Maybe it's this?

RM0368 2.1.7 AHB/APB bridges (APB): 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.

if this is true then a waveform that's [0x0001, 0x0002, 0x0003] will cause DMA to write 0x00010001, 0x00020002, 0x00030003. It should be easy to check: Make DMA do one write, then read with the CPU and look at the value.

Have you tried setting MSIZE=16bit, PSIZE=32bit in the DMA? It should work on chips with BDMA and GPDMA (all chips except F2, F4, F7, H7), because those will zero-extend the value (read 0x0001 from RAM, write 0x0000_00001 to TIMx). I think it's only chips with DMA that have this problem, because it will do packing (read 0x0001, 0x0002 from RAM, write 0x0002_0001 to TIM) and there seems no way to turn that off.

If this is the case, I still think we should try to make the 16bit waveforms work on 32bit timers in the chips we can. So it would be something like:

• do_waveform_16bit(): on F2, F4, F7, H7 it works only on 16bit timers, on others it works on all timers.
• do_waveform_32bit(): works only on 32bit timers.

[CNLHC]

Thanks for your suggestion!

Maybe it's this?

RM0368 2.1.7 AHB/APB bridges (APB): 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.

Never heard this behavior before, buf after reading the document you quoted, I believe this is the root cause, I will try to check it in the future.

Have you tried setting MSIZE=16bit, PSIZE=32bit in the DMA?

I have triedMSIZE=16bit, PSIZE=32bit in L4 series , things work as you expected.

If this is the case, I still think we should try to make the 16bit waveforms work on 32bit timers in the chips we can. So it would be something like:
do_waveform_16bit(): on F2, F4, F7, H7 it works only on 16bit timers, on others it works on all timers.
do_waveform_32bit(): works only on 32bit timers.

Agree, I will refactor this PR to fullfill this behavior

[CNLHC]

@Dirbaio May you have a look at the current version?

The core idea is to tweak the dma HAL to make it possible to use different mem_size and peripheral_size.

[Dirbaio]

bender run

(to run HIL tests, to check if the DMA changes break something)

[CNLHC]

ERROR - 0.000053 panicked at /ci/code/tests/stm32/src/bin/usart_dma.rs:44:40:

i will look at this error today 🤔

[Dirbaio]

it's a false positive, some tests randomly fail.

bender run

[lucasmerlin]

I ran into #2522, gave this PR a try and gen_waveform now works with TIM2. Great work!

[Dirbaio]

thank you!