About packed struct for MMIO

Brainstorming
1

One of the many things that got me excited in zig is the ability to use packed structs to do MMIO.
The bit shift dance that is usual in C always seems stupidly annoying and error prone to me.

At the moment, I finally have a concrete use case that gives me a good excuse to play with that stuff. So I’m generating my own register map, but now I realize I somehow missed this part of the zig docs :

pub const GpioRegister = packed struct(u8) {
    GPIO0: bool,
    GPIO1: bool,
    GPIO2: bool,
    GPIO3: bool,
    reserved: u4 = 0,
};

const gpio: *volatile GpioRegister = @ptrFromInt(0x0123);

pub fn writeToGpio(new_states: GpioRegister) void {
    // Example of what not to do:
    // BAD! gpio.GPIO0 = true; BAD!

    // Instead, do this:
    gpio.* = new_states;
}

I didn’t realize that. This seems annoying.

I found out this useful thread : MMIO access restriction using meta programming - #7 by lufe but this leaves me a bit … disappointed maybe ?

I like the microzig approach that seems generic and robust, but (except if I’m missing something huge) this does kill fields auto completion. I deal with a huge register map and straightforward autocompletion / using doc comments for registers and bitfields is an absolute killer feature.

Does anybody know more about why this is the current state of things ? I fail to see the reason for atomic read/write not being the default behavior with a volatile pointer in the example above.

2

Hmm… I would think that gpio.GPI0 = true; is identical with gpio |= GPI0 (assuming that GPI0 is a bit mask), or gpio |= (1<< GPI0) (assuming that GPI0 is the bit position).

If the memory-mapped register allows read-write that should be fine? But sometimes memory-mapped registers are write-only, or they trigger some action when read (like clearing the register), and in that case that pattern would break of course because these are read-modify-write operations. My only experience with MMIO is old-school home computers and emulators though, not modern embedded systems.

FWIW, for similar things in my emulator code I also tinkered with packed structs, but then went back to regular integer bit twiddling, just because the set of operators if ‘richer’ than what packed-structs allow (e.g. &, |, ^ on multiple bits, or <<, >> don’t work on packed structs without casting to the backing integer and back).

2 Likes
3

I like having a packed union which is either the raw backing integer, or a packed struct. That way, you can do regular bit twiddling and operations on the struct!

4

For MMIO I use a integer type (if its large an array of them) to represent the MMIO region itself but use extern/packed structs and enums in the usage code.

This guarantees the correct size of memory load/store for the region and makes it explicit when you are reading and writing preventing things like gpio.GPIO0 = true; gpio.GPIO1 = true; being two reads and two writes.

something like:

const gpio: Gpio = .{ .ptr = @ptrFromInt(0x0123); };

fn doGpioStuff() void {
    var v = gpio.read();
    v.GPIO0 = true;
    v.GPIO1 = true;
    gpio.write(v):
}

pub const Gpio = struct {
    ptr: *volatile u8,

    pub fn read(gpio: Gpio) Value {
        return @bitCast(gpio.ptr.*);
    }

    pub fn write(gpio: Gpio, value: Value) void {
        gpio.ptr.* = @bitCast(value);
    }

    pub const Value = packed struct(u8) {
        GPIO0: bool,
        GPIO1: bool,
        GPIO2: bool,
        GPIO3: bool,
        reserved: u4,
    };
};
3 Likes
5

That makes sense I guess. But this is how I would touch most of theses registers anyways, then.
I guess // BAD! gpio.GPIO0 = true; BAD! might have freaked me out more than needed.

Interesting data point.
But in my case I don’t see much use of << and friends. It’s more just setting fields with values, and taking advantage of that kind of things

    cfg: enum(u2) {
        disabled = 0,
        pulldown = 1,
        pullup = 3,
    } = .disabled,
1 Like
6

Thanks for sharing.

Yeah, this is definitely a valid approach. I saw something similar in this blog post

And now that I think about it, avoiding multiple read/write when updating multiple values in the same register is maybe more important than the atomicity of read / write

1 Like
7

I got bit using a packed struct over MMIO when I first started with zig, the register was 8 bits representing pending events but on read the bits are zeroed as a side-effect.

