On 02/11/2025 04:00, Alexandre Courbot wrote:
On Sun Nov 2, 2025 at 3:51 AM JST, Dirk Behme wrote:
On 09.10.25 13:28, Alexandre Courbot wrote:
On Thu Oct 9, 2025 at 8:16 PM JST, Danilo Krummrich wrote:
On Thu Oct 9, 2025 at 8:59 AM CEST, Dirk Behme wrote:
Assuming that register.rs is supposed to become the "generic" way to
access hardware registers I started to have a look to it. Some weeks
back testing interrupts I used some quite simple timer with 4 registers
[1]. Now, thinking about converting it to register!() I have three
understanding / usage questions:
* At the moment register!() is for 32-bit registers, only? So it can't
be used for my example having 8-bit and 16-bit registers as well?
Yes, currently the register!() macro always generates a 32-bit register type
(mainly because nova-core did not need anything else). However, this will of
course be generalized (which should be pretty straight forward).
Having a brief look at the TMU datasheet it looks like you should be able to
treat TSTR and TCR as 32-bit registers without any issues for testing the
register!() macro today. I.e. you can just define it as:
register!(TSTR @ 0x04, "Timer Start Register" {
2:2 str2 as bool, "Specifies whether TCNT2 is operated or
stopped.";
1:1 str1 as bool, "Specifies whether TCNT1 is operated or
stopped.";
0:0 str0 as bool, "Specifies whether TCNT0 is operated or
stopped.";
});
Same for TCR.
Patch 2 of this series actually adds support for 16 and 8 bit register
storage.
Hmm, how to use that with the register!() macro? I mean patch 2 adds
support for different storage widths for *bitfields*. But looking at
patch 4 [2] it looks like *register!()* still uses $name(u32)? With
that it looks like that the register!() macro still just supports 32
bit registers? Or what have I missed?
What I'm looking for is a way to specify if a register is 8, 16 or 32
bit. Using the example from above something like
register!(TSTR<u8> @ ....
Errr indeed, you are correct. The `register` macro's syntax has not been
updated to take advantage of `bitfield`'s storage types, and `u32` is
still hardcoded as of this series.
This looks like an oversight - a register is basically a bitfield with
some I/O, so making it support storage types should be trivial. I guess
this hasn't been done yet because Nova is the only user so far, and we
don't need/want to explicitly specify a type for each register since
they are invariably `u32`.
But it wouldn't look good to change the syntax of `register` after
moving it out, so I agree this should take place before the move. The
same applies to the visiblity feature.
One way to avoid a update all the declarations so far would be to give
Nova its own `register` macro that invokes the one in `kernel` with
the relevant parameters hardcoded.
Just fyi, hacking something like [1] below does work for my (very
limited) use case. And it defaults `register!` without type to <u32>.
Thanks!
Dirk
[1]
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -276,33 +276,38 @@ pub trait RegisterBase<T> {
/// ```
#[macro_export]
macro_rules! register {
- // Creates a register at a fixed offset of the MMIO space.
+ // Creates a register at a fixed offset of the MMIO space, defaults
to u32 if no type is specified.
($name:ident @ $offset:literal $(, $comment:literal)? {
$($fields:tt)* } ) => {
- ::kernel::bitfield!(pub(crate) struct $name(u32) $(, $comment)?
{ $($fields)* } );
- register!(@io_fixed $name @ $offset);
+ register!($name<u32> @ $offset $(, $comment)? { $($fields)* });
+ };
+
+ // Creates a register at a fixed offset of the MMIO space, explicit
type required.
+ ($name:ident<$ty:ty> @ $offset:literal $(, $comment:literal)? {
$($fields:tt)* } ) => {
+ ::kernel::bitfield!(pub(crate) struct $name($ty) $(, $comment)?
{ $($fields)* } );
+ register!(@io_fixed<$ty> $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with
its own fields.
- ($name:ident => $alias:ident $(, $comment:literal)? {
$($fields:tt)* } ) => {
- ::kernel::bitfield!(pub(crate) struct $name(u32) $(, $comment)?
{ $($fields)* } );
- register!(@io_fixed $name @ $alias::OFFSET);
+ ($name:ident<$ty:ty> => $alias:ident $(, $comment:literal)? {
$($fields:tt)* } ) => {
+ ::kernel::bitfield!(pub(crate) struct $name($ty) $(, $comment)?
{ $($fields)* } );
+ register!(@io_fixed<$ty> $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address
provider.
- ($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)?
{ $($fields:tt)* } ) => {
- ::kernel::bitfield!(pub(crate) struct $name(u32) $(, $comment)?
{ $($fields)* } );
- register!(@io_relative $name @ $base [ $offset ]);
+ ($name:ident<$ty:ty> @ $base:ty [ $offset:literal ] $(,
$comment:literal)? { $($fields:tt)* } ) => {
+ ::kernel::bitfield!(pub(crate) struct $name($ty) $(, $comment)?
{ $($fields)* } );
+ register!(@io_relative<$ty> $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias`
with its own fields.
- ($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? {
$($fields:tt)* }) => {
- ::kernel::bitfield!(pub(crate) struct $name(u32) $(, $comment)?
{ $($fields)* } );
- register!(@io_relative $name @ $base [ $alias::OFFSET ]);
+ ($name:ident<$ty:ty> => $base:ty [ $alias:ident ] $(,
$comment:literal)? { $($fields:tt)* }) => {
+ ::kernel::bitfield!(pub(crate) struct $name($ty) $(, $comment)?
{ $($fields)* } );
+ register!(@io_relative<$ty> $name @ $base [ $alias::OFFSET ]);
};
- // Creates an array of registers at a fixed offset of the MMIO space.
+ // Creates an array of registers at a fixed offset of the MMIO
space. (u32 only for now)
(
- $name:ident @ $offset:literal [ $size:expr ; $stride:expr ] $(,
$comment:literal)? {
+ $name:ident<u32> @ $offset:literal [ $size:expr ; $stride:expr
] $(, $comment:literal)? {
$($fields:tt)*
}
) => {
@@ -311,20 +316,20 @@ macro_rules! register {
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
- // Shortcut for contiguous array of registers (stride == size of
element).
+ // Shortcut for contiguous array of registers (stride == size of
element). (u32 only for now)
(
- $name:ident @ $offset:literal [ $size:expr ] $(,
$comment:literal)? {
+ $name:ident<u32> @ $offset:literal [ $size:expr ] $(,
$comment:literal)? {
$($fields:tt)*
}
) => {
- register!($name @ $offset [ $size ;
::core::mem::size_of::<u32>() ] $(, $comment)? {
+ register!($name<u32> @ $offset [ $size ;
::core::mem::size_of::<u32>() ] $(, $comment)? {
$($fields)*
} );
};
- // Creates an array of registers at a relative offset from a base
address provider.
+ // Creates an array of registers at a relative offset from a base
address provider. (u32 only for now)
(
- $name:ident @ $base:ty [ $offset:literal [ $size:expr ;
$stride:expr ] ]
+ $name:ident<u32> @ $base:ty [ $offset:literal [ $size:expr ;
$stride:expr ] ]
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
@@ -332,20 +337,19 @@ macro_rules! register {
register!(@io_relative_array $name @ $base [ $offset [ $size ;
$stride ] ]);
};
- // Shortcut for contiguous array of relative registers (stride ==
size of element).
+ // Shortcut for contiguous array of relative registers (stride ==
size of element). (u32 only for now)
(
- $name:ident @ $base:ty [ $offset:literal [ $size:expr ] ] $(,
$comment:literal)? {
+ $name:ident<u32> @ $base:ty [ $offset:literal [ $size:expr ] ]
$(, $comment:literal)? {
$($fields:tt)*
}
) => {
- register!($name @ $base [ $offset [ $size ;
::core::mem::size_of::<u32>() ] ]
+ register!($name<u32> @ $base:ty [ $offset:literal [ $size:expr
; ::core::mem::size_of::<u32>() ] ]
$(, $comment)? { $($fields)* } );
};
- // Creates an alias of register `idx` of relative array of
registers `alias` with its own
- // fields.
+ // Creates an alias of register `idx` of relative array of
registers `alias` with its own fields. (u32 only for now)
(
- $name:ident => $base:ty [ $alias:ident [ $idx:expr ] ] $(,
$comment:literal)? {
+ $name:ident<u32> => $base:ty [ $alias:ident [ $idx:expr ] ] $(,
$comment:literal)? {
$($fields:tt)*
}
) => {
@@ -354,17 +358,15 @@ macro_rules! register {
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx *
$alias::STRIDE ] );
};
- // Creates an alias of register `idx` of array of registers `alias`
with its own fields.
- // This rule belongs to the (non-relative) register arrays set, but
needs to be put last
- // to avoid it being interpreted in place of the relative register
array alias rule.
- ($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? {
$($fields:tt)* }) => {
+ // Creates an alias of register `idx` of array of registers `alias`
with its own fields. (u32 only for now)
+ ($name:ident<u32> => $alias:ident [ $idx:expr ] $(,
$comment:literal)? { $($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
::kernel::bitfield!(pub(crate) struct $name(u32) $(,
$comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx *
$alias::STRIDE );
};
- // Generates the IO accessors for a fixed offset register.
- (@io_fixed $name:ident @ $offset:expr) => {
+ // Generates the IO accessors for a fixed offset register, using
the type parameter.
+ (@io_fixed<$ty:ty> $name:ident @ $offset:expr) => {
#[allow(dead_code)]
impl $name {
pub(crate) const OFFSET: usize = $offset;
@@ -374,7 +376,15 @@ impl $name {
pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
{
- Self(io.read32($offset))
+ Self(
+ if core::any::TypeId::of::<$ty>() ==
core::any::TypeId::of::<u8>() {
+ io.read8($offset) as $ty
+ } else if core::any::TypeId::of::<$ty>() ==
core::any::TypeId::of::<u16>() {
+ io.read16($offset) as $ty
+ } else {
+ io.read32($offset) as $ty
+ }
+ )
}
/// Write the value contained in `self` to the register
address in `io`.
@@ -382,7 +392,13 @@ pub(crate) fn read<const SIZE: usize, T>(io: &T) ->
Self where
pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
{
- io.write32(self.0, $offset)
+ if core::any::TypeId::of::<$ty>() ==
core::any::TypeId::of::<u8>() {
+ io.write8(self.0 as u8, $offset)
+ } else if core::any::TypeId::of::<$ty>() ==
core::any::TypeId::of::<u16>() {
+ io.write16(self.0 as u16, $offset)
+ } else {
+ io.write32(self.0 as u32, $offset)
+ }
}
/// Read the register from its address in `io` and run `f`
on its value to obtain a new
@@ -401,8 +417,8 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
}
};
- // Generates the IO accessors for a relative offset register.
- (@io_relative $name:ident @ $base:ty [ $offset:expr ]) => {
+ // Generates the IO accessors for a relative offset register, using
the type parameter.
+ (@io_relative<$ty:ty> $name:ident @ $base:ty [ $offset:expr ]) => {
#[allow(dead_code)]
impl $name {
pub(crate) const OFFSET: usize = $offset;
@@ -420,9 +436,7 @@ pub(crate) fn read<const SIZE: usize, T, B>(
{
const OFFSET: usize = $name::OFFSET;
- let value = io.read32(
- <B as
::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET
- );
+ let value = io.read::<$ty>(<B as
::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET);
Self(value)
}
@@ -441,10 +455,7 @@ pub(crate) fn write<const SIZE: usize, T, B>(
{
const OFFSET: usize = $name::OFFSET;
- io.write32(
- self.0,
- <B as
::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET
- );
+ io.write::<$ty>(self.0, <B as
::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET);
}
/// Read the register from `io`, using the base address
provided by `base` and adding
--
2.48.0
