AdaCore / Ada_Drivers_Library

Ada source code and complete sample GNAT projects for selected bare-board platforms supported by GNAT.
BSD 3-Clause "New" or "Revised" License
236 stars 141 forks source link

Handling of drivers referencing non-existant hardware features in svd #340

Open Hendrik410 opened 4 years ago

Hendrik410 commented 4 years ago

Hello,

I am currently implementing the STM32F411RE Nucleo board. I have downloaded the svd files from ST and generated the svd ada files.

I've then copied the device folder from another device and started modifying. While doing so, I commented all the periphirals out, that the STM32F411 doesn't have (The extra timers, the extra usarts, the dac).

I've also created the necessary entries and runtime projects for the board.

When I open the duplicated example project, the compiler copmlains about the missing references of from the stm drivers. An example is seen here: image

Now if I understand it correctly, the drivers are common for all STM32 devices.

My question is, how should missing peripherals be handled?

Fabien-Chouteau commented 4 years ago

Hello @Hendrik410 ,

I've also created the necessary entries and runtime projects for the board.

Did you add the board and device definition in the config scripts?

Now if I understand it correctly, the drivers are common for all STM32 devices.

They are at least common to all the STM32 devices that we support so far. But since we do not know the hundreds of STM32 devices available (and the ones to come),

My question is, how should missing peripherals be handled?

There is no easy answer I am afraid. Adding to discussion @pat-rogers who wrote this package.

Hendrik410 commented 4 years ago

Hi @Fabien-Chouteau ,

yes, I've used the config scripts.

I'm not to firm in Ada, I know that in C, compiler dirictives would be used. Is there a similar construct in Ada?

Fabien-Chouteau commented 4 years ago

No, Ada doesn't have the same feature, which is a good thing in most cases :)

@pat-rogers what is your opinion on this?

pat-rogers commented 4 years ago
Now if I understand it correctly, the drivers are common for all
STM32 devices.

They are at least common to all the STM32 devices that we support so far. But since we do not know the hundreds of STM32 devices available (and the ones to come),

My question is, how should missing peripherals be handled?

There is no easy answer I am afraid. Adding to discussion @pat-rogers https://github.com/pat-rogers who wrote this package.

When I wrote this package I admit thinking to myself that these "timer classifier" functions needed to go in some device-specific location. A separate package from STM32.Timers certainly.

They can't go in package STM32.Device because that would create elaboration order circularities, due to the Timer formal parameter.

This is a fundamental issue so making a board-specific version of the STM32.Timers package itself is not the right approach.

(As an aside, based on the manual I think it is safe to say that the only issue is the presence or absence of any given timer itself, not whether any given timer should be in any given classifier function. In other words, for example if TIM2 exists (TIM2_Base is defined) then TIM2 has the remapping capability and can be in that classifier function.)

It may be that we have to put these functions in the STM32_SVD package hierarchy. We'd need to pass something other than Timer though, so this needs thinking about.

There are one or two other cases like this too, in other packages, IIRC.

I'll give this some thought over the next couple of days (can't do it tonight, in other words). If time is critical, in the short term you could make a separate device-specific version of the Timers package and edit the classifier functions, but that's not a good solution in the long term.

pat-rogers commented 4 years ago

They can't go in package STM32.Device because that would create elaboration order circularities, due to the Timer formal parameter.

...

I'll give this some thought over the next couple of days (can't do it tonight, in other words).  If time is critical, in the short term you could make a separate device-specific version of the Timers package and edit the classifier functions, but that's not a good solution in the long term.

It is not a simple problem, due to the elaboration order and the formal parameter type Timer.

One possibility, that would be simple and clean, would be to generate the declarations for all the timer addresses (eg TIM1_Base) defined by the manual, located as now in STM32_SVD, but giving the null address value to those not actually declared in a particular SVD file.

I don't consider that a hack, actually.

pat-rogers commented 4 years ago

Earlier I said:

One possibility, that would be simple and clean, would be to generate the declarations for all the timer addresses (eg TIM1_Base) defined by the manual, located as now in STM32_SVD, but giving the null address value to those not actually declared in a particular SVD file.

Modifying svd2ada is not the right approach because it is device and vendor independent and, as such, is driven purely by the svd input file. I think the right thing to do is for you to augment the supplied svd file to include the otherwise non-existent timers, assigning them null base addresses. Then the base address decls will be generated and the problematic functions (the timer classifiers) will compile (and function correctly). For example, let's say that TIM20 was a real timer not implemented on a given device, and therefore was not in the svd file. If we add the following to the svd file:

<peripheral derivedFrom="TIM3">
  <name>TIM20</name>
  <description>Nonexistent</description>
  <groupName>TIM</groupName>
  <baseAddress>0x0</baseAddress>