The intended usage was to read the register in full then loop over the bits in the value to learn of the events but I was accessing each field of the packed struct pointer meaning the moment I read the first field all other fields were zeroed.

It took me an embarrassingly long time to realise I was only ever detecting the first event as true and never seeing any other events. :sweat_smile:

2 Likes
8

Classic. Fall for similar stuff a few times. You don’t need packed struct for that. Had a similar thing with python over USB->SPI->CHIP :see_no_evil_monkey:

9

I prefer to use std.enums.EnumFieldStruct and std.enums.EnumSet
as the high-level and low-level interfaces respectively.

For example:

const GpioOption = enum(u2) {
    pulldown,
    pullup,
};

pub const GpioOptions = std.enums.EnumFieldStruct(GpioOption, bool, false);

pub const gpio_disabled: GpioOptions = .{};

The high-level interface represents the options as named fields:

const options: GpioOptions = .{
    .pulldown = true,
    .pullup = true,
};

Then, for the low-level interface, I use std.enums.EnumSet to obtain the register mask value:

For example:

const options: GpioOptions = .{ .pulldown = true, .pullup = true };
const mask_value = std.enums.EnumSet(GpioOption).init(flags).bits.mask;

This keeps the user-facing API expressive while still allowing direct
manipulation of the register value.

EnumSet treats enum values as bit positions, so the enum values need
to be sequential.

1 Like
10

The problem with accessing individual members (and the equivalent |=, &= etc. in C) is that it is a non-atomic read-modify-write (RMW) operation (with emphasis on non-atomic, as volatile does not have anything to do with atomicity whatsoever). Even with a single-core MCU, this may cause problems because either the CPU may drive an interrupt right in the middle of the RMW operation, or the peripheral changes the register in some way, causing the value you’ve just read to become outdated, which in turn causes you to corrupt the register.

So actually, microzig’s approach (and also what Linux does with accessor functions , fun fact) may be considered the only correct approach. Yes, missing auto-completion is slightly annoying, but then, Zig is very greppable, so the inconvenience becomes much more minor than in C where you have to chase headers.

EDIT: Also, some MCUs offer special registers for modify-operations, specifically because of this problem (e.g. the Microchip/Atmel SAMs’ have OUT for GPIO output, but then also OUTSET, OUTCLR, OUTTGL to set, clear, and toggle bits in the OUT register, respectively), so prefer those when available.

EDIT 2: Correction after further inspection of Microzig

2 Likes
11

Ok, this is interesting as well. Thanks for sharing

12

This leaves more confused actually.

This code here reads, set, and then writes. Is there something preventing an interrupt to change the value of the register in the middle ? What am I missing ?

Oh, for sure ! Order of magnitude less annoying. But autocompletion makes it out of the annoying category altogether…

Yes, sure, if you have a dedicated interface then you just write blindly in there and trust the digital designer. :wink: But to be fair, that was a bit blurry in my mind when I open the thread. It’s “safer” because you don’t read value altogether, not because you make the read/modif/write dance safer.

13

Hmm, I have to admit I only briefly looked at microzig’s Mmio after I’ve written my own (a very incomplete codebase), because I was interested in their approach. I missed this. I don’t think it’s correct, because exactly, the value might change in between.

What I do instead is that I use the atomic ldrex and strex ARM instructions. The meat being this:

while (true) {
    var r = self.rmw();
    r.raw().* &= orig_mask;
    r.raw().* |= new_data;
    r.commit() catch continue;
    break;
}

Where rmw() does ldrex (read) and creates a Rmw context (which gets optimized out in codegen), then I do my modifications, then commit() does strex (write) and lets me know whether the operation has actually completed (i.e. nothing else has done anything to the register in between the ldrex and strex). If it has not, we retry, making this an atomic operation.

2 Likes
14

I do agree, with you and to me the only thing that i think would make this choice even better is if something like this would work.

const Sensor = u32;

pub fn readValue(sensor: *const Sensor) u32 {
    return sensor.*; // not realistic but just for demo
}

pub fn main() !void {
    const my_sensor: u32 = 0;

    while (true) |_| {
        const value = my_sensor.readValue();
        try uart_write("sensor : {d}\n", .{value});
        try uart_flush();
    }
}

