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

PACs based on Internal Peripheral versions #4

Open
teskje opened this issue Sep 5, 2020 · 11 comments
Open

PACs based on Internal Peripheral versions #4

teskje opened this issue Sep 5, 2020 · 11 comments

Comments

@teskje
Copy link

teskje commented Sep 5, 2020

TLDR: The current approach of basing PACs on ST-provided SVD files is error-prone and I think we can do better by hand-writing register definitions for the different Internal Peripheral versions instead.

The Issue

In my (limited) experience with the stm32 PACs I've noticed that sometimes the peripherals defined in the SVD files (and therefore the PACs generated from these SVDs) don't quite fit the peripherals actually present on the MCUs. For example:

The root of the problem is that the SVDs are too coarse-grained: Usually one SVD describes a whole MCU family (e.g. stm32f303) or even multiple families (e.g. stm32f3x8). Usually the MCUs inside a family have quite a lot in common, but there are still differences in the included peripherals, as evident in the two issues mentioned above.

Currently, we "solve" this issue by adding missing registers and fields in every case, so the most feature-full MCUs inside each family are fully supported by their respective PACs. This also means that for MCUs with fewer features the PACs allow access to registers that don't actually exist on these MCUs. This can easily lead to bugs, for example if HAL maintainers don't carefully read the RMs for all involved devices and rely on what's provided by the PACs instead. In the spirit of having a "pit of success" I'd like to discuss if we can make this situation better somehow.

Note that I have been primarily looking at GPIO peripherals. I assume the same issues also manifest with other peripherals, but I can't say for sure. I assume that if we find a good solution for GPIO peripherals, this will hopefully be generic enough to be applicable to other peripherals too.

A Possible Solution

It seems like SVDs are not an ideal source of information to base the PACs on. In addition to being too coarse-grained, they are also full of bugs. Unfortunately, we don't have anything better if we want to auto-generate the PAC code. For simply retrieving the necessary information the reference manuals are fine, but it is not realistic to parse their PDFs to automatically extract this information.

What if we hand-write the PACs? By that I don't mean we should hand-write the Rust code that makes up the PACs directly of course. Rather we can manually collect the information from the RMs into some register definition format and then use that as a source for generating the PACs. There is some risk of human error while transcribing the register definitions by hand, but I don't think we would end up with more errors than we have now with the SVDs.

The second question we need to answer is: Which level of granularity do we need? If MCU families are too coarse-grained, do we need to have a different PAC for each MCU model instead? This is where the Internal Peripheral (IP) versions enter the game. IPs are something ST seems to use internally for there own tooling, such as the STM32CubeMX tool. Extracting them is not too difficult since the CubeMX database consists of a bunch of well-structured XML files which are easily parsed (see the cube-parse project). The number of different IP versions is way less than the number of MCU models. For example, between all the STM32F3 devices, there are only five different GPIO IPs found in the CubeMX database.

So my proposed solution approach to creating more accurate PACs is:

  1. Inspect the CubeMX database to identify the relevant IP versions
  2. Inspect the RMs to extract the register and field definitions of the different IPs
  3. Manually convert these definitions into some DSL our code-generation tools can work with
  4. Use code generation to create the final Rust code that makes up the PACs

To persuade myself that this approach can work in principle, I created a proof-of-concept that implements it for the STM32F3 GPIO peripherals: stm32f3-gpio. The register definition DSL is simply Python code and the codegen simply creates SVDs and then uses svd2rust to create the PAC code. But it works and was relatively easy to write.

If we follow the approach I just described, we end up with different PACs for different peripherals. We can wrap those inside the existing PAC crates so consumers don't need to collect all required peripherals manually and can continue to simply import, e.g., the stm32f3 crate, though they'd have to pass the correct features depending on the MCU they want to build for.


I realize there is a lot to unpack here and the change I am proposing is quite fundamental. I'd just like to start the discussion here to see what other people think about the issue and what better solutions might be out there.

@adamgreig
Copy link
Member

Thanks for writing up your thoughts on this! I think it's an important issue and should be the main direction for any big change in how we generate PACs.

Part of the problem is that today svd2rust can only take a single SVD and produce a single crate (or module, for stm32-rs). That means that right now if we did have an SVD for every device variant we'd have even larger crates, and they're already quite sizeable. That's why so far there hasn't been too much worry about SVDs defining peripherals that don't exist on every variant of chip in a family. More on this later.

