svd2rust icon indicating copy to clipboard operation
svd2rust copied to clipboard

Common traits for identical IP blocks

Open jameysharp opened this issue 7 years ago • 30 comments

I want to be able to write generic functions that can operate on any instance of the same peripheral across a vendor's product line, or any instance of the same peripheral within a given chip.

For a concrete example: as far as I know, ST Micro uses the same bxCAN IP on every chip in the STM32 line that has a CAN peripheral, and on some chips they have multiple copies of the bxCAN peripheral. So I'd like to be able to write generic code that uses a bxCAN peripheral, independent of which STM32 product you're building for and which instance of the peripheral you're using.

I think a lot of the relevant information can be inferred directly from the .svd files, by checking whether two peripherals have identical register definitions, ignoring reset values. But I can't decide how much manual intervention makes sense.

What are your thoughts?

jameysharp avatar May 24 '17 03:05 jameysharp

This sound a bit like the derivedFrom feature that SVD files have, except that this would have to operate across different files. I think it makes sense to have some sort of inheritance between files. For example:

Suppose that some peripherals (e.g. TIM7) are common to several STM32 devices so those could be factored out in a common file, e.g. stm32_common.svd then a particular SVD could refer to that common file:

<!-- stm32f100xx.svd -->
<peripherals>
  <peripheral derivedFrom="stm32_common.svd TIM7">
    <name>TIM7</name>
    <baseAddress>0xdeadbeef</baseAddress>
  </peripheral>
  ..
</peripherals>

Then svd2rust would generate something like this:

extern crate stm32_common;

pub const TIM7: Peripheral<stm32_common::tim7::RegisterBlock> = unsafe { Peripheral::new(0xdead_beef) };

Where stm32_common is the crate generated by svd2rust from stm32_common.svd. Then you could write your library generically against stm32_common and be able to use derived crates like stm32f100xx as the types would match.

The thing is that SVD files don't support this syntax, and to make this work we would have to modify SVD files to indicate the "file inheritance". So I'm not sure how to proceed here.

Perhaps we could have some external file that specifies that the peripheral TIM7 of stm32f100xx is derived from the peripheral TIM7 of stm32_common. That would make svd2rust ignore stm32f100xx.svd's information about TIM7 and generate code like above.

japaric avatar May 24 '17 17:05 japaric

I really like your external-file proposal, to avoid having to modify the vendor-provided files. It doesn't have to completely ignore the original SVD, though. For one thing, it could check that the API for the common version is a superset of the API it would have generated, perhaps adding enumerated values but not changing anything else, as a sanity check.

Also, ideally I'd want the reset value for each register taken from the vendor definition, not the common one. This matters even within one chip, not just across a product line. For example, on STM32F411, GPIOA and GPIOB are not derivedFrom any other GPIO block, but all of the GPIO blocks are identical except for reset value. Since reset value doesn't affect the API from the user's point of view, ideally it wouldn't force different types either.

For convenience, it'd be nice to also build a tool that generates an initial common.svd and external map from a directory full of SVD files for a particular product line, by automatically identifying register layouts that are equivalent. I think that's an entirely automatable process.

Thanks for thinking about this! I feel my thoughts are more clear already from your feedback. 😁

jameysharp avatar May 24 '17 18:05 jameysharp

As noted above, being able to have common interface would be useful not only across different devices, but also across different peripherals in the same device.

Moreover, some peripherals could have different features, but still share some common functionality.

For example, take STM32 timers. There are three types of timers: "basic", "general-purpose" and "advanced". All three share features of "basic" timer. "general-purpose" and "advanced" have few more features (like "slave mode").

Library that uses quadrature encoder feature, for example, might be coded against interface that is common between "general-purpose" and "advanced" timers (since "basic" timer does not support quadrature encoders). Some other library might be coded against interface that is common across all three types of timers. Motor control application would only work with "advanced" timer.

As for the common SVD file approach, would SVD "duck typing" work here? You can make it that any peripheral that matches some definition from common SVD would automatically implement traits generated from that SVD. Something along these lines.

Similar question is that how well common interfaces based on low-level registers would work? For example, GPIO ports have similar features in STM32F051 vs STM32F103 (pull-ups/downs, input/output, BRR/BSRR registers), but configuration registers are completely different. I don't really like HAL library, but, I think, this is the problem it tries to solve.

idubrov avatar May 24 '17 19:05 idubrov

I think in the case of timers peripheral inheritance would work. Basically the general purpose timer (e.g. TIM2 would derive from the basic timer (e.g. TIM7). In the SVD this would be represented by having <peripheral derivedFrom="TIM7"> for TIM2 plus the <peripheral> tree would include the registers that TIM2 has but TIM7 hasn't. In the svd2rust generated code the change would be that tim7::RegisterBlock would be able to deref to tim2::Registerblock. Finally the end user would be able to write APIs like this:

fn do_something_with_basic_timer(tim: &tim7::RegisterBlock} { .. }

do_something_with_basic_timer(&TIM7);
do_something_with_basic_timer(&TIM2);

This hasn't been implemented (I think there's an issue open about this) but it's something that SVDs support.

but configuration registers are completely different.

Peripheral inheritance only handles the case where the registers of a peripheral are a subset of other peripheral's. For this GPIO case you would have to write a HAL.

japaric avatar May 24 '17 20:05 japaric

Is it possible to inherit the whole peripheral and then add an extra field to some inherited register? For example, "basic" timer would not have field CKD (clock division) in CR1, but "general-purpose" timer would.

idubrov avatar May 24 '17 20:05 idubrov

The SVD spec says

derivedFrom Specify the peripheral name from which to inherit data. Elements specified subsequently override inherited values.

In your case if TIM2 defines a CR1 <register> tree then it must define all the fields of CR1. At that point TIM7.CR1 and TIM2.CR1 have different types so maybe my idea of using Deref would have be more complicated to implement that I expected (or not implementable in the way I depicted above).

japaric avatar May 24 '17 21:05 japaric

@idubrov, good point about duck typing. I can't imagine a circumstance where two peripherals with identical registers would need different types. So maybe the external file mapping definitions back to the common SVD is unnecessary.

