search

hackndev / zinc

The bare metal stack for rust
zinc.rs
Apache License 2.0
1k stars 100 forks source link

RFC: Generalize platformtree (generalized_platformtree) #286

Closed mcoffin closed 9 years ago

mcoffin commented 9 years ago

Allows construction of a platformtree with no additional macro code for each platform.

Motivation

As of right now, to get platformtree to be supported on a platform, a significant amount of macro/plugin code must be written. This is undesirable as this code is a maintenance liability with regards to platform code changes, as well as a huge burden when adding a new platform to zinc.

Detailed design

Basic Example

An example of the new platformtree (direct port of the lpc17xx uart demo) would be as follows. This demo assumes that the *Conf structs have been defined with the right contents (usually the same as the arguments to ::new... might even consider generating the structs with a syntax extension.

// ...

use zinc::hal::lpc17xx::{system_clock, pin, uart};
use zinc::hal::pin::{Gpio, GpioDirection, GpioConf};

// ...

platformdef!(
comp sys_clk: system_clock::Clock = ClockConf {
    source: "main-oscillator",
    source_frequency: 12_000_000,
    pll: Some(system_clock::Pll {
            mul: 50,
            div: 3,
            out_div: 4,
        }),
};

group pin::PinConf {
    comp uart_tx: pin::Pin = GpioConf {
        index: 2,
        direction: GpioDirection::Out,
    };

    comp uart_rx: pin::Pin = GpioConf {
        index: 3,
        direction: GpioDirection::Out,
    };

    comp led4: pin::Pin = pin::PinConf {
        port: pin::Port1,
        index: 23,
        function: pin::Function::Gpio(GpioDirection::Out),
    };
};

comp uart: uart::UART = uart::UARTConf {
    baud_rate: 115200,
    mode: "8N1",
    tx: $uart_tx,
    rx: $uart_rx,
};

entry run($led4, $uart);
);

// ...

fn run<P: Gpio>(txled: P, uart: &uart::UART) {
    // ...
}

There are 3 basic commands in the grammar for the new platformtree.

The macro would then expand do something similar to this. Note: this assumes that there exists the following FromUnsafe trait which would be defined in zinc, and that FromUnsafe<XConf> is implemented for all X where it needs to be.

use core::convert::From;

pub trait FromUnsafe<T>: From<T> {
    unsafe fn from_unsafe(T) -> Self;
}
mod pt {
    #[start]
    #[allow(ununsed_variables)]
    fn start(_: isize, _: *const *const u8) -> isize {
        unsafe {
            let sys_clk = system_clock::Clock::from_unsafe(ClockConf {
                // ...
            });
            let uart_tx = pin::Pin::from_unsafe(GpioConf {
                // ...
            });
            let uart_rx = pin::Pin::from_unsafe(GpioConf {
                // ...
            });
            let led4 = pin::Pin::from_unsafe(pin::PinConf {
                // ...
            });
            let uart: uart::UART = uart::UART::from_unsafe(uart::UARTConf {
                // ...
                tx: uart_tx,
                rx: uart_rx,
            });
            main(&led4, &uart);
            0
        }
    }
}

The from_unsafe method is unsafe because the passed configuration may be invalid, but it will interpret it as valid regardless to avoid any runtime checks. A runtime-checked version of from_unsafe should be in from. Using from_unsafe is possible because the platformtree macro will have checked the validity of the configuration at compile-time. This preserves zinc's ability to provide incredibly fast runtime code, due to the lack of need for runtime checks and computations.

Safety

There are 3 things that need to be checked by the macro to ensure that the generated code will be safe.

  1. Each configuration is actually valid for the hardware.
  2. There are no conflicts between multiple configurations in the same group.
  3. Each comp is consumed no more than once (fortunately the borrow checker will do this for us... see below).

Note that it is assumed that there can be no conflicts between comps in different groups (i.e. instantiating a timer will never conflict with instantiating a gpio pin). Therefore, it is necessary to group all things that may cause a direct conflict between one another.

There are a few conditions under which the generated code will not compile:

These failures take care of (3) for us (yay!). Note that components will have to be updated to not be copyable, however.

Conflict Checking

To check (2), all the *Conf expressions in a group must be checked to ensure that there are no conflicts amongst them (i.e. using the same pin index / port combo for gpio pins).

To check this, we have to add a little bit of macro-crate code for each platform... specifically, for each conf struct that is going to be the root of a group, we must implement a method to check if there will be conflicts between two of the structs. Then, for every struct T besides that one that will be used as the group root, we must implement something akin to FromUnsafe<T> to convert the config struct to the root config struct for use in conflict checking. Then, just check for conflicts between every combination of two configurations within the group.

To check (1), one simply needs to implement a method that checks the validity of each of the Conf structs individually. Again, this is a little macro code when compared to the current amound required to build a platformtree for a new platform.

After these checks have been performed, it's just a matter of emitting the code as above, which is a very simple task as you will note that every identifier used has been passed in to the platformtree somehow, meaning that there is no strange macro code converting between types or serializations.

Alternatively, the checking of conflicts and configuration validity could be left to a lint pass extension, which would be incredibly slick. I haven't much experience with lint extensions, so if you know of a good way you think you would implement conflict & configuration checking with a rust lint pass, please comment.

Drawbacks

This makes the platformtree written by the user much more verbose.

Alternatives

Other types of more generic key/value syntax were considered, but using rust-like syntax makes for easy passing of things onwards to rustc to avoid a lot of macro code.

If we do not implement something like this, platformtree will slowly become a maintenance hazard as a ton of macro code has to be written for every peripheral on every platform possible.

Unresolved questions

Syntax

What syntax should be used for cross references within the tree? I currently went with $var syntax because that's what the quote_ macros in rustc use.

General syntax things: pretty much any syntax in the platformtree is obviously up for debate.

Conflict Checking

A little more thought needs to be put in to the design of the conflict checking code as there could be some problems that arise with the static dispatch method mentioned above. Ultimiately, however, this kind of conflict checking based on the AST representations will be possible, and (I hope) much cleaner than the current amount of macro code required for every new peripheral / platform.

On second thought, conflict checking may actually be better left to a lint pass. That would be really slick. If you have a better idea: comment!

mcoffin commented 9 years ago

Crazy idea afterthought: If all conflict and configuration checking could be implemented with a lint pass alone, one could almost get rid of the platformtree! macro entirely...

While a lint pass may be useful for some things, it shouldn't be here. I tried to build it, and it ends up being very messy, and can actually be disabled with a simple #[allow(zinc_invalid_configs, zinc_config_conflicts)].

mcoffin commented 9 years ago

I wrote this waaaay too late at night. Needs more thought. Closing for the time being.

bharrisau commented 9 years ago

A precursor to this is deciding where the mapping from board to periperals occurs. i.e.

Then when the end user says "I have a Teensy 3.1, I want a uart on pin 0, and the led activated", Zinc knows what version of the UART HAL to use, which pins are able to be used, etc.

posborne commented 9 years ago

Good thoughts. I agree with @bharrisau that figuring out where to define "boards" (and give users the ability to provide boards) will live. With device tree in the kernel, this is handled via somewhat ad-hoc via inclusion of dtsi (device tree source includes). Typically a silicon vendor will define a bunch of stuff, most of which is marked as disabled. Board/SOM providers will then build on this, enabling some parts which have concrete usages and leaving others untouched. Finally, an end customer will define the final layer on top of this for his/her design.

I definitely don't like having to have a <peripheral>.rs and <peripheral>_pt.rs for each peripheral/target combination.

bharrisau commented 9 years ago

Additional thought, we can avoid the need to maintain a parser if we use the MetaItem parser.

#[zinc_main(teensy_3_1,
            clock(in(main, 12_000_000), out(pll, m=50, n=3, d=4)),
            timer(1, counter=25, divisor=4),
            gpio(teensy23, out),
            someVendorPeripheral(xxx, yyy))]
fn main(clock: ZincClock, mytimer: ZincTimer, led: ZincGPIO, special: zinc::hal::vendor::CustomPeripheral) {}

#[zinc_main(custom(clock=zinc::hal::freescale::Clock123, timer=mycrate::CustomTimer),
            clock(in(main, 12_000_000), out(pll, m=50, n=3, d=4)),
            timer(1, counter=25, divisor=4))]

Being able to handle peripherals defined outside of zinc complicates things (as the zinc_main macro can't just call the code at compile time). In the above example, the zinc_main macro holds the mappings for all the supported boards.

bharrisau commented 9 years ago

I'll start a wiki entry for the Zinc init process, as that needs to be defined first.

farcaller commented 9 years ago

Additional thought, we can avoid the need to maintain a parser if we use the MetaItem parser.

That looks more like a hack and you can hardly provide same error reporting in this case.

mcoffin commented 9 years ago

So I've actually been experimenting with just using a lint plugin and then checking every struct expression against each other to make sure you don't every define a conflicting PinConf struct... anywhere. It actually feels pretty slick.

mcoffin commented 9 years ago

Ok, so, what if we just allowed the user (or zinc) to define boards by defining some simple const traits on Conf structs.... for example, if we take the syntax of platformtree from above...

use zinc::hal::sam3x::{pin, uart};
use zinc::hal::{Gpio, GpioDirection};
use zinc::hal::sam3x::arduino_due;

platformdef! {
comp led4: pin::Pin = arduino_due::GpioConf {
    index: 13,
    direction: GpioDirection::Out,
};

comp uart_tx: pin::Pin = arduino_due::GpioConf {
    index: 14,
    direction: GpioDirection::Out,
};

comp uart: uart::UART = uart::UARTConf {
    baud_rate = 12435,
    ...
    tx: uart_tx,
};

entry main(&led4, &uart);
}

fn main<T: Gpio>(tx_led: &T, uart: &uart::UART) {
    // ...
}

That would let the user decide whether to use the board mapping style or that of the MCU, which I imagine would be rare, but at the very least handy for examples. This is an example expansion of the above macro.

use core::convert::Into;
use zinc::util::FromUnsafe;

const led4_CONF: pin::PinConf = arduino_due::GpioConf {
    // ... from the macro
}.into();

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
    let led4 = unsafe {
        pin::Pin::from_unsafe(led4_CONF)
    };
    // ... initialize the rest of the components the same way

    main(&led4, &uart);
    0
}

