Currently registers work for all untyped I/O regions, which is not ideal. It allows registers defined for device A to work for another device B and there is no safeguarding at all.
All users of the `register!` macro know what type it will be operating on, and that type is consistent across the driver. Therefore, add a `base` parameter to `register!`. Currently this parameter is unused in the generated code; it will be used when all users of `register!` is converted to gain the parameter. Signed-off-by: Gary Guo <[email protected]> --- rust/kernel/io.rs | 4 ++++ rust/kernel/io/register.rs | 51 +++++++++++++++++++++++++++++++++++++++------- rust/macros/io/register.rs | 13 +++++++++++- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index ea072c098b13..84dd876b3407 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -916,6 +916,8 @@ fn try_write<T, L>(self, location: L, value: T) -> Result /// }; /// /// register! { + /// base: Region; + /// /// VERSION(u32) @ 0x100 { /// 15:8 major; /// 7:0 minor; @@ -1058,6 +1060,8 @@ fn write<T, L>(self, location: L, value: T) /// }; /// /// register! { + /// base: Region<0x1000>; + /// /// VERSION(u32) @ 0x100 { /// 15:8 major; /// 7:0 minor; diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs index 45d2e73a92e7..49a5fe069e47 100644 --- a/rust/kernel/io/register.rs +++ b/rust/kernel/io/register.rs @@ -13,9 +13,14 @@ //! # Simple example //! //! ```no_run -//! use kernel::io::register; +//! use kernel::io::{ +//! register, +//! Region, +//! }; //! //! register! { +//! base: Region<0x1000>; +//! //! /// Basic information about the chip. //! pub BOOT_0(u32) @ 0x00000100 { //! /// Vendor ID. @@ -55,11 +60,14 @@ //! register, //! Io, //! IoLoc, +//! Region, //! }, //! num::Bounded, //! }; -//! # use kernel::io::{Mmio, Region}; +//! # use kernel::io::Mmio; //! # register! { +//! # base: Region<0x1000>; +//! # //! # pub BOOT_0(u32) @ 0x00000100 { //! # 15:8 vendor_id; //! # 7:4 major_revision; @@ -429,11 +437,14 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// io::{ /// register, /// Io, +/// Region, /// }, /// }; -/// # use kernel::io::{Mmio, Region}; +/// # use kernel::io::Mmio; /// /// register! { +/// base: Region<0x1000>; +/// /// FIXED_REG(u32) @ 0x100 { /// 15:8 high_byte; /// 7:0 low_byte; @@ -464,9 +475,14 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// the context: /// /// ```no_run -/// use kernel::io::register; +/// use kernel::io::{ +/// register, +/// Region, +/// }; /// /// register! { +/// base: Region<0x1000>; +/// /// /// Scratch register. /// pub SCRATCH(u32) @ 0x00000200 { /// 31:0 value; @@ -516,6 +532,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// /// ```ignore /// register! { +/// ... /// pub RELATIVE_REG(u32) @ Base + 0x80 { /// ... /// } @@ -542,9 +559,10 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// WithBase, /// }, /// Io, +/// Region, /// }, /// }; -/// # use kernel::io::{Mmio, Region}; +/// # use kernel::io::Mmio; /// /// // Type used to identify the base. /// pub struct CpuCtlBase; @@ -563,6 +581,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// /// // This makes `CPU_CTL` accessible from all implementors of `RegisterBase<CpuCtlBase>`. /// register! { +/// base: Region<0x1000>; +/// /// /// CPU core control. /// pub CPU_CTL(u32) @ CpuCtlBase + 0x10 { /// 0:0 start; @@ -579,6 +599,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// /// // Aliases can also be defined for relative register. /// register! { +/// base: Region<0x1000>; +/// /// /// Alias to CPU core control. /// pub CPU_CTL_ALIAS(u32) => CpuCtlBase + CPU_CTL { /// /// Start the aliased CPU core. @@ -621,15 +643,18 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// register, /// register::Array, /// Io, +/// Region, /// }, /// }; -/// # use kernel::io::{Mmio, Region}; +/// # use kernel::io::Mmio; /// # fn get_scratch_idx() -> usize { /// # 0x15 /// # } /// /// // Array of 64 consecutive registers with the same layout starting at offset `0x80`. /// register! { +/// base: Region<0x1000>; +/// /// /// Scratch registers. /// pub SCRATCH(u32)[64] @ 0x00000080 { /// 31:0 value; @@ -655,6 +680,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// // Alias to a specific register in an array. /// // Here `SCRATCH[8]` is used to convey the firmware exit code. /// register! { +/// base: Region<0x1000>; +/// /// /// Firmware exit status code. /// pub FIRMWARE_STATUS(u32) => SCRATCH[8] { /// 7:0 status; @@ -667,6 +694,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// // Here, each of the 16 registers of the array is separated by 8 bytes, meaning that the /// // registers of the two declarations below are interleaved. /// register! { +/// base: Region<0x1000>; +/// /// /// Scratch registers bank 0. /// pub SCRATCH_INTERLEAVED_0(u32)[16, stride = 8] @ 0x000000c0 { /// 31:0 value; @@ -688,6 +717,7 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// /// ```ignore /// register! { +/// ... /// pub RELATIVE_REGISTER_ARRAY(u8)[10, stride = 4] @ Base + 0x100 { /// ... /// } @@ -707,9 +737,10 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// WithBase, /// }, /// Io, +/// Region, /// }, /// }; -/// # use kernel::io::{Mmio, Region}; +/// # use kernel::io::Mmio; /// # fn get_scratch_idx() -> usize { /// # 0x15 /// # } @@ -731,6 +762,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// /// // 64 per-cpu scratch registers, arranged as a contiguous array. /// register! { +/// base: Region<0x1000>; +/// /// /// Per-CPU scratch registers. /// pub CPU_SCRATCH(u32)[64] @ CpuCtlBase + 0x00000080 { /// 31:0 value; @@ -758,6 +791,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// /// // Alias to `SCRATCH[8]` used to convey the firmware exit code. /// register! { +/// base: Region<0x1000>; +/// /// /// Per-CPU firmware exit status code. /// pub CPU_FIRMWARE_STATUS(u32) => CpuCtlBase + CPU_SCRATCH[8] { /// 7:0 status; @@ -768,6 +803,8 @@ fn into_io_op(self) -> (FixedRegisterLoc<T>, T) { /// // Here, each of the 16 registers of the array is separated by 8 bytes, meaning that the /// // registers of the two declarations below are interleaved. /// register! { +/// base: Region<0x1000>; +/// /// /// Scratch registers bank 0. /// pub CPU_SCRATCH_INTERLEAVED_0(u32)[16, stride = 8] @ CpuCtlBase + 0x00000d00 { /// 31:0 value; diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs index 8af12caa964b..2fb48e1be82d 100644 --- a/rust/macros/io/register.rs +++ b/rust/macros/io/register.rs @@ -27,6 +27,7 @@ }; mod kw { + syn::custom_keyword!(base); syn::custom_keyword!(stride); } @@ -139,16 +140,26 @@ fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> { } pub(crate) struct RegDef { + base: Option<Type>, regs: Vec<Reg>, } impl Parse for RegDef { fn parse(input: syn::parse::ParseStream<'_>) -> Result<Self> { + let base = if input.peek(kw::base) { + let _: kw::base = input.parse()?; + let _: Token![:] = input.parse()?; + let base = input.parse()?; + let _: Token![;] = input.parse()?; + Some(base) + } else { + None + }; let mut regs = Vec::new(); while !input.is_empty() { regs.push(input.parse()?); } - Ok(RegDef { regs }) + Ok(RegDef { base, regs }) } } -- 2.54.0
