stm32-data
stm32-data copied to clipboard
Extract DMA remapping data for F0/F3
This is the basis to fix embassy-rs/embassy#3643.
diff: https://ci.embassy.dev/jobs/1b945c6febda/artifacts/diff.html
diff: https://ci.embassy.dev/jobs/7b09db6e4a01/artifacts/diff.html
Putting a single bool in the generated data is too "ad hoc". I think the data should contain the full info of which peripheral+register+field to set to enable the remap.
Reasons:
- The bool is specifying the value, but it's not specifying which field. This makes users (like the embassy-stm32 PR you sent) to search it by name themselves. It's nicer if the data is "as cooked as possible" so using it is easier.
- There's more chips with "remaps" like this, for example F1 for GPIO. These are on different peripherals (AFIO) and registers (MAPR, MAPR2). The data model should ideally be the same for all chips, so remaps can be handled the same way.
For example it could be this:
{
"signal": "ADC2",
"channel": "DMA2_CH1",
"remap": {
"peripheral": "SYSCFG",
"register": "CFGR1",
"field": "TIM17_DMA_RMP",
"value": 0
}
}
Thank you very much for your feedback! I will incorporate it. I wasn't aware there were even more remapping bits.
I went down the rabbit hole and figured out that the cubedb XML files have not sufficient information to determine all remap fields. Example: F0 has SYSCFG_CFGR1.TIM16_DMA_RMP2. Whether RMP or RMP2 needs to be set is not defined in the XML file (there are other places in XML files where remaps are defined, but none of them provide these information):
<RefParameter Comment="DMA Remap" DefaultValue="DMA_REMAP_TIM16_DMA_CH6" Group="" Name="DMA_Remap" Type="list" Visible="false">
<PossibleValue Comment="TIM16 CH1/UP DMA CH6 remap" Value="DMA_REMAP_TIM16_DMA_CH6"/>
<Condition Diagnostic="" Expression="TIM16_CH1_UP_CH6_Remap&(Instance=DMA1_Channel6)"/>
</RefParameter>
Instead, DMA_REMAP_TIM16_DMA_CH6 from the XML file references to https://github.com/STMicroelectronics/stm32f0xx-hal-driver/blob/94399697cb5eeaf8511b81b7f50dc62f0a5a3f6c/Inc/stm32f0xx_hal_dma.h#L305.
Would you accept to include the header files from stm32f[03]xx-hal-driver in stm32-data-sources to extract the fields from there?
Up to you!
However, it seems it's turning out to be quite complex. Maybe it's easier to just enter enter the data manually? There's not that many remaps and they're only present in the older families which aren't seeing new releases anymore so it's unlikely to become a maintenance burden.
I agree, that'd be much easier! How about introducing a new category in the family extra YAML files to define the mappings? Would work for the F0 family - the remaps are all the same within the family, but within F3 there are at least three different maps. Or introduce data/dma/ with dedicated YAML files?
either a new yaml, or just put the data in a .rs file like we already do with the memory maps.
diff: https://ci.embassy.dev/jobs/5148eec17f0c/artifacts/diff.html
That took longer than expected to get all the device types and their IP versions straight. The ridiculous number of copy-paste errors and typos in the reference manuals didn’t help, though 😅 But in the end, it turned out relatively simple.
I also implemented your review feedback in the PR over in embassy.
looking good! two things:
- There's remaps that are split in two bits. I think it'd be cleaner to merge them into a single 2-bit field, then represent the values as numbers 0,1,2,3. This'd allow changing
remapto be one single object, not an array.
{
"signal": "CH1",
"channel": "DMA1_CH4",
"remap": [
{
"peripheral": "SYSCFG",
"register": "CFGR1",
"field": "TIM16_DMA_RMP",
"value": "true"
},
{
"peripheral": "SYSCFG",
"register": "CFGR1",
"field": "TIM16_DMA_RMP2",
"value": "false"
}
]
},
- Making the values
"true"and"false"as strings is a bit strange. I'd put0,1instead and make embassy-stm32 deal with this. It'll be more consistent with the 2-bit fields like the example above. Also, the JSON data is supposed to be language-agnostic, the fact that chiptool chooses to generate 1bit fields as bools is rust-specific.
I don't think the array can be eliminated easily:
- *_RMP and _*RMP2 are distinct fields in the register, they are not adjacent bits. Technically, it'd make sense to set them in one instruction, but then we'd need to include a bitmask (some also need to be cleared) and drop the field name.
- STM32F334x has bits split across CFGR1 and CFGR3 for a single remap.
You're right, value should be a number. Now, I check the metadata if a bool or number is needed in embassy. Let me know if there is a simpler solution. Unfortunately u32 has no .into() to convert to bool.
diff: https://ci.embassy.dev/jobs/7ab67f54a3de/artifacts/diff.html
- *_RMP and _*RMP2 are distinct fields in the register, they are not adjacent bits. Technically, it'd make sense to set them in one instruction, but then we'd need to include a bitmask (some also need to be cleared) and drop the field name.
- STM32F334x has bits split across CFGR1 and CFGR3 for a single remap.
WTF.
diff: https://ci.embassy.dev/jobs/830072a4b6de/artifacts/diff.html
lgtm, thanks! :rocket:
Thank you!