Ultimately I think that your idea of using well-written shared IPs and then defining a chip as made up of a list of specific IPs is ideal; a lot of those IPs are shared between families too, and if we could define the IPs by their common subsets and the small differences between them it would lead to much better consistency for users and HAL authors. Looking just at GPIOs is probably reasonable representative, but I think actually GPIO and RCC peripherals have the most variation between devices, and many other IPs are identical or much more similar across entire families.

It seems like SVDs are not an ideal source of information to base the PACs on. In addition to being too coarse-grained, they are also full of bugs.

Do you mean specifically ST's SVDs? It seems like your solution essentially ends up as "let's write our own better SVDs"; the SVD format is comprehensive and should fill our needs for describing peripherals, just we wouldn't want to hand-write the XML.

it is not realistic to parse their PDFs to automatically extract this information.

I've heard that a few people have had some measure of success with that approach, perhaps combined with manual oversight and checking. It is a huge amount of work to attempt to hand-write by collecting information from the RMs. Between the common_patches and peripherals folders in stm32-rs, we already have over 25kloc of YAML, and that's just for fixing mistakes, let alone redefining everything from scratch. We've got 69 devices containing over 3500 peripheral elements in total. Even knowing about which IPs are shared between devices, I don't think it's realistic to consider hand writing all that, or to expect it to be less error-prone or much more consistent.

do we need to have a different PAC for each MCU model instead?

I think this would need some careful work with svd2rust to avoid creating a huge number of PACs and pushing our crate files even larger; I think we should be able to instead keep one PAC per family but perhaps have more fine-grained features which select specific shared IPs, which should make crates smaller than they are currently.

Manually convert these definitions into some DSL

In principle, we could even use the existing YAML DSL, which would allow easier piecemeal migration to hand-written peripherals.


On a slightly higher level I was trying to solve many of these problems with stm32ral, which parses every SVD file together, then automatically refactors out all the shared peripherals within a family. It does so by factoring fields, registers, and peripherals which are identical to the level of names and bit widths and positions, with some heuristics to generate combined names. It's far from perfect but it does quite a good job of factoring out the many common peripherals already; perhaps a lower-effort solution would be a little more work patching ST's SVDs to improve consistency and then spending time on svd2rust to enable it to do similar refactoring to allow multiple devices in one PAC.

@teskje
Copy link
Author

teskje commented Sep 11, 2020

Do you mean specifically ST's SVDs? It seems like your solution essentially ends up as "let's write our own better SVDs"; the SVD format is comprehensive and should fill our needs for describing peripherals, just we wouldn't want to hand-write the XML.

Yes, sorry that was worded badly. The issue lies with the quality/correctness of ST's SVDs, not the SVD format itself. If other vendors provide SVDs that accurately reflect the MCU peripherals, there is no problem in the first place. And I agree that the SVD format appears to have everything needed to describe the information required for PAC generation.

I've heard that a few people have had some measure of success with that approach, perhaps combined with manual oversight and checking.

Interesting, I'd like to know more about that. PDF as a format is difficult to work with in an automated manner, because it has grown to include every feature under the sun and is made to be human-, not machine-, readable. But I guess all RMs are similarly structured, so parsing the PDF format of those in particular might not be such a big issue. A bigger issue will be that in my experience the information presented in the RMs is sometimes presented in a very unstructured way. Consider for example the GPIOx_LCKR register definitions in the F303's RM:

11.4.8 GPIO port configuration lock register (GPIOx_LCKR)

x= A, B and D in STM32F303xB/C and STM32F358xC devices, x= A, B, C, D and F in
STM32F303x6/8 and STM32F328x8 devices and x = A, B, C, D, E, F, G, H in
STM32F303xD/E devices.

Automatically parsing something like that would be hard and full of edge-cases. The issue is, again, that the RMs are per-subfamily, while the registers are better described in terms of peripheral IPs. In this example you can see that there are three different GPIO IPs involved, and describing all of them at the same time gets confusing fast.

I'm also not convinced that the RMs are free of errors. As a human reader you have a chance of detecting them, e.g. if a reset value just doesn't make sense logically. Automatically detecting errors would be hard I imagine.

