* [PATCH 0/2] gpu: nova-core: convert to kernel bitfield macro and remove local variant
@ 2026-06-17 14:02 Alexandre Courbot
2026-06-17 14:02 ` [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro Alexandre Courbot
2026-06-17 14:02 ` [PATCH 2/2] gpu: nova-core: remove local " Alexandre Courbot
0 siblings, 2 replies; 5+ messages in thread
From: Alexandre Courbot @ 2026-06-17 14:02 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Onur Özkan
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
Now that the kernel-wide bitfield macro is available, it is time for the
Nova-local version to be removed.
The first patch converts all uses of the local macro to the global one.
This involves redefining all bitfields since the syntax has changed, and
taking into account the use of `Bounded`.
The second patch removes the now unused Nova-local bitfield macro.
This is a follow-up to [1] containing the Nova patches that have been
dropped after [2].
This series applies on top of a merge of `drm-rust-next` and today's
`master`. It is to be applied after `-rc1` is tagged and merged into
`drm-rust-next`.
[1] https://lore.kernel.org/all/20260606-bitfield-v5-0-b92188820914@nvidia.com/
[2] https://lore.kernel.org/all/20260527-bitfield-v4-0-e8821d4efbde@nvidia.com/
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Alexandre Courbot (2):
gpu: nova-core: convert to kernel bitfield macro
gpu: nova-core: remove local bitfield macro
drivers/gpu/nova-core/bitfield.rs | 329 -------------------------------------
drivers/gpu/nova-core/fsp.rs | 8 +-
drivers/gpu/nova-core/gsp/fw.rs | 11 +-
drivers/gpu/nova-core/mctp.rs | 86 +++++-----
drivers/gpu/nova-core/nova_core.rs | 3 -
5 files changed, 56 insertions(+), 381 deletions(-)
---
base-commit: 31686e06c0342de47f40a0278bfe8126de86c3ec
change-id: 20260617-nova-bitfield-be4bbb298a00
Best regards,
--
Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro
2026-06-17 14:02 [PATCH 0/2] gpu: nova-core: convert to kernel bitfield macro and remove local variant Alexandre Courbot
@ 2026-06-17 14:02 ` Alexandre Courbot
2026-06-17 14:11 ` sashiko-bot
2026-06-22 5:23 ` Eliot Courtney
2026-06-17 14:02 ` [PATCH 2/2] gpu: nova-core: remove local " Alexandre Courbot
1 sibling, 2 replies; 5+ messages in thread
From: Alexandre Courbot @ 2026-06-17 14:02 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Onur Özkan
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
Replace uses of the Nova-local `bitfield!` macro with the kernel one.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/nova-core/fsp.rs | 8 +++-
drivers/gpu/nova-core/gsp/fw.rs | 11 ++---
drivers/gpu/nova-core/mctp.rs | 86 +++++++++++++++++++-------------------
drivers/gpu/nova-core/nova_core.rs | 3 --
4 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
index d949c03dd304..e8becf11a321 100644
--- a/drivers/gpu/nova-core/fsp.rs
+++ b/drivers/gpu/nova-core/fsp.rs
@@ -11,6 +11,7 @@
device,
dma::Coherent,
io::poll::read_poll_timeout,
+ num::TryIntoBounded,
prelude::*,
ptr::{
Alignable,
@@ -297,7 +298,12 @@ fn send_sync_fsp<M>(&mut self, dev: &device::Device, bar: Bar0<'_>, msg: &M) ->
return Err(EIO);
}
- if command_nvdm_type != u8::from(M::NVDM_TYPE).into() {
+ if command_nvdm_type
+ .try_into_bounded()
+ .ok_or(EINVAL)
+ .and_then(NvdmType::try_from)?
+ != M::NVDM_TYPE
+ {
dev_err!(
dev,
"Expected NVDM type {:?} in reply, got {:#x}\n",
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 4db0cfa4dc4d..5a8392b33b4a 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -10,6 +10,7 @@
use core::ops::Range;
use kernel::{
+ bitfield,
dma::Coherent,
prelude::*,
ptr::{
@@ -742,8 +743,8 @@ unsafe impl AsBytes for MsgqRxHeader {}
bitfield! {
struct MsgHeaderVersion(u32) {
- 31:24 major as u8;
- 23:16 minor as u8;
+ 31:24 major;
+ 23:16 minor;
}
}
@@ -752,9 +753,9 @@ impl MsgHeaderVersion {
const MINOR_TOT: u8 = 0;
fn new() -> Self {
- Self::default()
- .set_major(Self::MAJOR_TOT)
- .set_minor(Self::MINOR_TOT)
+ Self::zeroed()
+ .with_major(Self::MAJOR_TOT)
+ .with_minor(Self::MINOR_TOT)
}
}
diff --git a/drivers/gpu/nova-core/mctp.rs b/drivers/gpu/nova-core/mctp.rs
index 482786e07bc7..acc2abbd4b0c 100644
--- a/drivers/gpu/nova-core/mctp.rs
+++ b/drivers/gpu/nova-core/mctp.rs
@@ -7,55 +7,51 @@
//! Data Model) messages between the kernel driver and GPU firmware processors
//! such as FSP and GSP.
-use kernel::pci::Vendor;
+use kernel::{
+ bitfield,
+ pci::Vendor,
+ prelude::*, //
+};
-/// NVDM message type identifiers carried over MCTP.
-#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
-#[repr(u8)]
-pub(crate) enum NvdmType {
- #[default]
- /// Chain of Trust boot message.
- Cot = 0x14,
- /// FSP command response.
- FspResponse = 0x15,
-}
+use crate::{
+ bounded_enum,
+ num, //
+};
-impl TryFrom<u8> for NvdmType {
- type Error = u8;
-
- fn try_from(value: u8) -> Result<Self, Self::Error> {
- match value {
- x if x == u8::from(Self::Cot) => Ok(Self::Cot),
- x if x == u8::from(Self::FspResponse) => Ok(Self::FspResponse),
- _ => Err(value),
- }
- }
-}
-
-impl From<NvdmType> for u8 {
- fn from(value: NvdmType) -> Self {
- value as u8
+bounded_enum! {
+ /// NVDM message type identifiers carried over MCTP.
+ #[derive(Debug, Clone, Copy, PartialEq, Eq)]
+ pub(crate) enum NvdmType with TryFrom<Bounded<u32, 8>> {
+ /// Chain of Trust boot message.
+ Cot = 0x14,
+ /// FSP command response.
+ FspResponse = 0x15,
}
}
bitfield! {
- pub(crate) struct MctpHeader(u32), "MCTP transport header for NVIDIA firmware messages." {
- 31:31 som as bool, "Start-of-message bit.";
- 30:30 eom as bool, "End-of-message bit.";
- 29:28 seq as u8, "Packet sequence number.";
- 23:16 seid as u8, "Source endpoint ID.";
+ /// MCTP transport header for NVIDIA firmware messages.
+ pub(crate) struct MctpHeader(u32) {
+ /// Start-of-message bit.
+ 31:31 som;
+ /// End-of-message bit.
+ 30:30 eom;
+ /// Packet sequence number.
+ 29:28 seq;
+ /// Source endpoint ID.
+ 23:16 seid;
}
}
impl MctpHeader {
/// Builds a single-packet MCTP header (`SOM=1`, `EOM=1`, `SEQ=0`, `SEID=0`).
pub(crate) fn single_packet() -> Self {
- Self::default().set_som(true).set_eom(true)
+ Self::zeroed().with_som(true).with_eom(true)
}
/// Returns whether this is a complete single-packet message (`SOM=1` and `EOM=1`).
pub(crate) fn is_single_packet(self) -> bool {
- self.som() && self.eom()
+ self.som().into_bool() && self.eom().into_bool()
}
}
@@ -63,26 +59,30 @@ pub(crate) fn is_single_packet(self) -> bool {
const MSG_TYPE_VENDOR_PCI: u8 = 0x7e;
bitfield! {
- pub(crate) struct NvdmHeader(u32), "NVIDIA Vendor-Defined Message header over MCTP." {
- 31:24 nvdm_type as u8 ?=> NvdmType, "NVDM message type.";
- 23:8 vendor_id as u16, "PCI vendor ID.";
- 6:0 msg_type as u8, "MCTP vendor-defined message type.";
+ /// NVIDIA Vendor-Defined Message header over MCTP.
+ pub(crate) struct NvdmHeader(u32) {
+ /// NVDM message type.
+ 31:24 nvdm_type ?=> NvdmType;
+ /// PCI vendor ID.
+ 23:8 vendor_id;
+ /// MCTP vendor-defined message type.
+ 6:0 msg_type;
}
}
impl NvdmHeader {
/// Builds an NVDM header for the given message type.
pub(crate) fn new(nvdm_type: NvdmType) -> Self {
- Self::default()
- .set_msg_type(MSG_TYPE_VENDOR_PCI)
- .set_vendor_id(Vendor::NVIDIA.as_raw())
- .set_nvdm_type(nvdm_type)
+ Self::zeroed()
+ .with_const_msg_type::<{ num::u8_as_u32(MSG_TYPE_VENDOR_PCI) }>()
+ .with_vendor_id(Vendor::NVIDIA.as_raw())
+ .with_nvdm_type(nvdm_type)
}
/// Validates this header against the expected NVIDIA NVDM format and type.
pub(crate) fn validate(self, expected_type: NvdmType) -> bool {
- self.msg_type() == MSG_TYPE_VENDOR_PCI
- && self.vendor_id() == Vendor::NVIDIA.as_raw()
+ u8::from(self.msg_type()) == MSG_TYPE_VENDOR_PCI
+ && u16::from(self.vendor_id()) == Vendor::NVIDIA.as_raw()
&& matches!(self.nvdm_type(), Ok(nvdm_type) if nvdm_type == expected_type)
}
}
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 735b8e17c6b6..a61406ba5c0b 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -10,9 +10,6 @@
InPlaceModule, //
};
-#[macro_use]
-mod bitfield;
-
mod driver;
mod falcon;
mod fb;
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] gpu: nova-core: remove local bitfield macro
2026-06-17 14:02 [PATCH 0/2] gpu: nova-core: convert to kernel bitfield macro and remove local variant Alexandre Courbot
2026-06-17 14:02 ` [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro Alexandre Courbot
@ 2026-06-17 14:02 ` Alexandre Courbot
1 sibling, 0 replies; 5+ messages in thread
From: Alexandre Courbot @ 2026-06-17 14:02 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Onur Özkan
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
This module is now orphaned code, superseded by a kernel-global
implementation, so remove it.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Acked-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/nova-core/bitfield.rs | 329 --------------------------------------
1 file changed, 329 deletions(-)
diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs
deleted file mode 100644
index 660c3911402d..000000000000
--- a/drivers/gpu/nova-core/bitfield.rs
+++ /dev/null
@@ -1,329 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-//! Bitfield library for Rust structures
-//!
-//! Support for defining bitfields in Rust structures. Also used by the [`register!`] macro.
-
-/// Defines a struct with accessors to access bits within an inner unsigned integer.
-///
-/// # Syntax
-///
-/// ```rust
-/// use nova_core::bitfield;
-///
-/// #[derive(Debug, Clone, Copy, Default)]
-/// enum Mode {
-/// #[default]
-/// Low = 0,
-/// High = 1,
-/// Auto = 2,
-/// }
-///
-/// impl TryFrom<u8> for Mode {
-/// type Error = u8;
-/// fn try_from(value: u8) -> Result<Self, Self::Error> {
-/// match value {
-/// 0 => Ok(Mode::Low),
-/// 1 => Ok(Mode::High),
-/// 2 => Ok(Mode::Auto),
-/// _ => Err(value),
-/// }
-/// }
-/// }
-///
-/// impl From<Mode> for u8 {
-/// fn from(mode: Mode) -> u8 {
-/// mode as u8
-/// }
-/// }
-///
-/// #[derive(Debug, Clone, Copy, Default)]
-/// enum State {
-/// #[default]
-/// Inactive = 0,
-/// 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! {
-/// pub struct ControlReg(u32) {
-/// 7:7 state as bool => State;
-/// 3:0 mode as u8 ?=> Mode;
-/// }
-/// }
-/// ```
-///
-/// This generates a struct with:
-/// - Field accessors: `mode()`, `state()`, etc.
-/// - Field setters: `set_mode()`, `set_state()`, etc. (supports chaining with builder pattern).
-/// Note that the compiler will error out if the size of the setter's arg exceeds the
-/// struct's storage size.
-/// - Debug and Default implementations.
-///
-/// Note: Field accessors and setters inherit the same visibility as the struct itself.
-/// In the example above, both `mode()` and `set_mode()` methods will be `pub`.
-///
-/// Fields are defined as follows:
-///
-/// - `as <type>` simply returns the field value casted to <type>, typically `u32`, `u16`, `u8` or
-/// `bool`. Note that `bool` fields must have a range of 1 bit.
-/// - `as <type> => <into_type>` calls `<into_type>`'s `From::<<type>>` implementation and returns
-/// the result.
-/// - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
-/// and returns the result. This is useful with fields for which not all values are valid.
-macro_rules! bitfield {
- // Main entry point - defines the bitfield struct with fields
- ($vis:vis struct $name:ident($storage:ty) $(, $comment:literal)? { $($fields:tt)* }) => {
- bitfield!(@core $vis $name $storage $(, $comment)? { $($fields)* });
- };
-
- // All rules below are helpers.
-
- // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
- // `Default`, and conversion to the value type) and field accessor methods.
- (@core $vis:vis $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
- $(
- #[doc=$comment]
- )?
- #[repr(transparent)]
- #[derive(Clone, Copy)]
- $vis struct $name($storage);
-
- impl ::core::convert::From<$name> for $storage {
- fn from(val: $name) -> $storage {
- val.0
- }
- }
-
- bitfield!(@fields_dispatcher $vis $name $storage { $($fields)* });
- };
-
- // Captures the fields and passes them to all the implementers that require field information.
- //
- // Used to simplify the matching rules for implementers, so they don't need to match the entire
- // complex fields rule even though they only make use of part of it.
- (@fields_dispatcher $vis:vis $name:ident $storage:ty {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- bitfield!(@field_accessors $vis $name $storage {
- $(
- $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- )*
- });
- bitfield!(@debug $name { $($field;)* });
- bitfield!(@default $name { $($field;)* });
- };
-
- // Defines all the field getter/setter methods for `$name`.
- (
- @field_accessors $vis:vis $name:ident $storage:ty {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- $(
- bitfield!(@check_field_bounds $hi:$lo $field as $type);
- )*
-
- #[allow(dead_code)]
- impl $name {
- $(
- bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- );
- )*
- }
- };
-
- // Boolean fields must have `$hi == $lo`.
- (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
- #[allow(clippy::eq_op)]
- const _: () = {
- ::kernel::build_assert::build_assert!(
- $hi == $lo,
- concat!("boolean field `", stringify!($field), "` covers more than one bit")
- );
- };
- };
-
- // Non-boolean fields must have `$hi >= $lo`.
- (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
- #[allow(clippy::eq_op)]
- const _: () = {
- ::kernel::build_assert::build_assert!(
- $hi >= $lo,
- concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB")
- );
- };
- };
-
- // Catches fields defined as `bool` and convert them into a boolean value.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
- => $into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(
- @leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$into_type>::from(f != 0) }
- bool $into_type => $into_type $(, $comment)?;
- );
- };
-
- // Shortcut for fields defined as `bool` without the `=>` syntax.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
- $(, $comment:literal)?;
- ) => {
- bitfield!(
- @field_accessor $vis $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;
- );
- };
-
- // Catches the `?=>` syntax for non-boolean fields.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- ?=> $try_into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$try_into_type>::try_from(f as $type) } $type $try_into_type =>
- ::core::result::Result<
- $try_into_type,
- <$try_into_type as ::core::convert::TryFrom<$type>>::Error
- >
- $(, $comment)?;);
- };
-
- // Catches the `=>` syntax for non-boolean fields.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- => $into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$into_type>::from(f as $type) } $type $into_type => $into_type $(, $comment)?;);
- };
-
- // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- $(, $comment:literal)?;
- ) => {
- bitfield!(
- @field_accessor $vis $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;
- );
- };
-
- // Generates the accessor methods for a single field.
- (
- @leaf_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
- { $process:expr } $prim_type:tt $to_type:ty => $res_type:ty $(, $comment:literal)?;
- ) => {
- ::kernel::macros::paste!(
- 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)
- }
-
- ::kernel::macros::paste!(
- $(
- #[doc="Sets the value of this field:"]
- #[doc=$comment]
- )?
- #[inline(always)]
- $vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
- const MASK: $storage = $name::[<$field:upper _MASK>];
- const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- let value = ($storage::from($prim_type::from(value)) << SHIFT) & MASK;
- self.0 = (self.0 & !MASK) | value;
-
- self
- }
- );
- };
-
- // Generates the `Debug` implementation for `$name`.
- (@debug $name:ident { $($field:ident;)* }) => {
- impl ::kernel::fmt::Debug for $name {
- fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
- f.debug_struct(stringify!($name))
- .field("<raw>", &::kernel::prelude::fmt!("{:#x}", &self.0))
- $(
- .field(stringify!($field), &self.$field())
- )*
- .finish()
- }
- }
- };
-
- // Generates the `Default` implementation for `$name`.
- (@default $name:ident { $($field:ident;)* }) => {
- /// Returns a value for the bitfield where all fields are set to their default value.
- impl ::core::default::Default for $name {
- fn default() -> Self {
- let value = Self(Default::default());
-
- ::kernel::macros::paste!(
- $(
- let value = value.[<set_ $field>](Default::default());
- )*
- );
-
- value
- }
- }
- };
-}
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro
2026-06-17 14:02 ` [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro Alexandre Courbot
@ 2026-06-17 14:11 ` sashiko-bot
2026-06-22 5:23 ` Eliot Courtney
1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-06-17 14:11 UTC (permalink / raw)
To: Alexandre Courbot; +Cc: dri-devel, Danilo Krummrich, ojeda
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Refactored FSP message validation incorrectly bubbles up enum conversion errors via `?`, bypassing diagnostic logging and changing the return code from `-EIO` to `-EINVAL`.
--
commit affc1bd88bbaf7649b7fc1c329019739603b4580
Author: Alexandre Courbot <acourbot@nvidia.com>
gpu: nova-core: convert to kernel bitfield macro
This commit replaces uses of the Nova-local bitfield! macro with the
kernel one.
> diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
> index 8fc243c66e356..91885b355d462 100644
> --- a/drivers/gpu/nova-core/fsp.rs
> +++ b/drivers/gpu/nova-core/fsp.rs
[ ... ]
> @@ -274,7 +275,12 @@ fn send_sync_fsp<M>(&mut self, dev: &device::Device, bar: Bar0<'_>, msg: &M) ->
> return Err(EIO);
> }
>
> - if command_nvdm_type != u8::from(M::NVDM_TYPE).into() {
> + if command_nvdm_type
> + .try_into_bounded()
> + .ok_or(EINVAL)
> + .and_then(NvdmType::try_from)?
> + != M::NVDM_TYPE
> + {
> dev_err!(
> dev,
> "Expected NVDM type {:?} in reply, got {:#x}\n",
[Severity: Medium]
Does the ? operator here prematurely return EINVAL if the firmware returns an
unknown or invalid NVDM type?
It looks like if NvdmType::try_from fails, this will immediately propagate the
error out of send_sync_fsp(). Does this bypass the dev_err! diagnostic block
below that was intended to log the unexpected hardware value?
By returning early here, this also appears to alter the original error
reporting semantics by returning EINVAL instead of the expected EIO for a
device failure.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-nova-bitfield-v1-0-ec10c6772666@nvidia.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro
2026-06-17 14:02 ` [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro Alexandre Courbot
2026-06-17 14:11 ` sashiko-bot
@ 2026-06-22 5:23 ` Eliot Courtney
1 sibling, 0 replies; 5+ messages in thread
From: Eliot Courtney @ 2026-06-22 5:23 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Daniel Almeida, Tamir Duberstein, Onur Özkan
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, dri-devel, linux-kernel, rust-for-linux,
dri-devel
On Wed Jun 17, 2026 at 11:02 PM JST, Alexandre Courbot wrote:
> Replace uses of the Nova-local `bitfield!` macro with the kernel one.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Acked-by: Danilo Krummrich <dakr@kernel.org>
> ---
Modulo the comment raised by sashiko about skipping the log on error,
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-22 5:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-17 14:02 [PATCH 0/2] gpu: nova-core: convert to kernel bitfield macro and remove local variant Alexandre Courbot
2026-06-17 14:02 ` [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro Alexandre Courbot
2026-06-17 14:11 ` sashiko-bot
2026-06-22 5:23 ` Eliot Courtney
2026-06-17 14:02 ` [PATCH 2/2] gpu: nova-core: remove local " Alexandre Courbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox