hackndev / zinc

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

Port ioregs to procedural macros #56

Closed farcaller closed 10 years ago

bharrisau commented 10 years ago

I started having at look at this last night. Ended up with something along the lines of this (trying to be very Rust).

ioreg! {
  match SSP {
    0x00 => CR0,
    0x04 => clear(CR0), // For alias registers that are clear on write
    0x08 => set(CR0),  // For alias registers that are set on write
    0x0C => ro(CR1), // All of CR1 is read only
    0x20 => CR2 // There is a large gap of nothing before CR2 (but you don't need to count it by hand)
  }
  struct CR0 {
    reg1: (0..1),  // reg1 is bits 0 and 1 of CR0
    reg2: dmb(3), // reg2 is bit 3, but needs a barrier instruction for memory access
    reg3: ro(dmb(5))  // reg3 is bit 5, but is read only and also needs barrier instruction
  }
  struct CR1 {
    ...
  }
  let SSP0: SSP = (0x4000_7000, "iomem_SSP0");
  let SSP1: SSP = (0x4000_8000, "iomem_SSP1");
}    

A little 'nicer' than.

iroeg!(SSP[CR0, CR1, DR, SR:r, CPSR, IMSC, RIS, MIS, ICR, DMACR], SSP0@iomem_SSP0, SSP1@iomem_SSP1)
bharrisau commented 10 years ago

The memory addresses are needed in the macro to handle bitband access for single bit registers. I can feed them to the linker script with a rake script to parse the AST. You might want to have either #[cfg(...)] support or something to handle sub-architectures.

farcaller commented 10 years ago

This one is much more verbose though. Well, I see reasoning behind that one given we want to have bit-band access.

Can you provide a bit more details on clear() and set()? I'm not sure why do we care about those in a special way.

farcaller commented 10 years ago

One more thing is that I'm not sure it needs to be that much rust-y. It currently looks like a reasonably usual rust code but it's not, as you would process this with a custom parser rather than rust parser. One might think that it's a valid rust code and add something (like arithmetic operation) that's not supported by custom parser. I think it would be better to keep the syntax simple.

bharrisau commented 10 years ago

Clear and set is for the write-only registers on some things that clear or set the relevant bits in the aliased register. I've seen some peripherals have them and figured it would be better to sugar them a little instead of duplication.

CR0::reg1();
CR0::set_reg1(x);
CR0::set_reg1();
CR0::clear_reg1();
CR0::toggle_reg1(); // some things also have a toggling alias register

The GPIO peripheral on the K20 is one example of these special aliases.

I feel that a custom syntax leaves more room for errors that the compiler will accept (whereas using arithmetic would cause a compiler error). Given that this is the most unsafe part of our code, I think it should be the most verbose and hardest to make a mistake.

Just remembered that I wanted to include enum support too, to sugar the writes to multi-bit variables.

errordeveloper commented 10 years ago

Should we close this as the discussion is now in #90?

farcaller commented 10 years ago

Makes sense.