Using some automated parsing in tandem with human review might work, but I'm feeling that the effort of writing all that tooling and performing that manual checking might still be worse than just writing the register definitions by hand from the start. I see that you are skeptical, though, and you are probably right that I vastly underestimate the effort required. Do you think it would be worth extending my GPIO PoC or doing a different one to get a more concrete picture of how much work hand-writing everything would be?

In principle, we could even use the existing YAML DSL, which would allow easier piecemeal migration to hand-written peripherals.

On thing that worked very well in that F3 GPIO PoC I made was defining a GPIO "prototype" that had all possible registers and register fields and then removing and modifying registers to create the actual IP definitions. I found that to be the method that required the least code duplication. Is something like that possible with the current YAML DSL?

On a slightly higher level I was trying to solve many of these problems with stm32ral, which parses every SVD file together, then automatically refactors out all the shared peripherals within a family.

I unfortunately haven't gotten around looking at your ral project yet. From what you describe there would still be the problem of sourcing our information from ST's SVDs which are family-based, not IP based. So we wouldn't solve the issue I described in the OP that way. Unless we somehow generate our own finer-grained SVDs, but that's the hard part of course.

@jcsoo
Copy link

jcsoo commented Sep 20, 2020

This is a subject that I spent a significant amount of time working on - you can see the approach that I took in the Bobbin SDK project repository. I started with SVDs but converted them into a custom DSL which was then used to generate multiple levels of crates. This DSL included higher-level concepts such as alternate function routing, shared peripherals, and board definitions. Codegen produced several levels of crates as well as hand-written peripheral drivers.

Much of this was done in a semi-automated fashion. For instance, I had scripts that, with some assistance, able process PDF datasheets to produce usable pin alternate-function table definitions (using Tabula to extract the raw text). There was also a tool that could produce peripheral signatures that could be used to compare across processor models and even families.

Philosophically the overall strategy was different than the current svd2rust ecosystem. Most svd2rust crates operate strictly as a downstream derivative of the original SVD files with varying amounts of patching to clean up and refactor those files, but ultimately limited to the data model supported by the SVD format and data model. Hand-written crates are then layered on top of the PACs.

Bobbin SDK took the approach that Bobbin DSL was the source of truth and that it should (a) support more concepts than SVD and (b) be easier to hand edit than SVD. This did mean that it wouldn't be possible to automatically handle upstream SVD revisions, but I didn't see that a big loss given how infrequently these SVD files were updated.

A major benefit was that Bobbin SDK had true shared peripheral crates such as bobbin-stm32/stm32-common which allowed writing a single driver for a peripheral variant that could then be used across all compatible MCUs.

Unfortunately there were big differences between this approach and the svd2rust approach, and I couldn't see how to justify that effort without becoming a commercial Rust embedded tools vendor (which I considered for some time), and I had too many other things going on to try taking on a big open source project.

At the same time, I've spent the last few days working on implementing peripherals for the stm32g4xx-hal crate, and I've found it incredibly frustrating. Most of my effort has gone into cutting and pasting code from various other stm32-rs HAL crates and then tediously trying to edit them to work with a slightly different clock management system. In many cases I can see that these peripheral drivers have themselves been cut and pasted from other similar crates.

This all feels very un-Rust-like, not in the least because it means that if I want to write a driver or application that uses, for instance, a particular common STM32 ADC variant, I will have to sprinkle it with #cfg blocks and feature flags for every processor variant I might want to use it with. Even worse, if a newer MCU comes out, I need to go back and explicitly add a feature flag and then update all my #cfg blocks. And if the authors of one HAL crate have implemented some functionality I need but the authors of another haven't, I'm just plain out of luck. At that point I'm better off writing my own private HAL and maintaining it on my own - which I'm sure a lot of people are doing.

@adamgreig
Copy link
Member

Thanks for weighing in @jcsoo! Your Bobbin project looked really nice and I'd be very interested in ways we could bring in some of its ideas. I agree that that the current model of "one SVD file produces one crate which represents one device" is not ideal; by not explicitly modelling the shared peripherals we miss a lot of ability to write common drivers. My approach has been to use all the SVD files together to try and infer what peripherals are shared, but small inconsistencies in the SVDs can ruin that, and doing it manually is a lot of work as I'm sure you know. I'm also not the biggest fan of the svd2rust generated API, although it has quite a lot of inertia now.