Then the arduino_due mod just has to define the GpioConf struct and const impl Into<pin::PinConf> for GpioConf and you're off to the races.

farcaller commented 9 years ago

That's how it used to work, actually?

mcoffin commented 9 years ago

Why exactly did the project choose to move away from that?

While we don't have a 1:1 mapping between platformtree keys/values and something in rust, there will always be lots of platform-specific parser-related code (I think).

awelkie commented 9 years ago

The current implementation of const fn doesn't allow for const implementations of traits, so you'd have to wait for full CTFE to land before you can do exactly what you propose (see the const fn proposal for details)

But once CTFE does land, could we do something like the following:

trait ZincConfig {
    type VerfiyError;
    const fn verify(&self) -> Result<Self, Self::VerifyError>;
}

Then the user would just do something like this:

#[inital_config]
const MyConfig = MyBoardConfig {
    gpios: [GpioConf{...},],
    ....
}

And the initial_config syntax extension would just need to check that MyConfig is Ok(a). This would keep almost all of the platformtree logic out of syntax extensions. This is only do-able, of course, once rust has CTFE, but you could probably fake it by doing the name mangling yourself (e.g. every board has a my_board_config_verify function instead of impling a trait.

awelkie commented 9 years ago

Also, what do you mean when you say there isn't a 1:1 mapping between platformtree and rust? Could you give an example? Does this mean platformtree will always need to be a syntax extension?

farcaller commented 9 years ago

Why exactly did the project choose to move away from that?

The reasoning was that back at that time such structs could sometime pollute the .rodata and their only use was to create a bunch of other structs. As I later started to consider power planes and hardware re-init, PT started to actually shape back to those structs.

CTFE would really help with the messy macro approach.

mcoffin commented 9 years ago

@awelkie When I said there's no 1:1 mapping between PT and rust code, I meant that every object in the platformtree doesn't necessarily have an equivalent in rust. For example, take the gpio PT code from the blink_pt example...

    gpio {
      1 {
        led1@18 { direction = "out"; }
        led2@20 { direction = "out"; }
      }
    }

What exactly would the gpio object map to? Well, it maps to the collection of all GPIO peripherals on the lpc17xx board. There is no such abstraction in zinc, as we go straight to the level of a Pin.

This can be interpreted in one of two ways.

  1. This is a fault in platformtree as platformtree should map 1:1 with zinc's hal code.
  2. This is a fault in the zinc hal code for the platform, as it doesn't closely enough resemble the hardware so that the hardware configuration can map 1:1 with hal code.

After writing that, I'm leaning more towards the 2nd interpretation. Perhabps too much abstraction was done too quickly in writing the hal code. Maybe the hal code should represent a complete peripheral (or collection thereof). That would be a lot of work to change, but in the end might be a necessary evil to sort out what abstraction should be done where.

mcoffin commented 9 years ago

Note for self: related #207