dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 10/10] gpu: nova-core: use conversion derives for two register enums
       [not found] <20260929135855.36775-1-chaoji_xinren@163.com>
@ 2026-09-29 13:58 ` Kaiqi Guo
       [not found] ` <20260929135855.36775-9-chaoji_xinren@163.com>
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Kaiqi Guo @ 2026-09-29 13:58 UTC (permalink / raw)
  To: rust-for-linux, Miguel Ojeda
  Cc: Jesung Yang, Alexandre Courbot, Danilo Krummrich, nova-gpu,
	dri-devel, linux-kernel, Kaiqi Guo, Alice Ryhl, David Airlie,
	Simona Vetter

Replace bounded_enum! for Architecture with TryFrom and Into using the
shared convert helper. Keep the six discriminants, Bounded<u32, 6> input
and output, and Error/EINVAL on invalid inputs.

Use From and Into for FalconCoreRevSubversion, which covers every value
of Bounded<u32, 2>. Its existing infallible conversion and blanket
TryFrom error type remain unchanged. Leave other register enums and
Chipset conversion behavior in place.

This exercises both fallible and exhaustive conversions in actual
register field users without changing register accesses or GPU
initialization logic.

Link: https://lore.kernel.org/rust-for-linux/DHHJCEG8BC47.2VC6GLDRRZH1B@nvidia.com/
Signed-off-by: Kaiqi Guo <chaoji_xinren@163.com>
---
 drivers/gpu/nova-core/falcon.rs | 19 +++++++++----------
 drivers/gpu/nova-core/gpu.rs    | 22 ++++++++++------------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 65cb12d26e2b..15bbbda7df28 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -59,16 +59,15 @@ pub(crate) enum FalconCoreRev with TryFrom<Bounded<u32, 4>> {
     }
 }
 