For all that SVD files cannot represent every detail, I think they do have a lot of expressive power, and being a widely used and documented standard has a lot of advantages, so it seemed to make sense to try and produce clean SVDs as a source of truth. Having the GPIO AF assignments spelled out in the HAL code doesn't seem too bad, but there's probably a bunch of other information I'm not thinking of which it would be nice to capture.

For a lot of the more complicated drivers like usb and ethernet, which are also typically the same across many devices in many families, there has been some success writing a generic driver (https://github.com/stm32-rs/synopsys-usb-otg, https://github.com/stm32-rs/stm32-usbd, https://github.com/stm32-rs/stm32-eth, etc) which the family HALs integrate. For the USB drivers, the crate basically includes its own register definition which is the same across all the chips, without using the svd2rust-generated API at all. If we did the same sort of thing for more peripherals, we could hopefully avoid each HAL having to copy-paste large files with minor tweaks to accommodate the inconsistent SVDs.

However, for simpler drivers, there seem to more often be small differences between chips and between families, which unfortunately results in totally distinct types with the current model. In C where the types are basically changed for each build by specifying a different device header file it doesn't really matter, because so long as the field name exists it will work across all devices, but with how we currently write MMIO types that's harder in Rust. How did you handle that sort of thing, where one device in a family might have one extra bit in a register, but otherwise all the behaviour is the same?

@jcsoo
Copy link

jcsoo commented Sep 21, 2020

I want to make clear that I think having these cleaned-up, normalized SVDs is a great thing! In fact, I think it would be incredibly useful to have a collective cross-MCU, cross-language project to produce and organize these SVDs. There's no reason why other MCU families shouldn't benefit from all the tooling that has been developed here. These files are useful for many other purposes outside of generating Rust PACs.

The challenge is figuring out how to go beyond SVD so that we can model all the additional information that we need.

I've always viewed SVD as a lossy output format for one specific use case: debuggers. Although theoretically suitable for other uses, vendors never actually cared about any of them and I don't think any of them have ever used SVD for any other purpose. They all have their own internal data and tools for generating all of their header files, interactive code generators, layout tools, etc. For most of these vendors I expect that "data" should be read as "folders of spreadsheets and random text files" and "tools" be read as "scripts written 20 years ago and/or programmers tasked with cutting and pasting or manually editing header files by hand".

My vision was to come up with a data model and representation that was rich enough to be used for all the tooling you'd expect in a modern SDK - not just C headers and Rust register descriptions, but also interactive debuggers and code builders similar to STM32CubeMX. That data model needs concepts such as AF tables, memory maps and physical pin assignments.

Most importantly it needs first-class cross-device peripherals. Those peripherals in turn need fine-grained feature support down to the individual register bit level so that you can say "device A has peripheral B version C variant D with features E, F and G". That in turn allows creating peripheral crates targeting variants and features rather than specific device models, which in turn allows creating applications that target peripherals with a certain level of functionality.

(At some point it's not worth having every single variation show up in the peripheral register API; for relatively obscure variations, I think it's appropriate to include the superset of bits and then describe the availability in documentation, similar to how errata or chip state-dependent variations are handled. This is a judgement call.)

I attempted to tackle this with my own DSL, but that's not the only approach. One possibility is to extend SVD, but this has the disadvantage that those files wouldn't be compatible with existing tools that use SVD. Another is to add supplemental data alongside SVD; that would be OK for some types of data (for instance memory maps and physical pin assignments) but really cumbersome for others (shared peripheral definitions).

(As another aside, I found it really useful in practice to have a single human-readable, searchable source file with all of the information I needed to work with a chip in one place - see the definitions for the stm32f40x or samd51 an example.)

Maybe it would be useful for me to take another look at my Bobbin work and figure out what parts might still be the most useful, and then write up and publish those pieces as independent crates. There are some things that I miss a lot such as having automatically generated Debug implementations for registers that show individual bitfields, plus I am also not the biggest fan of the svd2rust style API. I think we are still at the stage where there should be more experimentation with different approaches.

@adamgreig
Copy link
Member

If nothing else, I guess we could bootstrap a new representation from the current SVDs, even if we then moved to modifying the new representation by hand (feasible, if essentially all redundancy was eliminated, as the only reason to use the current YAML patching of SVDs is because a) hand editing XML is hell b) need to apply same fix to many SVD files). We could also consider generating SVD files from the new representation to feed tooling that consumes them.

