How to make a combined register?

[flaviojs]

Hi, I'm using tock-registers v0.10.0 to interface with SDHC spec 1.00 registers in hardware that has problems with u8/u16 writes (Nintendo Wii), so I have to combine registers and write as u32.

I know I can manually define an alternate register with register_bitfields! but the usage ergonomics of 2-4 registers in one isn't good. A simplified example of the register map:

register_structs! {
    #[doc = "SD Host Control Register Map (spec 1.00)"]
    pub SdhcRegistersSpec100 {
        (0x000 => _0_27),
        (0x028 => pub host_ctl: ReadWrite<u8, HostControl::Register>),
        (0x029 => pub power_ctl: ReadWrite<u8, PowerControl::Register>),
        (0x02a => pub blockgap_ctl: ReadWrite<u8, BlockGapControl::Register>),
        (0x02b => pub wakeup_ctl: ReadWrite<u8, WakeupControl::Register>),
        (0x02c => pub clock_ctl: ReadWrite<u16, ClockControl::Register>),
        (0x02e => pub timeout_ctl: ReadWrite<u8, TimeoutControl::Register>),
        (0x02f => pub softreset_ctl: ReadWrite<u8, SoftwareResetControl::Register>),
        (0x030 => _30_fb),
        (0x0fc => pub sintr_status; ReadOnly<u16, SlotInterruptStatus::Register>),
        (0x0fe => pub hc_version: ReadOnly<u16, HostControllerVersion::Register>),
        (0x100 => @END),
    }
}
register_structs! {
    #[doc = "SD Host Control Register Map (spec 1.00)"]
    #[doc = "- combines u8/u16 registers into a u32 register"]
    pub SdhcRegistersSpec100_32 {
        (0x000 => _0_27),
        (0x028 => pub host_power_blockgap_wakeup_ctl: ReadWrite<u32, HostPowerBlockGapWakeupControl_32::Register>),
        (0x02c => pub clock_timeout_softreset_ctl: ReadWrite<u32, ClockTimeoutSoftwareResetControl_32::Register>),
        (0x030 => _30_fb),
        (0x0fc => pub sintr_status_hc_version: ReadOnly<u32, SlotInterruptStatusHostControllerVersion_32::Register>),
        (0x100 => @END),
    }
}

Ideally I want to replace the u32 type with a u32-sized struct that has the original registers as sub-registers but I'm having trouble thinking how to do it in a compatible way. Can anyone point me in the right direction?

[lschuermann]

(We're in the process of moving tock-registers to its own, dedicated repository, so moving this discussion here.)

I don't think we have a great story around support this particular feature (sub-registers) right now.

The best approach that works with vanilla tock-registers is to use the native integer width that the hardware expects you to use for accesses, and then define multiple register_fields! entries within that integer to represent the "sub-registers".

Alternatively, you could try implementing the UIntLike trait for your own field type (e.g., a newtype around an integer), which would make it useable as ReadWrite<$YOUR_TYPE>. On that type you can then have convenience methods to access the subfields as smaller integers, wrapped in an LocalRegisterCopy type.

Natively supporting this seems like a can of worms. For instance, if we had different accessor methods for sub-fields but those would translate into reads of the same underlying full-width read / write, that would be able to cause unintended side effects due to needing to do a read-modify-write of multiple registers. The LiteX platform solved the inverse problem, where a single register could be stretched among multiple word-sized addresses. But that solution is neither nice or stable, and will likely be phased out as part of tock/tock#4001.

[flaviojs]

Thanks for the help.
I agree it would be a can of worms to support subregisters natively.
I tried a bunch of stuff and reframing the problem of a subregister as a group of fields in a tree-like structure gave decent ergonomics (ex: R_FC::HostControllerVersion and R_FC::HostControllerVersion::SPEC).

Here is an example of what I ended up with:

macro_rules! register_bitfields_combined {
    (
        $valtype:ident, $(
            $(#[$r_meta:meta])*
            $vis:vis $reg:ident [$(
                $(#[$g_meta:meta])*
                $group:ident  OFFSET($g_offset:expr) NUMBITS($g_numbits:expr) [$(
                    $(#[$f_meta:meta])*
                    $field:ident  OFFSET($f_offset:expr) NUMBITS($f_numbits:expr) $f_values:tt
                ),* $(,)?]
            ),* $(,)?]
        ),* $(,)?
    ) => {
        // unfortunately vanilla `register_bitfields!` does not support groups of fields
        // so flatten the group and it's inner fields as fields
        // and build the tree structure that we want to use for ergonomics
        $(
        #[allow(non_upper_case_globals)]
        #[allow(non_snake_case)]
        #[allow(unused)]
        $(#[$r_meta])*
        $vis mod $reg {
            use tock_registers::register_bitfields;
            use tock_registers::fields::Field;

            register_bitfields!($valtype,
                $(#[$r_meta])*
                $vis $reg [$(
                    $(#[$g_meta])*
                    $group  OFFSET($g_offset) NUMBITS($g_numbits) [],
                    $(
                        $(#[$f_meta])*
                        $field  OFFSET($g_offset+$f_offset) NUMBITS($f_numbits) $f_values,
                    )*
                )*],
            );

            pub use $reg::Register;

            $(
            pub const $group: Field<$valtype, Register> = self::$reg::$group;
            $(#[$g_meta])*
            pub mod $group {
                use tock_registers::fields::Field;

                pub use super::$reg::Register;
                pub use super::$reg::$group::*;

                $(
                pub const $field: Field<$valtype, Register> = super::$reg::$field;
                const _: () = assert!(($field.mask << $field.shift) & !(super::$group.mask << super::$group.shift) == 0,
                    "field bits must be contained in the group bits");
                $(#[$f_meta])*
                pub mod $field {
                    pub use super::super::$reg::$field::*;
                }
                )*
            }
            )*
        }
        )*
    };
}

register_bitfields_combined![u32,
    #[doc = "Combined register:"]
    #[doc = "- Slot Interrupt Status Register (Offset 0FCh RO)"]
    #[doc = "- Host Controller Version Register (Offset 0FEh RO)"]
    pub R_FC [
        #[doc = "Slot Interrupt Status Register (Offset 0FCh RO)"]
        SlotInterruptStatus  OFFSET(0) NUMBITS(16) [
            SLOT1  OFFSET(0) NUMBITS(1) [],
            SLOT2  OFFSET(1) NUMBITS(1) [],
            SLOT3  OFFSET(2) NUMBITS(1) [],
            SLOT4  OFFSET(3) NUMBITS(1) [],
            SLOT5  OFFSET(4) NUMBITS(1) [],
            SLOT6  OFFSET(5) NUMBITS(1) [],
            SLOT7  OFFSET(6) NUMBITS(1) [],
            SLOT8  OFFSET(7) NUMBITS(1) [],
            // bit 8-15
        ],
        #[doc = "Host Controller Version Register (Offset 0FEh RO)"]
        HostControllerVersion  OFFSET(16) NUMBITS(16) [
            #[doc = "Specification Version Number"]
            SPEC  OFFSET(0) NUMBITS(8) [
                #[doc = "SD Host Specification Version 1.00"]
                Version100 = 0,
            ],
            #[doc = "Vendor Version Number"]
            REV  OFFSET(8) NUMBITS(8) [],
        ]
    ]
];