-bounded_enum! {
-    /// Revision subversion number of a falcon core, used in the
-    /// [`crate::regs::NV_PFALCON_FALCON_HWCFG1`] register.
-    #[derive(Debug, Copy, Clone)]
-    pub(crate) enum FalconCoreRevSubversion with From<Bounded<u32, 2>> {
-        Subversion0 = 0,
-        Subversion1 = 1,
-        Subversion2 = 2,
-        Subversion3 = 3,
-    }
+/// Revision subversion number of a falcon core, used in the
+/// [`crate::regs::NV_PFALCON_FALCON_HWCFG1`] register.
+#[derive(Debug, Copy, Clone, kernel::macros::From, kernel::macros::Into)]
+#[convert(Bounded<u32, 2>)]
+pub(crate) enum FalconCoreRevSubversion {
+    Subversion0 = 0,
+    Subversion1 = 1,
+    Subversion2 = 2,
+    Subversion3 = 3,
 }
 
 bounded_enum! {
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index fd1414004dd0..66249bb5b369 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -14,7 +14,6 @@
 };
 
 use crate::{
-    bounded_enum,
     driver::Bar0,
     falcon::{
         gsp::Gsp as GspFalcon,
@@ -155,17 +154,16 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     }
 }
 
-bounded_enum! {
-    /// Enum representation of the GPU generation.
-    #[derive(fmt::Debug, Copy, Clone)]
-    pub(crate) enum Architecture with TryFrom<Bounded<u32, 6>> {
-        Turing = 0x16,
-        Ampere = 0x17,
-        Hopper = 0x18,
-        Ada = 0x19,
-        BlackwellGB10x = 0x1a,
-        BlackwellGB20x = 0x1b,
-    }
+/// Enum representation of the GPU generation.
+#[derive(fmt::Debug, Copy, Clone, kernel::macros::TryFrom, kernel::macros::Into)]
+#[convert(Bounded<u32, 6>)]
+pub(crate) enum Architecture {
+    Turing = 0x16,
+    Ampere = 0x17,
+    Hopper = 0x18,
+    Ada = 0x19,
+    BlackwellGB10x = 0x1a,
+    BlackwellGB20x = 0x1b,
 }
 
 #[derive(Clone, Copy)]


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v6 08/10] rust: macros: test exhaustive conversion derives
       [not found] ` <20260929135855.36775-9-chaoji_xinren@163.com>
@ 2026-09-29 14:07   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-29 14:07 UTC (permalink / raw)
  To: Kaiqi Guo; +Cc: Danilo Krummrich, Alexandre Courbot, gary, dri-devel, ojeda

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: macros: non-vertical import formatting in doc tests
--

commit b361cafb4eb66d1f226f7d0ead63f4a791967d6c
Author: Kaiqi Guo <chaoji_xinren@163.com>

rust: macros: test exhaustive conversion derives

This commit adds extensive tests for From, Into, and TryFrom exhaustive
conversion derives. It covers bounded ranges, boolean, full-width primitives,
and verifies various compile-time error cases for incomplete domains and
invalid layouts.

> diff --git a/rust/macros/convert.rs b/rust/macros/convert.rs
> index 7fa8a22dc9133..c23b8819572e3 100644
> --- a/rust/macros/convert.rs
> +++ b/rust/macros/convert.rs
> @@ -1671,3 +1671,375 @@ mod overflow_assert_works_on_unsigned_int {}
>      /// ```
>      mod any_try_from_target_overflow_is_rejected {}
>  }
> +
> +mod derive_from_tests {
> +    /// ```
> +    /// use kernel::{macros::{From, Into}, num::Bounded};

[Severity: Low]
Does this import (and others in the added doc tests) violate the kernel's
vertical import style guidelines? The Rust subsystem guidelines require
vertical trailing-comma syntax for imports, but this uses horizontal grouping.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929135855.36775-1-chaoji_xinren@163.com?part=8

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v6 02/10] rust: macros: add derive macro for `TryFrom`
       [not found] ` <20260929135855.36775-3-chaoji_xinren@163.com>
@ 2026-09-29 14:10   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-29 14:10 UTC (permalink / raw)
  To: Kaiqi Guo; +Cc: Danilo Krummrich, gary, dri-devel, Alexandre Courbot, ojeda

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] rust: macros: unused_comparisons warning on 128-bit boundary checks
- [Low] rust: macros: dead_code and unused_variables on empty enums
--

commit 2f4a5e53f9efd805dce115afdbd130dde8b72a62
Author: Jesung Yang <y.j3ms.n@gmail.com>

rust: macros: add derive macro for `TryFrom`

Introduce a procedural macro TryFrom to automatically implement the TryFrom
trait for unit-only enums, reducing boilerplate when interpreting numeric
values as enum variants.

> diff --git a/rust/macros/convert.rs b/rust/macros/convert.rs
> index 6702c2e48c1fe..c64e35447859c 100644
> --- a/rust/macros/convert.rs
> +++ b/rust/macros/convert.rs

[ ... ]

> @@ -245,6 +253,56 @@ fn from(#param: #enum_ident) -> #input_ty {
>          }
>      }
>  
> +    fn impl_try_from(
> +        enum_ident: &Ident,
> +        variants: &[Ident],
> +        repr_ty: &syn::Path,
> +        input_ty: &ValidTy,
> +    ) -> TokenStream {
> +        let param = Ident::new("value", Span::call_site());

[Severity: Low]
Does this trigger an unused_variables warning if the user derives TryFrom on a
variant-less (empty) enum?

The generated try_from function never uses the value parameter in that case,
which could result in a build failure under CONFIG_WERROR.

> +
> +        let overflow_assertion = emit_overflow_assert(enum_ident, variants, repr_ty, input_ty);

[Severity: Low]
Can this generated overflow assertion cause a dead_code warning for empty enums?

When an enum is empty, no variant assertions are generated to invoke the
internal fits() helper function, leaving it unused.

Also, when deriving TryFrom for a 128-bit target type, does the generated
fits() function trigger the unused_comparisons lint?

For example, the bounds checks in emit_overflow_assert() generate comparisons
like this:

    (d as ::core::primitive::u128) <= (dst_max as ::core::primitive::u128)

If dst_max is the maximum possible value for a u128, the comparison is
trivially true and triggers the unused_comparisons lint. This can cause build
failures under CONFIG_WERROR.

Would it make sense to apply #[allow(unused_comparisons)] to the entire fits()
function scope rather than just the signedness boolean variables?

> +        let emit_cast = |variant| {
> +            let qualified_variant = ::quote::quote! { #enum_ident::#variant };
> +            match input_ty {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929135855.36775-1-chaoji_xinren@163.com?part=2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v6 01/10] rust: macros: add derive macro for `Into`
       [not found] ` <20260929135855.36775-2-chaoji_xinren@163.com>
@ 2026-09-29 14:10   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-29 14:10 UTC (permalink / raw)
  To: Kaiqi Guo; +Cc: ojeda, dri-devel, Danilo Krummrich, gary, Alexandre Courbot

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] rust: macros: Into derive fails on repr with nested arguments
- [Low] rust: macros: Bounded validation incorrectly accepts bool
--

commit f3bc2f572cd1e2c7a2b7b3fee89fc39e25420448
Author: Jesung Yang <y.j3ms.n@gmail.com>

rust: macros: add derive macro for Into

This commit introduces a procedural macro Into to automatically derive the Into
trait for unit-only enums. It generates From implementations based on the into
or repr attributes, supporting primitive types and the Bounded type. It also
generates compile-time assertions to ensure variant discriminants fit within
the destination type.

Link: https://lore.kernel.org/rust-for-linux/20250624132337.2242-1-dakr@kernel.org/ [1]