I've always viewed SVD as a lossy output format for one specific use case: debuggers. Although theoretically suitable for other uses, vendors never actually cared about any of them and I don't think any of them have ever used SVD for any other purpose. They all have their own internal data and tools for generating all of their header files, interactive code generators, layout tools, etc. For most of these vendors I expect that "data" should be read as "folders of spreadsheets and random text files" and "tools" be read as "scripts written 20 years ago and/or programmers tasked with cutting and pasting or manually editing header files by hand".

Firm agree.

(At some point it's not worth having every single variation show up in the peripheral register API; for relatively obscure variations, I think it's appropriate to include the superset of bits and then describe the availability in documentation, similar to how errata or chip state-dependent variations are handled. This is a judgement call.)

Yep, totally agree.

Did you find any issues with your DSL? Would you do it differently today? I definitely have some regrets around not making the YAML patching format a bit more well specified (and well implemented...) but it might be a "worse is better" situation in that it worked well enough to be useful. I'm not sure how easy it is to achieve, but I do like the idea of a single really good DSL representation of the peripherals and ICs. The amount of work involved to make, check, and keep up-to-date such a thing is very intimidating; I'm not sure if you could even expect to stay ahead of new devices without a lot of tedious work. Having excellent tools that consumes the new format might help justify the effort in maintaining it though.

There are some things that I miss a lot such as having automatically generated Debug implementations for registers that show individual bitfields

I don't think there's any reason this couldn't be added to svd2rust, to be fair.

I think we are still at the stage where there should be more experimentation with different approaches.

Definitely. I've been glad to see Bobbin and TockOS have their own totally different register manipulation APIs, and I did a very simple one in stm32ral too. There's also the related issue of "derefgate" i.e. rust-embedded/wg#387 which seems to call for a different register manipulation concept anyway.

@jcsoo
Copy link

jcsoo commented Sep 24, 2020

If nothing else, I guess we could bootstrap a new representation from the current SVDs, even if we then moved to modifying the new representation by hand (feasible, if essentially all redundancy was eliminated, as the only reason to use the current YAML patching of SVDs is because a) hand editing XML is hell b) need to apply same fix to many SVD files). We could also consider generating SVD files from the new representation to feed tooling that consumes them.

Yes, both of these are good ideas. I have some tooling that I developed for the bootstrapping part and heuristics for deciding whether peripherals are similar to each other. The good news is that these tools don't have to be perfect since there's a human that will clean up the output, and there aren't all that many unique peripherals across a vendor's product line. Ideally you can do CI in some way against the SVDs to catch typos, etc.

If the DSL is easy to write and/or you have tooling to import from CSV, it only takes a few minutes to generate a peripheral description by hand. It's a big help if you can define arrays of fields and registers.

Did you find any issues with your DSL? Would you do it differently today? I definitely have some regrets around not making the YAML patching format a bit more well specified (and well implemented...) but it might be a "worse is better" situation in that it worked well enough to be useful. I'm not sure how easy it is to achieve, but I do like the idea of a single really good DSL representation of the peripherals and ICs. The amount of work involved to make, check, and keep up-to-date such a thing is very intimidating; I'm not sure if you could even expect to stay ahead of new devices without a lot of tedious work. Having excellent tools that consumes the new format might help justify the effort in maintaining it though.

Going back to look at the DSL after a year or two has been a helpful exercise. There are a few things that I can think of right away, in no particular order:

First, it needs to be decomposed into a set of simpler, related DSLs. Right now things like Pin / Alternate Function assignments are inlined into the main MCU definition file, which makes them harder to maintain and also forces all tools to understand the full DSL. It would be much better to have individual DSLs that correspond more closely to the tables that you typically see in reference manuals and datasheets. This would make each DSL easier to understand and greatly simplify tooling, while making it easier to evolve more experimental DSLs such as clock-tree descriptions or external peripheral interface specifications (SPI, I2C, CAN, and Serial protocol parameters, for instance).