it’s a shame we can’t alias/typedef values, without having to wrap it in something in order to be able to do field access. I’m curious why this wouldn’t be desirable, and would love to know why this wasn’t considered. The only argument outside of the usual complexity or burden I can think of, is that it reduce the overall readability for non readers, because you can’t really know for sure if what’s being accessed is an aggregate type or a trivially copy able type.

Either way I agree with you I too tend to rely on just bitshifting, but I would love to try a language that has such ability to alias even simple type and still get method syntax

15

I’ve played quite a bit with Cortex-M controllers and zig, and so far I’ve really enjoyed the flow and abstractions provided by SVD4ZIG GitHub - rbino/svd4zig: Convert System View Description (svd) files to Zig headers for baremetal development · GitHub (out of date, but I maintain a fork when updates come in)

Agreed that atomicity isn’t present, and you would have to decide if your use case warrants a more complex design. I’ve found it a very nice abstraction layer so far

1 Like
16

So, I ended up trying something that I think will do the trick well enough.

This gives an API close to svd4zig it seems.

Since I own the code generation, I realize I don’t need to go crazy on meta programming and settle on something along theses lines :

const Struct_CONFIG = struct {
    const Enum_ALARM_ON_ERROR = enum(u1) { DIS = 0, EN = 1 };

    const Packed = packed struct {
        ALARM_ON_ERROR: Enum_ALARM_ON_ERROR,
        WATEVER: u5,
        READ_ONLY: u1,
        _res: u25,
    };

    const Opts = struct {
        ALARM_ON_ERROR: ?Enum_ALARM_ON_ERROR = null,
        WATEVER: ?u5 = null,
    };
};

I generate on of these for each of the registers, and then

pub const PERIPH = struct {
    pub const CONFIG= Register(Struct_CONFIG.Packed, Struct_CONFIG.Opts).init(0x40060000);   
    ....
}

And finally, the comptime glue

pub fn Register(RegPacked: type, RegOpts: type) type {
    return struct {
        raw_ptr: *volatile u32,

        const Self = @This();

        pub fn init(address: usize) Self {
            return .{ .raw_ptr = @ptrFromInt(address) };
        }
        pub inline fn read(self: Self) RegPacked {
            return @bitCast(self.raw_ptr.*);
        }
        pub inline fn write(self: Self, value: RegPacked) void {
            self.raw_ptr.* = @bitCast(value);
        }
        pub inline fn update(self: Self, new_values: RegOpts) void {
            var reg_copy = self.read();
            inline for (comptime std.meta.fieldNames(RegOpts)) |field_name| {
                const new_val_maybe = @field(new_values, field_name);
                if (new_val_maybe) |new_val| @field(reg_copy, field_name) = new_val;
            }
            self.write(reg_copy);
        }
    };
}

This allow me to do :

PERIPH.CONFIG.update(.{ .ALARM_ON_ERROR = .DIS});

Auto-Completion of both fields and enums works. :clap:
Clearly it doesn’t solve the atomicity concern, but in my case know what the periphs I’m touching and I would have done the same in C anyways.
Note that I don’t generate the fields in Opts for the read only fields. This is not really protecting, you can still change the value if you copy and write it back, but then it’s on purpose. If you try to update a padding field or whatnot you get a nice compile error (and they are not proposed in the first place :wink: )

I don’t know what to thing of the inline in the “meta struct”. I added them because otherwise the assembly in debug mode is scattered of method calls and un-readable. But I don’t know if I should trust the compiler instead in release mode.

17

I also put inline on all of those functions in my mmio, the rationale being that I explicitly want them to compile down to basically single instructions (str, ldr, strex, ldrex) or short groups thereof, which would make no sense to be deduped into functions. I personally think it makes sense for this sort of low level machinery.

18

I think what you want is a distinct type, not an alias.
Aliases is the behavior we already have, where Sensor is just another name for u32.

The distinct types we already have at home (ones with poor/no support for operators) are enums.
(like enum(u32) { _ })
I also think distinct types could potentially be a good addition to the language.

3 Likes
19

yes exactly, poor choice of word on my end, but yeah I’d love to use Distinct type at least try if they are ok in the language.

1 Like