stm32-data icon indicating copy to clipboard operation
stm32-data copied to clipboard

Extract DMA remapping data for F0/F3

Open fwolter opened this issue 11 months ago • 4 comments

This is the basis to fix embassy-rs/embassy#3643.

fwolter avatar Dec 15 '24 18:12 fwolter

diff: https://ci.embassy.dev/jobs/1b945c6febda/artifacts/diff.html

embassy-ci[bot] avatar Dec 15 '24 18:12 embassy-ci[bot]

diff: https://ci.embassy.dev/jobs/7b09db6e4a01/artifacts/diff.html

embassy-ci[bot] avatar Dec 15 '24 19:12 embassy-ci[bot]

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
  }
}

Dirbaio avatar Jan 01 '25 23:01 Dirbaio

Thank you very much for your feedback! I will incorporate it. I wasn't aware there were even more remapping bits.

fwolter avatar Jan 02 '25 08:01 fwolter

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&amp;(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?

fwolter avatar Jul 09 '25 09:07 fwolter

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.

Dirbaio avatar Jul 09 '25 13:07 Dirbaio

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?

fwolter avatar Jul 09 '25 15:07 fwolter

either a new yaml, or just put the data in a .rs file like we already do with the memory maps.

Dirbaio avatar Jul 09 '25 16:07 Dirbaio

diff: https://ci.embassy.dev/jobs/5148eec17f0c/artifacts/diff.html

embassy-ci[bot] avatar Jul 12 '25 15:07 embassy-ci[bot]

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.

fwolter avatar Jul 12 '25 15:07 fwolter

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 remap to 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 put 0,1 instead 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.

Dirbaio avatar Jul 13 '25 23:07 Dirbaio

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.

fwolter avatar Jul 15 '25 07:07 fwolter

diff: https://ci.embassy.dev/jobs/7ab67f54a3de/artifacts/diff.html

embassy-ci[bot] avatar Jul 15 '25 08:07 embassy-ci[bot]

  • *_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.

Dirbaio avatar Jul 15 '25 09:07 Dirbaio

diff: https://ci.embassy.dev/jobs/830072a4b6de/artifacts/diff.html

embassy-ci[bot] avatar Jul 17 '25 14:07 embassy-ci[bot]

lgtm, thanks! :rocket:

Dirbaio avatar Jul 17 '25 15:07 Dirbaio

Thank you!

fwolter avatar Jul 17 '25 15:07 fwolter