Second, the concept of variants needs to be integrated at a deeper level and in a thoughtful way that allows tools to decide on their own policy for handling differences. This should be done in a cross-cutting way so that it can be used in all contexts - describing interrupt vector tables, memory layouts, clock trees, AF assignments, peripherals, registers, individual fields, and anything else someone wants to model, but it still needs to be easily understandable. This should apply to both MCUs and standalone ICs.

The key to making this all work in the long run is making the minimum useful subset have a very good cost-benefit ratio so that many people will be willing to do it for their own use, and making it easy for them to contribute that work so that others can benefit. Producing an Alternate Function table for a new device only takes a few minutes if you can base it off an existing one; tools such as documentation and code generators (for Rust and C / C++) can make it worthwhile to invest that effort and then share those updates. There's a lot to be learned from Cargo and the whole crates.io ecosystem.

Looking at the big picture, this isn't really just about Rust; it's about making it easier to do embedded development without depending on proprietary vendor tools, no matter what the language.

Definitely. I've been glad to see Bobbin and TockOS have their own totally different register manipulation APIs, and I did a very simple one in stm32ral too. There's also the related issue of "derefgate" i.e. rust-embedded/wg#387 which seems to call for a different register manipulation concept anyway.

I revisit this every once in a while just for the fun of it; I've probably produced a half-dozen variations that have never been published. Right now I'm trying out operator overloading (with type checking and using volatile_read / volatile_write behind the scenes):

UART[1].cfgr  |= UART_CFGR_PARITY_NONE | UART_CFGR_BITS_8;
UART[1].baudr |= UART_BAUD_DIVIDER_19200;
UART[1].cr    |= UART_CR_ENABLE;

while UART[1].sr & UART_SR_DATA_READY == 0 {}
let data = UART[1].dr & UART_DR_DATA;

I haven't decided whether this is a good idea yet, but I find it a lot easier to read and write than the typical method-chaining approach.

@therealprof
Copy link
Member

The more I think about the tooling (and especially svd2rust), the more I'm convinced that generating Rust code directly is not the best approach (anymore). For one the Rust compiler sucks are compiling those enormous chunks of code (and it also fails spectacularly to generate useful binary code in dev mode which some people do care about) and secondly the used tools also suck at generating size-efficient (and also human readable) Rust code and I've been repeatedly told by the authors that doing so is an anti-goal and will not be given any consideration. Those are two very strong indications for me that we should be looking into going a different direction with svd2rust which could very well mean to have a DSL and a set of proc macros to generate an efficient abstraction on demand (for compile time performance and generated binary efficiency) and on the fly (to avoid lugging dozens of megabytes of unused data around).

I do NOT agree with deviating too far from the current concept of a PAC, that concept is proven and works quite well and we certainly need to have a basic MMIO register abstraction like that.

In that sense I also don't see how a PAC can be responsible for abstracting over peripherals over different series; the only thing that stays completely identical is the concept, a lot of details are quite different which makes the idea totally unfeasible. Some new models from an originally old serious even include different versions of a peripheral in a single chip. The idea of handling all peripherals via a single HAL abstraction is certainly a good one but even that is going to be rather tricky (in the end there's a lot of explicit detailed knowledge required) and I don't quite see how a PAC can help there.

Since this is totally not STM specific but applies to all devices from any vendor on any architecture this is likely not even the right place to have this discussion.

@jcsoo
Copy link

jcsoo commented Sep 28, 2020

Although these issues aren't specific to STM32, I do think they are the most obvious here because of the large variety of device families / devices and the range of people using them. Most other architectures only attempt to support at most a few variants from the same generation, so they can probably get away with assuming peripherals are structurally identical and using cfg attributes to substitute in the appropriate PAC.

What we're seeing here is that there is a huge amount of policy that is deeply embedded within svd2rust that has big impacts on not only the way that code is written in an individual project, but how downstream projects can even be structured. The project has been critical to making Rust possible to consider for embedded development, but its success means that it's locked into those choices.

Here's a fundamental example: svd2rust includes register reset values as part of its register API. The result is that even within a single device, there is not a single GPIO type (see gpioa, gpiob and gpioi for the stm32f407). This means that it is not possible to create a single standalone GPIO crate that doesn't require including the underlying device and using macros for code generation; that in turn means that CI (even just to check if the code compiles) can only be done for a single device at a time and requires compiling the complete device crate each time; that in turn means that a full CI run would require doing a build for every supported device variant!