Regarding your other two points, I think they're important questions, maybe with the same answer. Is it enough to hand-write traits and implementations to cover the commonalities between peripherals that are not identical but have overlapping functionality? Regarding STM32 timers, I'm imagining that the crate generated by svd2rust for the common SVD would supplement the auto-generated Rust module with a handwritten module of traits and their implementations for the common peripherals.

@japaric, as long as the fields in corresponding registers are a subset, and fields that are in both are at the same offsets, then a Deref implementation based on mem::transmute should work fine, I think?

jameysharp avatar May 24 '17 21:05 jameysharp

as long as the fields in corresponding registers are a subset, and fields that are in both are at the same offsets, then a Deref implementation based on mem::transmute should work fine, I think?

Yes that would work. Just pointing out that the derivedFrom feature does overrides instead of "append this stuff to the base". This makes the subset check trickier because you have to recursively check that all uses of derivedFrom are being used to extend the base, and not to override some of its properties.

And I forgot to comment on this before:

by checking whether two peripherals have identical register definitions, ignoring reset values

It is required that the reset values match otherwise register.reset() would behave differently on each device. Also all write operations start from the reset value: the methods of the W proxy modify that reset value, and the modified value is written into the register. Again, if the reset values don't match then register.write(|w| w.bitfield().bits(1)) would behave differently.

japaric avatar May 24 '17 21:05 japaric

While I agree that both .reset and .write need to behave differently on peripherals with different reset values, that has no effect on their API—they are source-code compatible. So I'd still like to be able to write code that is generic over different instances of a peripheral even if those instances have different reset states. The Rust compiler would wind up producing different binary code, but the source code needn't change. If the differing reset values are in fields that are meaningful to the generic code, then it should set them explicitly.

jameysharp avatar May 24 '17 22:05 jameysharp

If the differing reset values are in fields that are meaningful to the generic code, then it should set them explicitly.

I think this sets the bar too high. Now the author of the generic code needs to (a) check every single device that their code might be used against while writing the implementation, or (b) write the implementation very defensively making sure to never use reset() and to set all the fields when using write() without compiler assistance. If they forget even one field in some write call they will have possibly introduced a bug in their code that will go unnoticed during their testing until it blows up in someone else's face. IMO, if we want to expose generic code where the reset value is unknown then the API should reflect that and (a) not expose reset and (b) somehow force the caller of write to set all the register's fields.

japaric avatar May 25 '17 20:05 japaric

That's a reasonable position. I'm not sure I agree, but I'd be happy to see a version of svd2rust that addresses my other concerns without being generic over reset values. Once we have experience with some level of generic API, we'll be better able to evaluate whether it makes sense to be generic over reset values too.

So, what would you suggest for next steps?

jameysharp avatar May 25 '17 20:05 jameysharp

Tentative next steps:

  • Come up with a definition for "peripheral equality". When two peripherals are equal svd2rust can automatically use the current "derivedFrom" logic. But when are two peripherals considered equal?

    • My first estimate would be: peripheral names don't matter, base addresses don't matter, all the registers must be "equal" -- this register equality can leave off documentation (<description> fields). I consider that reset values of each register must match in value as well but the OP disagrees. Interrupt information would likely be left off from the analysis as it doesn't matter on which peripheral that information appears (*)
  • Put this definition to test. IMO, the easiest way would be to write a tool that uses the same SVD parser that svd2rust uses and looks for identical peripherals within a single SVD file . The SVD files @ryankurte linked in rust-embedded/rfcs#33 seem like a good test suite: it seems that e.g. USART1 and USART0 are identical; the tool should confirm this.

    • At this point we could simplify some SVD files by using "derivedFrom" in places where it was not previously used. The tool would indicate where "derivedFrom" can be used.
  • Once the definition is in good shape the tool could be extended to search for equivalent peripherals across different SVD files. It would good to run the tool across a bunch of SVD files that roughly belong to the same device family, e.g. all the STM32F1 devices, to get an idea of how similar those devices are. Running the tool across all the stm32 devices would be interesting as well. Is there any shared peripherall across all those?

  • The next step would be to decide how to deal with equivalent peripherals across different SVD files in svd2rust. The case where N devices share the same peripheral seems easy to handle with a single common crate and reexports, but if there are 3 or 4 variants of a peripheral across, let's say, 20 SVD files then how should the common crates be organized?

    • As I elaborated in https://github.com/rust-embedded/rfcs/issues/33#issuecomment-314297200 I consider that reducing the number of types and having device crates as re-exports of a common crate is the proper way to deduplicate code. But the trait approach should also be explored.

    • This experimentation even could be done "out of tree" by manually creating a SVD file with common definitions (by copy pasting the definitions from other files) and then running svd2rust on it: that would be the common crate. Then other devices crates can be simply written as reexports of the common crate, at least for the simplest cases.

It would good to run the tool across a bunch of SVD files that roughly belong to the same device family, e.g. all the STM32F1 devices, to get an idea of how similar those devices are.

@pftbest collected some similar information about MSP430 devices (see https://github.com/japaric/svd2rust/issues/122#issuecomment-313337126). Maybe they can share some insights about how they collected that information?

(*) derivedFrom leaves off interrupt information. See below:

    <peripheral derivedFrom="DMA1">
      <name>DMA2</name>
      <baseAddress>0x40020400</baseAddress>
      <interrupt>
        <name>DMA2_Channel1</name>
        <description>DMA2 Channel1 global interrupt</description>
        <value>56</value>
      </interrupt>
      <interrupt>
        <name>DMA2_Channel2</name>
        <description>DMA2 Channel2 global interrupt</description>
        <value>57</value>
      </interrupt>
      <interrupt>
        <name>DMA2_Channel3</name>
        <description>DMA2 Channel3 global interrupt</description>
        <value>58</value>
      </interrupt>
      <interrupt>
        <name>DMA2_Channel4_5</name>
        <description>DMA2 Channel4 and DMA2 Channel5 global
        interrupt</description>
        <value>59</value>
      </interrupt>
    </peripheral>

japaric avatar Jul 12 '17 01:07 japaric

Running the tool across all the stm32 devices would be interesting as well. Is there any shared peripherall across all those?

From my experience the answer is - almost all of them. For example in STM32F030F4 and in STM32F407VE general purpose timers are almost identical (the only difference I know is slave mode setings). GPIO have only two variants (if you ignore reset values): one in older STM32F1's and another everywhere else.

So at least for STM32 it could be practical to have a single common crate for all devices. It will probably have at most 2-3 variants of each peripheral (except RCC and AFIO which are very device dependent).

I will look into different reference manuals to see how far this compatibility really goes (I never worked with most powerful devices, like F7).

protomors avatar Jul 12 '17 09:07 protomors

I've just written a quick hack at using svd-parser to identify duplicate peripherals across one or more SVD files (https://github.com/jameysharp/share-svd). Most of the code involves re-implementing only the parts of svd-parser's data types that we should consider when deciding whether two peripherals are the same, so the compiler can automatically derive Ord/Eq for that subset of fields. If you want to try different rules for what makes peripherals equivalent, you can just change the struct definitions. For example:

  • Per @japaric's argument, I made reset value and reset mask part of what's compared, but one could try removing them and see what difference that makes.
  • I also made register and field names part of the comparison, but perhaps sometimes those are renamed arbitrarily without actually changing their behavior, so maybe pure structural equality would be useful.

Generally, I'd appreciate review of what I included and what I excluded to see if those choices make sense.

With that done, I've evaluated all of the STM32 parts in https://github.com/posborne/cmsis-svd and found that:

  1. Apparently, STMicro consistently uses derivedFrom for every peripheral that my current code considers equivalent. I counted non-derivedFrom peripherals using grep -oF '<peripheral>' *.svd | wc -l, then counted the number of peripherals my tool considered distinct, and in every one of those SVD files, the number of peripherals were equal. So there's nothing to be gained from using this analysis to add derivedFrom attributes to individual files, at least for those from STMicro.

  2. Across all 52 of those files, there are 1923 peripheral declarations without a derivedFrom attribute. According to this tool, 429 of those peripherals are actually distinct. So there's a lot of duplication we can avoid across the STM32 product line.

I think that leaves us at the "decide how to deal with equivalent peripherals across different SVD files in svd2rust" step (unless I missed some questions you still want answers to from earlier steps). I don't have a clear idea how to answer that question, so I'm hoping somebody else does. 😅

jameysharp avatar Jul 12 '17 19:07 jameysharp

Thanks @jameysharp. That's pretty insightful.

Across all 52 of those files, there are 1923 peripheral declarations without a derivedFrom attribute. According to this tool, 429 of those peripherals are actually distinct.

429 is much more higher than I was hoping for; I was actually hoping for a number smaller than 100. It may be worth to take a close look into the peripherals that are supossedly different but have the same names (e.g. stm32f103xx::GPIOA vs stm32f102xx::GPIOA) to see if they are actually different or simply appear to be different due to how the SVD files are structured.

I forked share-svd to make it report the clusters of equivalent peripherals across files. I ran the fork on all the stm32 SVD files and also an all the silab SVD files. Interestingly the silabs SVD files seem much more homogenous: across 1802 peripherals from 54 files there were only 55 unique peripherals. cc @ryankurte

In any case it seems like it would be easier to experiment with the "common crate + re-exports" idea using the silabs SVD file database.

japaric avatar Jul 13 '17 00:07 japaric

BTW, the silabs SVD files are here.

japaric avatar Jul 13 '17 00:07 japaric

Awesome! I just ran the same test for the silabs SVDs, and have just updated the repo with a pile more SVDs / automation to make adding further packages easier.

A couple of thoughts:

  • I would be interested to know whether it's feasible to have sub/supersets of these common peripherals
  • I think there must be an error, or at least opportunities for optimisation if there are that many peripheral definitions across stm32 cores, perhaps the derivedFrom origins are not correctly compared?
  • I think any hand modification of SVD files will make it difficult to maintain. Ideally the addition of a processor should be basically autonomous, and the creation / management of these base repos should be too.
    • But we might also be able to do a better job with hand defined base files, I guess that could be tested once there is a working mechanism.
  • Should we be thinking about another layer so it's vendor -> family -> device?

Maybe some kind of pipeline would work / allow each of the tasks to be split effectively.

  1. Process all SVDs to extract common peripherals into a base SVD
  2. Process all SVDs to output optimised (ie. using derivedFrom) device SVD files using references from the base file
  3. Generate rust2svd bindings for the base SVD
  4. Generate rust2svd bindings for optimised device SVDs using components exposed from the base SVD

ryankurte avatar Jul 13 '17 02:07 ryankurte

@japaric I have a bit of a mess going on but the latest copies of things are here (or SVDs here) because I hadn't worked out whether / how to split things yet. Might hide the other repositories for now.

ryankurte avatar Jul 13 '17 02:07 ryankurte

Thanks @japaric, the cluster reporting is definitely an improvement... although it didn't compile as-is for me, because the map's keys don't implement Debug. So I've pushed to my repo a version that does compile, and changed the output so I can use tools like sort -k2 or awk '$1>2' to answer more questions.

I spot-checked several peripherals that had the same name but different definitions across the STM32 line, and found enough cases of fundamentally different fields that I think these results are more or less accurate.

That said, using the cluster reporting, I can see that only 274 peripherals are shared across at least two members of the STM32 product line, so that's a little more reasonable than 429.

But, for example, the STM32F4 family has a pretty reasonable set of shared peripherals: There are only 72 distinct peripherals used by at least two devices in the F4 family. So maybe @ryankurte's question about "another layer so it's vendor -> family -> device" is important here?

Except there are also peripherals that cross a couple of families. For example: CAN has one version for F4 and F7, another for F2, another for F0 and F1, and two variants for F3. In the last case, one variant has a bunch of filter-bank registers that the other one doesn't, so they are legitimately different, although one is a strict subset of the other.

I'm still not sure where this is going, but I hope the additional data helps somehow. 😄

jameysharp avatar Jul 13 '17 06:07 jameysharp

Number 429 comes up only when you run script against ALL SVD files for STM32. Which includes STM32W108 with a lot of unique peripherals. Also there is RCC, which is different in almost all devices but is used mainly for initialization. So the situation is not as bad as first looks.

I will try to analyze results for peripherals that are most useful for HAL layer. Maybe they can be unified even more. For example in STM32F1 family UART4 different from UART5 only by DMA support (and only in high-density devices).

protomors avatar Jul 13 '17 07:07 protomors

@ryankurte

I would be interested to know whether it's feasible to have sub/supersets of these common peripherals

I'd say it's possible but I'm not sure if it can be done automatically. A tool can check if two peripherals, or even registers, have a structural superset - subset relationship, but that may not imply a semantic superset - subset relationship.

I guess it depends on how you define the subset relationship; something like "a peripheral is a subset of another if it contains more registers, while preserving the memory layout, and all the common registers are 'equal'" seems like should be semantics preserving as in you can treat the subset as if it were the superspet (Subset: AsRef<Superset>).

OTOH, I don't think a subset relationship at the register level is semantics preserving. For instance, the superset register has bits 0:1 reserved and the subset registers assign some meaning to bits 0:1 but both have the same reset values. Treating the subset register as if it were the superset (e.g. using a generic function) and then writing, say, the reset value to it may cause a different effect have different meaning if the modified register was actually the subset register.

It also worries me that we are not including <enumeratedValues> information in the equality analysis. If we are not considering that information then we may be consider two registers equal if the have the same bitfield layout even though one register assigns meaning to some bit pattern while the other may consider the bit pattern as reserved.

perhaps the derivedFrom origins are not correctly compared?

Peripherals that contain <derivedFrom> information are not considered in the analysis. This under reports the number of peripherals that belongs to the same cluster, because of N peripherals that are equivalent due to <derivedFrom> within a file only one is counted, but it doesn't affect the number of clusters found across several files.

Ideally the addition of a processor should be basically autonomous

Yeah, ideally but the quality of SVD files is not uniform. For instance, some older files don't include <enumeratedValues> information, so we have been adding this information to some older files.

Should we be thinking about another layer so it's vendor -> family -> device?

Yes. The cluster analysis should tell us where the commonalities are. It should not only confirm that some peripherals are shared across the same device family (e.g. all the F4 devices) but it should also reveal if some peripherals are shared across device families.

Process all SVDs to extract common peripherals into a base SVD

This assumes that all those SVDs have at least one peripheral which is equal across all the files. It could be the case that from N files a single peripheral may have 2 or more variants.

@jameysharp thanks for taking a closer look

For example: CAN has one version for F4 and F7, another for F2, another for F0 and F1, and two variants for F3.

This is what I was referring to. We have to capture these variants into some structure. Maybe something like this?

// stm32 crate
pub mod f0 {
    pub struct CAN { .. }
}

pub mod f1 {
    pub use ::f4::CAN;
}

pub mod f2 {
    pub struct CAN { .. }
}

pub mod f3 {
    pub struct CAN1 { .. }
    pub struct CAN2 { .. }
}

pub mod f4 {
    pub struct CAN { .. }
}

pub mod f7 {
    pub use ::f4::CAN;
}
// stm32f103xx crate
pub use stm32::f1::CAN;

@protomors

I will try to analyze results for peripherals that are most useful for HAL layer.

Thanks, that would be great. I agree that we should focus on peripherals that will be directly involved in doing I/O, which is what embedded-hal covers. Configuration related peripherals, like RCC, seem to have less commonalities so there seems to be less to win in that space.

japaric avatar Jul 13 '17 20:07 japaric

Have also been playing with @jameysharp's share-svd also and adapted it to look across device families / output which peripherals are duplicated.

Also in thinking about it more I realised that all the latest silabs cores are basically the same architecture, so running across them:

Silabs peripherals for new cores

barry:share-svd ryan$ ./target/release/share-svd -i /Volumes/ram
Discovered families: ["EFM32JG12B", "EFM32JG1B", "EFM32PG12B", "EFM32PG1B", 
"EFR32BG12P", "EFR32FG12P", "EFR32MG12P"]
Loading
......................................................
Loaded 54 devices from 7 families in 14.03 seconds
Analysing peripherals
Family: EFM32PG1B Peripheral count: 22 unique (150 total)
Family: EFM32JG1B Peripheral count: 21 unique (144 total)
Family: EFM32PG12B Peripheral count: 29 unique (240 total)
Family: EFM32JG12B Peripheral count: 28 unique (234 total)
Family: EFR32FG12P Peripheral count: 28 unique (400 total)
Family: EFR32BG12P Peripheral count: 28 unique (400 total)
Family: EFR32MG12P Peripheral count: 28 unique (400 total)
Analysis complete
Overall peripherals: 40 unique (184 total)
Peripheral Overview:
    - ACMP0		[("EFM32PG1B100F128GM32", 39), ("EFM32PG12B500F1024GL125", 102)]
    - ADC0		[("EFM32PG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 54)]
    - CMU		[("EFM32PG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 54)]
    - CRYOTIMER		[("EFM32PG1B100F128GM32", 68)]
    - CRYPTO		[("EFM32PG1B100F128GM32", 21)]
    - CRYPTO0		[("EFM32PG12B500F1024GL125", 102)]
    - CSEN		[("EFM32PG12B500F1024GL125", 48), ("EFM32PG12B500F512GL125", 9)]
    - EMU		[("EFM32PG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 54)]
    - ETM		[("EFM32PG12B500F1024GL125", 54)]
    - FPUEH		[("EFM32PG1B100F128GM32", 54)]
    - GPCRC		[("EFM32PG1B100F128GM32", 68)]
    - GPIO		[("EFM32PG12B500F1024GL125", 54), ("EFM32PG1B100F128GM32", 21)]
    - I2C0		[("EFM32PG1B100F128GM32", 110)]
    - IDAC0		[("EFM32PG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 54)]
    - LDMA		[("EFM32PG1B100F128GM32", 68)]
    - LESENSE		[("EFM32PG12B500F1024GL125", 54)]
    - LETIMER0		[("EFM32PG1B100F128GM32", 68)]
    - LEUART0		[("EFM32PG1B100F128GM32", 68)]
    - MSC		[("EFM32PG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 54)]
    - PCNT0		[("EFM32PG1B100F128GM32", 152)]
    - PRS		[("EFM32PG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 54)]
    - RMU		[("EFM32PG12B500F1024GL125", 54), ("EFM32PG1B100F128GM32", 21)]
    - RTCC		[("EFM32PG1B100F128GM32", 68)]
    - SMU		[("EFM32PG12B500F1024GL125", 54)]
    - TIMER0		[("EFM32PG1B100F128GM32", 39), ("EFM32PG12B500F1024GL125", 198)]
    - TRNG0		[("EFM32PG12B500F1024GL125", 54)]
    - USART0		[("EFM32PG1B100F128GM32", 212)]
    - VDAC0		[("EFM32PG12B500F1024GL125", 54)]
    - WDOG0		[("EFM32PG1B100F128GM32", 110)]

Silabs peripherals for all cores

share-svd across all silabs cores

barry:share-svd ryan$ ./target/release/share-svd -i /Volumes/ram
Discovered families: ["EFM32GGxxx", "EFM32Gxxx", "EFM32JG12B", "EFM32JG1B", "EFM32PG12B", "EFM32PG1B", "EFM32WGxxx", "EFR32BG12P", "EFR32FG12P", "EFR32MG12P", "EZR32HG", "EZR32WG"]
Loading
......................................................................................................................................................................................................................................................................
Loaded 262 devices from 12 families in 47.60 seconds
Analysing peripherals
Family: EFM32JG1B Peripheral count: 21 unique (144 total)
Family: EFM32JG12B Peripheral count: 28 unique (234 total)
Family: EFM32PG1B Peripheral count: 22 unique (150 total)
Family: EFM32PG12B Peripheral count: 29 unique (240 total)
Family: EFM32Gxxx Peripheral count: 28 unique (903 total)
Family: EFR32BG12P Peripheral count: 28 unique (400 total)
Family: EFR32FG12P Peripheral count: 28 unique (400 total)
Family: EFM32GGxxx Peripheral count: 33 unique (1508 total)
Family: EFR32MG12P Peripheral count: 28 unique (400 total)
Family: EZR32HG Peripheral count: 21 unique (630 total)
Family: EFM32WGxxx Peripheral count: 35 unique (2358 total)
Family: EZR32WG Peripheral count: 26 unique (1533 total)
Analysis complete
Overall peripherals: 135 unique (327 total)
Peripheral Overview:
    - ACMP0		[("EFM32JG1B100F128GM32", 39), ("EFM32JG12B500F1024GL125", 102), ("EFM32G200F16", 130), ("EFM32GG230F1024", 384)]
    - ADC0		[("EFM32G200F16", 66), ("EFM32GG230F1024", 86), ("EFM32WG230F128", 173), ("EZR32HG220F32R55", 58), ("EFM32JG1B100F128GM32", 21), ("EFM32JG12B500F1024GL125", 54)]
    - AES		[("EFM32G210F128", 60), ("EZR32HG220F32R55", 58), ("EFM32GG230F1024", 194)]
    - BURTC		[("EFM32GG230F1024", 194)]
    - CMU		[("EFM32GG230F1024", 10), ("EFM32GG840F1024", 10), ("EFM32GG280F1024", 14), ("EFM32GG880F1024", 14), ("EFM32GG330F1024", 10), ("EFM32GG940F1024", 10), ("EFM32GG380F1024", 14), ("EFM32GG900F1024", 18), ("EFM32G200F16", 8), ("EFM32G210F128", 4), ("EFM32G222F128", 8), ("EFM32G230F128", 14), ("EFM32G840F128", 14), ("EFM32G280F128", 14), ("EFM32G800F128", 16), ("EZR32HG220F32R55", 30), ("EZR32HG320F32R55", 30), ("EFM32WG230F128", 14), ("EFM32WG840F128", 14), ("EZR32WG230F128R55", 44), ("EFM32WG280F128", 20), ("EFM32WG880F128", 20), ("EFM32WG330F128", 14), ("EFM32WG940F128", 14), ("EFM32WG360F128", 8), ("EZR32WG330F128R55", 44), ("EFM32WG380F128", 20), ("EFM32WG900F256", 22), ("EFM32JG1B100F128GM32", 21), ("EFM32JG12B500F1024GL125", 54)]
    - CRYOTIMER		[("EFM32JG1B100F128GM32", 68)]
    - CRYPTO		[("EFM32JG1B100F128GM32", 21)]
    - CRYPTO0		[("EFM32JG12B500F1024GL125", 102)]
    - CSEN		[("EFM32JG12B500F1024GL125", 48), ("EFM32JG12B500F512GL125", 9)]
    - DAC0		[("EFM32G200F16", 66), ("EFM32GG230F1024", 194)]
    - DMA		[("EZR32HG220F32R55", 58), ("EFM32G200F16", 66), ("EFM32GG230F1024", 194)]
    - EBI		[("EFM32G280F128", 28), ("EFM32GG280F1024", 92)]
    - EMU		[("EFM32JG1B100F128GM32", 21), ("EZR32HG220F32R55", 58), ("EZR32WG230F128R55", 86), ("EFM32WG230F128", 130), ("EFM32G200F16", 66), ("EFM32GG230F1024", 86), ("EFM32JG12B500F1024GL125", 54)]
    - ETM		[("EFM32JG12B500F1024GL125", 54), ("EFM32WG230F128", 173), ("EFM32GG230F1024", 86)]
    - FPUEH		[("EFM32WG230F128", 173), ("EFM32PG1B100F128GM32", 54)]
    - GPCRC		[("EFM32JG1B100F128GM32", 68)]
    - GPIO		[("EZR32HG220F32R55", 58), ("EFM32G200F16", 66), ("EFM32GG230F1024", 194), ("EFM32JG12B500F1024GL125", 54), ("EFM32JG1B100F128GM32", 21)]
    - I2C0		[("EFM32G200F16", 66), ("EFM32GG230F1024", 413), ("EFM32JG1B100F128GM32", 110)]
    - IDAC0		[("EZR32HG220F32R55", 58), ("EFM32JG1B100F128GM32", 21), ("EFM32JG12B500F1024GL125", 54)]
    - LCD		[("EFM32G800F128", 28), ("EFM32GG840F1024", 78)]
    - LDMA		[("EFM32JG1B100F128GM32", 68)]
    - LESENSE		[("EFM32GG230F1024", 194), ("EFM32JG12B500F1024GL125", 54)]
    - LETIMER0		[("EFM32JG1B100F128GM32", 68), ("EFM32G200F16", 66), ("EFM32GG230F1024", 194)]
    - LEUART0		[("EFM32G200F16", 116), ("EZR32HG220F32R55", 58), ("EFM32GG230F1024", 384), ("EFM32JG1B100F128GM32", 68)]
    - MSC		[("EFM32G200F16", 66), ("EZR32HG220F32R55", 58), ("EFM32WG230F128", 173), ("EFM32GG230F1024", 86), ("EFM32JG1B100F128GM32", 21), ("EFM32JG12B500F1024GL125", 54)]
    - MTB		[("EZR32HG220F32R55", 58)]
    - PCNT0		[("EFM32G200F16", 172), ("EFM32GG230F1024", 574), ("EFM32JG1B100F128GM32", 152), ("EZR32HG220F32R55", 58)]
    - PRS		[("EZR32HG220F32R55", 58), ("EFM32G200F16", 66), ("EFM32GG230F1024", 194), ("EFM32JG1B100F128GM32", 21), ("EFM32JG12B500F1024GL125", 54)]
    - RMU		[("EFM32G200F16", 66), ("EZR32HG220F32R55", 58), ("EFM32GG230F1024", 194), ("EFM32JG12B500F1024GL125", 54), ("EFM32JG1B100F128GM32", 21)]
    - RTC		[("EFM32G200F16", 66), ("EFM32GG230F1024", 223)]
    - RTCC		[("EFM32JG1B100F128GM32", 68)]
    - SMU		[("EFM32JG12B500F1024GL125", 54)]
    - TIMER0		[("EFM32G200F16", 186), ("EFM32GG230F1024", 764), ("EZR32HG220F32R55", 170), ("EFM32JG1B100F128GM32", 39), ("EFM32JG12B500F1024GL125", 198)]
    - TRNG0		[("EFM32JG12B500F1024GL125", 54)]
    - USART0		[("EFM32G200F16", 206), ("EFM32GG230F1024", 569), ("EFM32JG1B100F128GM32", 212), ("EFM32WG230F128", 546), ("EZR32HG220F32R55", 114)]
    - USB		[("EFM32WG330F128", 92), ("EFM32GG330F1024", 46), ("EZR32HG320F32R55", 30)]
    - VCMP		[("EFM32G200F16", 246)]
    - VDAC0		[("EFM32JG12B500F1024GL125", 54)]
    - WDOG		[("EFM32G200F16", 246)]
    - WDOG0		[("EFM32JG1B100F128GM32", 110)]

Which is rather interesting, for my use I will probably focus on the newer cores and not worry too much about the others for now.

ryankurte avatar Jul 15 '17 03:07 ryankurte

Had a go at adding enumeratedValues to peripherals and it makes rather a big difference (from 40 unique to 160 unique on the previous test case). The good news is that it looks like, at least with these devices, there is a sub/superset thing going on with the enumeratedValues on each peripheral.

Which makes a lot of sense from an architectural perspective, same architecture with different features available. I wonder if we could extract the common denominators (as with the peripherals), then add the extras in the per-device packages? Not sure if that achievable in SVD land or not, but that would put us in a position of at least being able to use the commonalities in a common hal. The disclaimer is that we have to handle both the sub/superset and the completely different cases, and I have no idea how to approach the latter.

@japaric so if we create that common structure like your example in rust, is there a useful way we can teach rust2svd to use matching structs from it, or would we be better to do that with common svd files?

Good point about multiple definitions in one file, I wonder whether we could parse a pile of svd files, extract / minimize any commonalities, then export a rewritten set of files and dependencies split by commonality as appropriate. It's getting a little bit complicated, but it seems like we're going to end up with crazy fragments of hal to match whatever is common across devices :-/

ryankurte avatar Jul 18 '17 00:07 ryankurte

Will try to share my findings so far. I decided to start from GPIO and it looks like that there is really only two types of it in all STM32 devices. First in oldest STM32F1 devices (like STM32F103) and another - everywhere else. And the most important part (registers IDR, ODR and BSRR) works the same. Only configurations is different, new GPIO has much more modes and different configuration register structure.

So it is probably possible to implement traits that will work with GPIO across all STM32 devices (or at least have only two varians).

In this discussion there was concern about reset values. About this - configuration registers for some ports have different reset values because JTAG/SWD is enabled by default and its pins configured accordingly. For me it is not a problem because I usually modify configuration registers instead of writing to them. But because of this all ports on device are not completely identical (GPIOA and GPIOB have different reset values).

There are also some specific cases when other peripheral takes over pins regardless of GPIO settings (like pins OSC_IN and OSC_OUT when HSE is on). But this cases and AFIO configuration are not in scope of HAL anyway. So I think GPIO is the easiest peripheral that we can try to unify.

protomors avatar Jul 21 '17 10:07 protomors

@ryankurte great additions to share-svd :+1:

@protomors's comment got me thinking. For some HALs / APIs we don't need to use all the registers in a peripheral. Maybe we don't need to use all the bit fields in, say, a status register either. So I'm wondering if we could use something like "common struct views" to achieve code reuse in HAL implementations. Basically given a set of peripherals from different devices we can "downcast" them into a struct that exposes only the common bits among them. With code it could look like this:

// crate: stm32_common
struct Gpio {
   // padding
   pub idr: IDR,
   pub odr: ODR,
   pub bsrr: BSRR,
   // padding
}

// crate: stm32f103xx
struct Gpio {
    // configuration registers
   pub idr: stm32_common::IDR,
   pub odr: stm32_common::ODR,
   pub bsrr: stm32_common::BSRR,
   // more registers
}

// the implementation is just a pointer conversion: &Gpio -> &stm32_common::Gpio
impl AsRef<stm32_common::Gpio> for Gpio { .. }

And for status registers where we may be able to ignore some of the bit fields it could look like this:

// crate: stm32_common
struct Usart1 {
    // padding
    pub sr: Sr,
    pub dr: Dr,
    // padding
}

impl Sr {
    fn read(&self) -> .. { .. }
    fn modify(&self, ..) { .. }
    // NOTE no `write` method so the reset value doesn't matter
}

// crate: stm32f103xx
struct Usart1 {
    // padding
    // NOTE different `Sr` type which has more bit fields than `stm32_common::Sr`
    pub sr: Sr,
    pub dr: stm32_common::Dr,
    // padding
}

impl AsRef<stm32_common::Usart1> for Usart1 { .. }

We could do some "layering" to get some reuse for configuration / initialization APIs: (this is basically what I sketched before)

// crate: stm32_common

// shared by (almost) all devices
struct Usart1 {
    // padding
    pub sr: Sr,
    pub dr: Dr,
    // padding
}

// shared by (almost) all the F1 devices
pub mod f1 {
    pub struct Usart1 {
        pub cr1: Cr1,
        // ..
        pub sr: Sr,
        pub dr: Dr,
        // ..
    }

    impl AsRef<super::Usart1> for Usart1 { .. }
}

pub mod f2 {
    // ..
}

// crate: stm32f103xx
pub use stm32_common::f1::Usart1;

// crate: stm32f70xx
// super duper specific peripheral
pub struct Usart1 { .. }

impl AsRef<stm32_common::f7::Usart1> for Usart1 { .. }
impl AsRef<stm32_common::Usart1> for Usart1 { .. }

Here AsRef lets us "downcast" one peripheral into more than one "view". Deref would constraint us to a single view.

Anyone up for tweaking share-svd to find commonalities at the register level between similar looking peripherals?


so if we create that common structure like your example in rust, is there a useful way we can teach rust2svd to use matching structs from it, or would we be better to do that with common svd files?

I think it may be worthwhile to architecture this into two phases: one tool that collects commonalities across several SVD files into a single common SVD file. Then svd2rust would learn to generate a crate from a SVD and the common SVD file as depicted above. So svd2rust --common stm32_common.svd -i stm32f103xx.svd would generate something like I've shown above, and svd2rust -i stm32_common.svd would generate the common stm32_common crate. However, I think this approach wouldn't support creating modules like f1 in the stm32_common crate because those modules can't be expressed in a SVD file. So this may only work for the single common view case.

The other approach is to teach svd2rust to deal with different files and have it automatically create the common crate. In that case svd2rust -i A.svd -i B.svd (..) would create a bunch of device crates and the common crate in the current directory. My issues with this approach is maintainability: it will likely involve maintaining a bunch of crates in a single repo which may make releases tricky (they'll all have be coordinated), but maybe that can be automated.

japaric avatar Jul 21 '17 21:07 japaric

The downcasting approach looks neat (though I must admit more complex than I had envisaged coming from other languages), I was thinking of implementing std::cmp::ord for sub/super sets of peripherals/registers/enumerations/etc. in svd-parser to help with the identification/extraction of commonalities, but haven't got there yet.

I think I like the two phase approach too, a tool that extracts commonalities and rewrites SVDs into the most useful form for processing, then a processing stage that maps that to rust. That means if we need it for odd devices we can insert other stages as required, and if we need to hand extract things or use different tools it's compartmentalised from svd2rust.

To approach the modules problem, what if svd2rust accepted an ordered / prioritised list of common files? So svd2rust --common stm32_f1.svd,stm32_common.svd -i stm32f103xx.svd would preferentially use matches from f1 and fall back to common if not found?

ryankurte avatar Jul 22 '17 02:07 ryankurte

@japaric Something like this I had in mind when proposed #122. I just thought that we will have to write base and device SVD files by hand for it to work. But if it is possible to find similarities in peripherals automatically - this is even better. For me it looks like the best approach. If I for example write library for interfacing with LCD screen and make it use API from stm32_common can this library be used with any device crate? Main application just will have to have to initialize GPIO and point my library to correct ports. Will this work? If I am understanding right main application and library will use the same memory location but will see it as different types.

As for USART - it is the next peripheral I am trying to compare. And it looks like that basic functionality (setting baud rate, transmitting, receiving, main status flags) works the same across all devices. There are only some minor differences in configuration. For example STMF2 has support for oversampling that STMF1 does not have. But this does not cause any conflicts because corresponding bits in STMF1 are declared as reserved. ST almost always takes this approach - expanding functionality of peripherals. GPIO is one of few examples where configuration registers are incompatible.

As for USART - it is the next peripheral I am trying to compare. And it looks like that basic functionality (setting baud rate, transmitting, receiving, main status flags) works the same across all devices. There are only some minor differences in configuration. For example STMF2 has support for oversampling that STMF1 does not have. But this does not cause any conflicts because corresponding bits in STMF1 are declared as reserved. ST almost always takes this approach - expanding functionality of peripherals. GPIO is one of few examples where configuration registers are incompatible.

Also there are different types of USART in the same device. Some of them are full featured and others lack such things like hardware flow control, synchronous mode etc. But basic TX/RX functionality looks the same. So extraction of common parts should provide a usable API.

@ryankurte Are you close to automatic extraction of similarities? Because the next thing I was going to do is to compare USART description in SVD files for different devices to confirm my thoughts. But doing it by hand will be tedious.

protomors avatar Jul 22 '17 07:07 protomors

@protomors I have had a play at getting share-svd to extract common registers, started trying to work out dependencies but it was simpler to extract common bits and diff the rest. It's in my fork.

It's only outputting information at the moment, and I haven't worked out how to structure the dependency map / regenerate SVGs from it.

share-svd with common register extraction

Discovered families: ["EFM32JG12B", "EFM32JG1B", "EFM32PG12B", "EFM32PG1B"]
Loading
........................
Loaded 24 devices from 4 families in 4.87 seconds
Analysing peripherals
Family: EFM32JG1B Peripheral count: 21 unique (144 total)
Family: EFM32PG1B Peripheral count: 22 unique (150 total)
Family: EFM32PG12B Peripheral count: 29 unique (240 total)
Family: EFM32JG12B Peripheral count: 28 unique (234 total)
Analysis complete
Overall peripherals: 40 unique (100 total)
Peripheral Overview:
    - ACMP0             [("EFM32JG1B100F128GM32", 39), ("EFM32PG12B500F1024GL125", 39)]
                                Common: 13 regs, diffs: [(EFM32JG1B100F128GM32: 0 regs) (EFM32PG12B500F1024GL125: 1 regs) ]
    - ADC0              [("EFM32JG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 21)]
                                Common: 30 regs, diffs: [(EFM32JG1B100F128GM32: 0 regs) (EFM32PG12B500F1024GL125: 0 regs) ]
    - CMU               [("EFM32JG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 21)]
                                Common: 44 regs, diffs: [(EFM32JG1B100F128GM32: 1 regs) (EFM32PG12B500F1024GL125: 4 regs) ]
    - CRYOTIMER         [("EFM32JG1B100F128GM32", 35)]
    - CRYPTO            [("EFM32JG1B100F128GM32", 21)]
    - CRYPTO0           [("EFM32PG12B500F1024GL125", 39)]
    - CSEN              [("EFM32PG12B500F1024GL125", 15), ("EFM32PG12B500F512GL125", 9)]
                                Common: 23 regs, diffs: [(EFM32PG12B500F1024GL125: 0 regs) (EFM32PG12B500F512GL125: 0 regs) ]
    - EMU               [("EFM32JG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 21)]
                                Common: 28 regs, diffs: [(EFM32JG1B100F128GM32: 2 regs) (EFM32PG12B500F1024GL125: 7 regs) ]
    - ETM               [("EFM32PG12B500F1024GL125", 21)]
    - FPUEH             [("EFM32PG1B100F128GM32", 21)]
    - GPCRC             [("EFM32JG1B100F128GM32", 35)]
    - GPIO              [("EFM32PG12B500F1024GL125", 21), ("EFM32JG1B100F128GM32", 21)]
                                Common: 112 regs, diffs: [(EFM32PG12B500F1024GL125: 1 regs) (EFM32JG1B100F128GM32: 0 regs) ]
    - I2C0              [("EFM32JG1B100F128GM32", 47)]
    - IDAC0             [("EFM32JG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 21)]
                                Common: 10 regs, diffs: [(EFM32JG1B100F128GM32: 0 regs) (EFM32PG12B500F1024GL125: 0 regs) ]
    - LDMA              [("EFM32JG1B100F128GM32", 35)]
    - LESENSE           [("EFM32PG12B500F1024GL125", 21)]
    - LETIMER0          [("EFM32JG1B100F128GM32", 35)]
    - LEUART0           [("EFM32JG1B100F128GM32", 35)]
    - MSC               [("EFM32JG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 21)]
                                Common: 18 regs, diffs: [(EFM32JG1B100F128GM32: 0 regs) (EFM32PG12B500F1024GL125: 5 regs) ]
    - PCNT0             [("EFM32JG1B100F128GM32", 59)]
    - PRS               [("EFM32JG1B100F128GM32", 21), ("EFM32PG12B500F1024GL125", 21)]
                                Common: 22 regs, diffs: [(EFM32JG1B100F128GM32: 0 regs) (EFM32PG12B500F1024GL125: 0 regs) ]
    - RMU               [("EFM32PG12B500F1024GL125", 21), ("EFM32JG1B100F128GM32", 21)]
                                Common: 5 regs, diffs: [(EFM32PG12B500F1024GL125: 0 regs) (EFM32JG1B100F128GM32: 0 regs) ]
    - RTCC              [("EFM32JG1B100F128GM32", 35)]
    - SMU               [("EFM32PG12B500F1024GL125", 21)]
    - TIMER0            [("EFM32JG1B100F128GM32", 39), ("EFM32PG12B500F1024GL125", 75)]
                                Common: 37 regs, diffs: [(EFM32JG1B100F128GM32: 0 regs) (EFM32PG12B500F1024GL125: 0 regs) ]
    - TRNG0             [("EFM32PG12B500F1024GL125", 21)]
    - USART0            [("EFM32JG1B100F128GM32", 89)]
    - VDAC0             [("EFM32PG12B500F1024GL125", 21)]
    - WDOG0             [("EFM32JG1B100F128GM32", 47)]
barry:share-svd ryan$


Also thinking it would be a great idea for us to create/extract a bunch of examples from some SVDs so we can put together a useful set of test inputs and outputs, because as you said it is otherwise very tedious.

ryankurte avatar Jul 28 '17 00:07 ryankurte

Hey, have been working on japaric/svd#37 towards being able to re-encode SVGs so that we can output these new-fangled compressed/optimised/standardised files.

Still a work in progress, but, would appreciate any thoughts you might have on the approach / PR?

ryankurte avatar Aug 22 '17 11:08 ryankurte

Hi everyone! I missed this conversation at the time but coincidentally was working on something similar and would be interested in pushing this issue forward.

I downloaded the official ST SVD files for all the STM32 devices (here) and wrote a bunch of scripts to parse and compare them initially. One main result is this big set of tables. For a representative member of each STM32F family (I did the same for L and H too), you can see which unique peripherals there are and which devices have them, and then drill down to all the chips in that family, and to each register and field and bitfield in each as well. For example, here you can see all STM32F7s have the same CRC peripheral at the same address, but for some reason two of them shift the INIT and POL register addresses.

Overall there are a lot of peripherals that are common or have common supersets. One really helpful way to group them is looking at the ChibiOS STM32 HAL organisation, e.g. it has USARTv1 and USARTv2 and you can see which chips use which drivers.

My attempt at making these at least usable by e.g. macros in drivers (so they'd work with same-named registers even if not the actual same type) was to created 'patched' SVD files which automatically fill in and match up details from shared peripherals, fully annotated with descriptions and enums etc. That's what's in this repo; in peripherals/ there are files for each shared peripheral design in a yaml format that applies patches to the SVD entries, and in devices/ there is a file for each STM32 specifying which SVD file to start from and which peripherals to apply and any per-file fixes as well (there is no shortage of typos or inconsistencies in ST's SVDs). Then a Makefile generates all the patched SVDs and runs svd2rust against them, creating a crate for each family with a cfg-gated library for each chip. There's no shared traits between the chips, but they are as consistent as possible given the hardware.

For relatively complete examples, check out the STM32F405 -- it has a couple of typos to correct, then pulls in a bunch of common and more-specific peripheral files. For example, dac_common_2ch, which includes dac_common_1ch, and adds some extra fields relating to having a second channel. Because all the fields are fully documented, the output of svd2rust is as useful as possible (including the result of building documentation against the source).

I'm not sure this is the best approach to take -- it was just an experiment at the time and I'm not using it today. Honestly I wonder if the best approach is actually hand-written traits and libraries (plus generous helpings of automation), perhaps using svd2rust, perhaps not. It would be so nice to have a register-level HAL that worked on all the STM32s, the equivalent of the good C header files being available from ST, both for people who prefer coding against registers directly instead of using higher level abstractions, and for building drivers etc on top of.

adamgreig avatar Mar 13 '18 23:03 adamgreig