</peripheral>

we will get a declaration for TIM20_Base, which is what the classifier functions need:

TIM20_Base : constant System.Address := System'To_Address (16#0#);

We'll also get a corresponding device declaration:

-- Nonexistent TIM20_Periph : aliased TIM3_Peripheral with Import, Address => TIM20_Base;

The Import aspect ensures no initialization code accesses address zero, and no other code will reference the fake device (only the base address is referenced in the classifier function comparisons).

By having the svd input say that TIM20 is derived from some other timer that is truly present, we will get a usable type for the fake object declaration (TIM3_Peripheral above). The actual type doesn't matter.

Now, all that said, I would prefer a better solution so I will keep thinking about how to address it at the device driver level, rather than the svd generator.

pat-rogers commented 3 years ago

I'm still thinking about this, in the background.

kevlar700 commented 2 years ago

I have found that svd editing is required anyway, so keeping patches may be a good idea? "https://community.st.com/s/feed/0D53W00001JWpMGSA1"

I am currently wondering whilst looking at STM32L4R9 if Timer record types showing the register differences (Basic, Advanced, 15, 16_17, Low_Power) and using Generics or overrides to enable common functionality would make this package easier to digest and port to other STM devices by making it clearer which Timers have which registers?

pat-rogers commented 2 years ago

On 21-Feb-22 8:28 AM, Kevin Chadwick wrote:

I am currently wondering whilst looking at STM32L4R9 if Timer record types showing the register differences (Basic, Advanced, 15, 16_17) and using Generics or overrides to enable common functionality would make this package easier to digest and port to other STM devices by making it clearer which Timers have which functionality?

I looked into various approaches somewhat, initially, but I didn't spend a great amount of time trying to arrange them into something simpler. I looked into an inheritance hierarchy but don't recall the problems(s) encountered.

I got the impression from the online support groups that the STM32 timers were unusual in their complexity as a collection, in that it is difficult to understand them individually without understanding the entirety.

A better approach would be very welcome.

Best regards,

Pat

kevlar700 commented 2 years ago

A better approach would be very welcome.

Better is easy, until you notice the regressions!

I looked into various approaches somewhat, initially, but I didn't spend a great amount of time trying to arrange them into something simpler. I looked into an inheritance hierarchy but don't recall the problems(s) encountered. I got the impression from the online support groups that the STM32 timers were unusual in their complexity as a collection, in that it is difficult to understand them individually without understanding the entirety.

Okay, Thank You. I like the info that the contracts provide in the ads file. In a couple of places, it seems like a counter size and Channel mode selection enforced by 5-7 Timer types instead of throwing exceptions on pre conditions, would be nice. I am not fluent on ST hw yet. Therefore, I will try it out piecemeal and make considerations as I require the functionality in real world use cases instead of trying to understand everything now. Then review it further at a later date.

Regards, Kc

FredPraca commented 2 years ago

On 21-Feb-22 8:28 AM, Kevin Chadwick wrote: I am currently wondering whilst looking at STM32L4R9 if Timer record types showing the register differences (Basic, Advanced, 15, 16_17) and using Generics or overrides to enable common functionality would make this package easier to digest and port to other STM devices by making it clearer which Timers have which functionality? I looked into various approaches somewhat, initially, but I didn't spend a great amount of time trying to arrange them into something simpler. I looked into an inheritance hierarchy but don't recall the problems(s) encountered. I got the impression from the online support groups that the STM32 timers were unusual in their complexity as a collection, in that it is difficult to understand them individually without understanding the entirety. A better approach would be very welcome. Best regards, Pat

That's something we discussed a long time ago in #196 . There was some circularities problem AFAIR.

pat-rogers commented 2 years ago

On 24-Feb-22 8:19 AM, Kevin Chadwick wrote:

A better approach would be very welcome.

Better is easy, until you notice the regressions!

I looked into various approaches somewhat, initially, but I didn't
spend a great amount of time trying to arrange them into something
simpler. I looked into an inheritance hierarchy but don't recall the
problems(s) encountered. I got the impression from the online
support groups that the STM32 timers were unusual in their
complexity as a collection, in that it is difficult to understand
them individually without understanding the entirety.

Okay, Thank You. I like the info that the contracts provide in the ads file. In a couple of places, it seems like a counter size and Channel mode selection enforced by 5-7 Timer types instead of throwing exceptions on pre conditions, would be nice. I am not fluent on ST hw yet. Therefore, I will try it out piecemeal and make considerations as I require the functionality in real world use cases instead of trying to understand everything now. Then review it further at a later date.

Sounds like a good plan.

Good luck!

Pat