Unfortunately this isn't something where svd2rust can evolve to solve these issues, even if the authors want to. The good news is that alternatives can be built that can coexist. stm32-rs might be a good place to prove them out and refine them before pushing for wider adoption.

@therealprof
Copy link
Member

Most other architectures only attempt to support at most a few variants from the same generation, so they can probably get away with assuming peripherals are structurally identical and using cfg attributes to substitute in the appropriate PAC.

I don't think that's accurate to say. STM32 is quite diverse even within a single family. Other vendors like e.g. Nordic have almost identical register sets through all of their (few) models so it's easy to say: Here's a superset of all things, the rest is handled in the driver by using appropriate config gating. Yet other vendors may be more like Nordic or even worse than ST; if the maintainers only support a fraction of the models, it's not because they don't want to but because it requires crazy amount of resources...

What we're seeing here is that there is a huge amount of policy that is deeply embedded within svd2rust that has big impacts on not only the way that code is written in an individual project, but how downstream projects can even be structured. The project has been critical to making Rust possible to consider for embedded development, but its success means that it's locked into those choices.

As stm32-ral has shown it is possible for multiple approaches to co-exist just fine, so if we have a strategy we can certainly evolve svd2rust in a way that would allow us to have two different APIs in parallel; people could take their time to migrate the HALs to the new API while continuing to use the old API as long as they need.

there is not a single GPIO type

GPIO is the absolute worst case. I think it's close to impossible to crate a sane and safe low-level abstraction across all families. Clock setup is similarly impossible.

Unfortunately this isn't something where svd2rust can evolve to solve these issues, even if the authors want to.

I disagree. The authors want to evolve svd2rust and if we have a plan (and conensus) we can do it in svd2rust.

@jcsoo
Copy link

jcsoo commented Sep 28, 2020

When you say "family" you are meaning a single related set of devices coming from a single vendor, usually sharing the same vendor SDK?

I don't think that's accurate to say. STM32 is quite diverse even within a single family. Other vendors like e.g. Nordic have almost identical register sets through all of their (few) models so it's easy to say: Here's a superset of all things, the rest is handled in the driver by using appropriate config gating. Yet other vendors may be more like Nordic or even worse than ST; if the maintainers only support a fraction of the models, it's not because they don't want to but because it requires crazy amount of resources...

I think we are actually in agreement on this. I am arguing that the current svd2rust approach makes the costs higher than they otherwise could be, which increases the resources it takes to support additional models in some families and makes it less likely to happen.

GPIO is the absolute worst case. I think it's close to impossible to crate a sane and safe low-level abstraction across all families. Clock setup is similarly impossible.

I think I wasn't clear enough - I wasn't talking about creating GPIO peripherals across families; even within families, different generations might have incompatible variants. What I was pointing out is that even within a single device, I cannot have a single peripheral type implementing the current svd2rust register API for all GPIO peripherals in that device, even if I attempt to write it by hand as is done in stm32-usbd. This is because of reset() and because write(|w| ...) passes the reset value as the argument of the closure.

Ok, this isn't strictly true - it might be possible to work around this within a single device by having reset() and write() look up the reset value based on the base address of the peripheral. But it still holds for someone attempting to create a within-family driver crate for a specific peripheral variant that doesn't require specifying the exact device at build time. Even the stm32f3 crate mentioned in the original post resorts to having a full hard-coded register definition for each individual variant. This just isn't scalable.

This is how I was pulled in.... I have a project using the STM32G74, and I was looking for a reasonable driver for the ADC, which is almost identical to the one in the STM32F3 (see AN5310). I started by trying to implement within the HAL, which was an exercise in frustration, and then looked at what it would take to do a standalone crate, which eventually led me to this issue. And I previously coded basic ADC drivers for all the mainstream STM32 variants I could get my hands on at the time.

I disagree. The authors want to evolve svd2rust and if we have a plan (and conensus) we can do it in svd2rust.

If the svd2rust authors have ideas for evolving svd2rust, that's great, and if I thought there was a way to do it incrementally I'd contribute. But, as I've mentioned above, I think there are more fundamental changes required beyond SVD to Rust code generation, particularly if we want to enable more advanced usage such as build-time configuration tools and proc macros.

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

4 participants