>>>
>>> Is there any reason why you replace the UPPERCASE register names with
>>> CamelCase ones?
>>>
>>> I was under the impression that we want to use UPPERCASE for register
>>> names. Like in nova
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/nova-core/regs.rs
>> Not really. UPPERCASE for non-const items will trigger the linter. The Nova
>> people chose to #[allow] this to align with OpenRM and, IIRC from the LPC
>> discussions, their registers are automatically generated from some internal
>> docs.
>> We have only a few, we can simply convert them to CamelCase.
>
>
> I'm under the impression that we define the "future RFL register!() style
> standard" here.
>
> So we want to make the CamelCase the default? And nova is the exception?
I’m not sure I would say this. It’s just that you would hit this lint
[0]. If UPPER_CASE was the “default", we would have to have the #[allow] on
every driver.
>
> I'm fine with that. Just want to make sure we talked about it :)
>
>
> ....
>>>> pub(crate) const MCU_CONTROL_ENABLE: u32 = 1;
>>>> pub(crate) const MCU_CONTROL_AUTO: u32 = 2;
>>>> pub(crate) const MCU_CONTROL_DISABLE: u32 = 0;
>>>>
>>>> -pub(crate) const MCU_STATUS: Register<0x704> = Register;
>>>> +register!(McuControl @ 0x700, "Controls the execution state of the MCU
>>>> subsystem" {
>>>> + 1:0 req as u32, "Request state change";
>>>> +});
>>>
>>>
>>> Any reason why req is a u32 and not a u8? Same for some other places.
>> I tend to default to u32/i32 in general, as that’s usually the native
>> machine integer type.
>> All we get from smaller types is a spam of `into()`, `from()` and their
>> `try_`
>> equivalents. When stored in a struct, they usually do not save space due to
>> padding that is usually inserted to fix the alignment for the type. IMHO not
>> worth it unless it really matters. Correct me if I'm wrong, but it doesn't
>> seem
>> to be the case here.
>
>
> Wouldn't using u8 prevent any accidental access to 31:8 ?
The macro is doing the appropriate masking according to the bit ranges you pass
in (i.e.: 31:8), not according to the type (u8 or u32), i.e.:
const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> =
$lo..=$hi; <------
const [<$field:upper _MASK>]: $storage = {
// Generate mask for shifting
match ::core::mem::size_of::<$storage>() {
1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
_ => ::kernel::build_error!("Unsupported storage type size")
}
};
const [<$field:upper _SHIFT>]: u32 = $lo;
);
$(
#[doc="Returns the value of this field:"]
#[doc=$comment]
)?
#[inline(always)]
$vis fn $field(self) -> $res_type {
::kernel::macros::paste!(
const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
);
let field = ((self.0 & MASK) >> SHIFT);
$process(field)
}
>
>
> Best regards
>
> Dirk
[0]:
https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/nonstandard_style/static.NON_CAMEL_CASE_TYPES.html