Re: [PATCH] gpu: nova-core: bitfield: fix broken Default implementation

[Gary Guo]

On Tue Mar 31, 2026 at 8:56 AM BST, Eliot Courtney wrote:
> The current implementation does not actually set the default values for
> the fields in the bitfield. Set it and add a test.
>
> Signed-off-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
> ---
> It doesn't actually set the default values for each field. I know that
> bitfields are being moved soon, so maybe this fix is unnecessary but I
> saw it and it's a simple fix.

Good catch! This is why I think it's important to use `with_` for this sort of
methods as they are confusingly non-mutating :)

Could you add a Fixes tag?

Best,
Gary

> ---
>  drivers/gpu/nova-core/bitfield.rs | 50 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs
> index 16e143658c51..22d68fc21a1f 100644
> --- a/drivers/gpu/nova-core/bitfield.rs
> +++ b/drivers/gpu/nova-core/bitfield.rs
> @@ -4,6 +4,8 @@
>  //!
>  //! Support for defining bitfields in Rust structures. Also used by the [`register!`] macro.
>  
> +use kernel::prelude::*;
> +
>  /// Defines a struct with accessors to access bits within an inner unsigned integer.
>  ///
>  /// # Syntax
> @@ -314,12 +316,11 @@ fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
>          /// Returns a value for the bitfield where all fields are set to their default value.
>          impl ::core::default::Default for $name {
>              fn default() -> Self {
> -                #[allow(unused_mut)]
> -                let mut value = Self(Default::default());
> +                let value = Self(Default::default());
>  
>                  ::kernel::macros::paste!(
>                  $(
> -                value.[<set_ $field>](Default::default());
> +                let value = value.[<set_ $field>](Default::default());
>                  )*
>                  );
>  
> @@ -328,3 +329,46 @@ fn default() -> Self {
>          }
>      };
>  }
> +
> +#[kunit_tests(nova_core_bitfield)]
> +mod tests {
> +    use super::*;
> +
> +    #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
> +    enum State {
> +        Inactive = 0,
> +        #[default]
> +        Active = 1,
> +    }
> +
> +    impl From<bool> for State {
> +        fn from(value: bool) -> Self {
> +            if value {
> +                State::Active
> +            } else {
> +                State::Inactive
> +            }
> +        }
> +    }
> +
> +    impl From<State> for bool {
> +        fn from(state: State) -> bool {
> +            match state {
> +                State::Inactive => false,
> +                State::Active => true,
> +            }
> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestBitfield(u32) {
> +            0:0 state as bool => State;
> +        }
> +    }
> +
> +    #[test]
> +    fn default_impl() -> Result {
> +        assert_eq!(TestBitfield::default().state(), State::Active);
> +        Ok(())
> +    }
> +}
>
> ---
> base-commit: 7c50d748b4a635bc39802ea3f6b120e66b1b9067
> change-id: 20260331-fix-bitfield-a03f6d1b9e00
>
> Best regards,
> --  
> Eliot Courtney <ecourtney@xxxxxxxxxx>