> diff --git a/rust/macros/convert.rs b/rust/macros/convert.rs
> new file mode 100644
> index 0000000000000..6702c2e48c1fe
> --- /dev/null
> +++ b/rust/macros/convert.rs
> @@ -0,0 +1,527 @@
[ ... ]
> +fn parse_attrs(
> +    target: DeriveTarget,
> +    attrs: &[Attribute],
> +) -> syn::Result<(Vec<Type>, bool, Option<Ident>)> {
> +    let helper = target.get_helper_name();
> +
> +    let mut is_repr_c = false;
> +    let mut repr_ty = None;
> +    let mut helper_tys = Vec::new();
> +    for attr in attrs {
> +        if attr.path().is_ident("repr") {
> +            attr.parse_nested_meta(|meta| {
> +                let ident = meta.path.get_ident();
> +                if let Some(i) = ident {
> +                    if is_valid_primitive(i) {
> +                        repr_ty = ident.cloned();
> +                    } else if i == "C" {
> +                        is_repr_c = true;
> +                    }
> +                }
> +                // Delegate `repr` attribute validation to rustc.
> +                Ok(())

[Severity: Low]
Does this closure returning Ok(()) without consuming the inner tokens cause
parsing to fail? 

For example, if a developer applies #[derive(Into)] to an enum with a
#[repr(align(4))] attribute, the inner token 4 is not consumed. Will this
cause the syn crate to return a parsing error and break the build?

> +            })?;
> +        } else if attr.path().is_ident(helper) {
> +            let args = attr.parse_args_with(Punctuated::<Type, Token![,]>::parse_terminated)?;
> +            helper_tys.extend(args);
> +        }
> +    }
> +
> +    Ok((helper_tys, is_repr_c, repr_ty))
> +}
[ ... ]
> +fn validate_bounded(path_segment: &PathSegment) -> syn::Result<ValidTy> {
> +    let PathSegment { ident, arguments } = path_segment;
> +    return match arguments {
> +        PathArguments::AngleBracketed(inner) if ident == Bounded::NAME => {
> +            let AngleBracketedGenericArguments {
> +                colon2_token, args, ..
> +            } = inner;
[ ... ]
> +            let Some(base_ty_ident) = base_ty_lowered.path.get_ident() else {
> +                return Err(make_base_ty_err(base_ty));
> +            };
> +
> +            if !is_valid_primitive(base_ty_ident) {

[Severity: Low]
Does this validation step incorrectly accept bool as a base type for Bounded?

Because is_valid_primitive() returns true for bool, specifying
Bounded<bool, 1> passes this check. 

Later in the code, emit_min() attempts to emit MIN and BITS properties:

rust/macros/convert.rs:emit_min() {
    ...
    ::quote::quote! { #base_ty::MIN >> (#base_ty::BITS - #bits) }
    ...
}

Since bool::MIN and bool::BITS do not exist, will this generate invalid code
that causes the build to fail?

> +                return Err(make_base_ty_err(base_ty));
> +            }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929135855.36775-1-chaoji_xinren@163.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v6 06/10] rust: macros: exercise the intended conversion doctest failures
       [not found] ` <20260929135855.36775-7-chaoji_xinren@163.com>
@ 2026-09-29 18:47   ` Miguel Ojeda
  0 siblings, 0 replies; 5+ messages in thread
From: Miguel Ojeda @ 2026-09-29 18:47 UTC (permalink / raw)
  To: Kaiqi Guo
  Cc: rust-for-linux, Miguel Ojeda, Jesung Yang, Alexandre Courbot,
	Danilo Krummrich, nova-gpu, dri-devel, linux-kernel,
	Charalampos Mitrodimas

On Tue, Sep 29, 2026 at 3:59 PM Kaiqi Guo <chaoji_xinren@163.com> wrote:
>
> Suggested-by: Charalampos Mitrodimas <charmitro@posteo.net>
>
> Link: https://lore.kernel.org/rust-for-linux/m2qzr81jrd.fsf@ip-192-168-1-196.ap-southeast-1.compute.internal/

I have been seeing more and more lately these newlines between tags --
is there perhaps a tutorial out there mentioning to do this or a
script that ends up splitting them or similar?

It is especially strange in cases like this one, because the Link is
for the Suggested-by, no? So logically they should be tied together
(if the kernel allowed newlines to group tags to begin with).

By the way, since I am here: it sounds like this could be split into
different patches, although it is not a big deal.

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-30 12:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260929135855.36775-1-chaoji_xinren@163.com>
2026-09-29 13:58 ` [PATCH v6 10/10] gpu: nova-core: use conversion derives for two register enums Kaiqi Guo
     [not found] ` <20260929135855.36775-9-chaoji_xinren@163.com>
2026-09-29 14:07   ` [PATCH v6 08/10] rust: macros: test exhaustive conversion derives sashiko-bot
     [not found] ` <20260929135855.36775-3-chaoji_xinren@163.com>
2026-09-29 14:10   ` [PATCH v6 02/10] rust: macros: add derive macro for `TryFrom` sashiko-bot
     [not found] ` <20260929135855.36775-2-chaoji_xinren@163.com>
2026-09-29 14:10   ` [PATCH v6 01/10] rust: macros: add derive macro for `Into` sashiko-bot
     [not found] ` <20260929135855.36775-7-chaoji_xinren@163.com>
2026-09-29 18:47   ` [PATCH v6 06/10] rust: macros: exercise the intended conversion doctest failures Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox