Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Common traits for identical IP blocks #96

Open
jameysharp opened this issue May 24, 2017 · 30 comments
Open

Common traits for identical IP blocks #96

jameysharp opened this issue May 24, 2017 · 30 comments

Comments

@jameysharp
Copy link

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?

@japaric
Copy link
Member

japaric commented May 24, 2017

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.

@jameysharp
Copy link
Author

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. 😁

@idubrov
Copy link
Contributor

idubrov commented May 24, 2017

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.

@japaric
Copy link
Member

japaric commented May 24, 2017

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.

@idubrov
Copy link
Contributor

idubrov commented May 24, 2017

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.

@japaric
Copy link
Member

japaric commented May 24, 2017

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).

@jameysharp
Copy link
Author

@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?

@japaric
Copy link
Member

japaric commented May 24, 2017

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.

@jameysharp
Copy link
Author

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.

@japaric
Copy link
Member

japaric commented May 25, 2017

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.

@jameysharp
Copy link
Author

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?

@japaric
Copy link
Member

japaric commented Jul 12, 2017

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
    Device family HALs in rust wg#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
      Device family HALs in rust wg#33 (comment) 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 #122 (comment)). 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>

@protomors
Copy link

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).

@jameysharp
Copy link
Author

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. 😅

@japaric
Copy link
Member

japaric commented Jul 13, 2017

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
Copy link
Member

japaric commented Jul 13, 2017

BTW, the silabs SVD files are here.

@ryankurte
Copy link

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
Copy link

ryankurte commented Jul 13, 2017

@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.

@jameysharp
Copy link
Author

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. 😄

@protomors
Copy link

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).

@japaric
Copy link
Member

japaric commented Jul 13, 2017

@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.

@ryankurte
Copy link

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
Copy link

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 :-/

@protomors
Copy link

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.

@japaric
Copy link
Member

japaric commented Jul 21, 2017

@ryankurte great additions to share-svd 👍

@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.

@ryankurte
Copy link

ryankurte commented Jul 22, 2017

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?

@protomors
Copy link

@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.

@ryankurte
Copy link

ryankurte commented Jul 28, 2017

@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
Copy link

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?

@adamgreig
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants