Re: [PATCH 1/2] gpu: nova-core: impl From for u32 for enums used from register!

[Danilo Krummrich <dakr@kernel.org>]

On Tue, Jun 24, 2025 at 10:48:57PM +0900, Alexandre Courbot wrote:
> On Tue Jun 24, 2025 at 10:23 PM JST, Danilo Krummrich wrote:
> > Implement From for u32 for all enum types used within the register!()
> > macro.
> >
> > This avoids a conflict with [1] as reported in [2].
> >
> > Cc: Alexandre Courbot <acourbot@nvidia.com>
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Link: https://lore.kernel.org/r/20250615-ptr-as-ptr-v12-5-f43b024581e8@gmail.com [1]
> > Link: https://lore.kernel.org/all/20250624173114.3be38990@canb.auug.org.au/ [2]
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> 
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> 
> Also confirmed that Ampere still successfully probed with this:
> 
> Tested-by: Alexandre Courbot <acourbot@nvidia.com>
> 
> One small comment and question below.
> 
> > ---
> >  drivers/gpu/nova-core/falcon.rs | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> > index 07be1c30668c..1affffb109ec 100644
> > --- a/drivers/gpu/nova-core/falcon.rs
> > +++ b/drivers/gpu/nova-core/falcon.rs
> > @@ -20,6 +20,16 @@
> >  mod hal;
> >  pub(crate) mod sec2;
> >  
> > +macro_rules! impl_from_enum_to_u32 {
> > +    ($enum_type:ty) => {
> > +        impl From<$enum_type> for u32 {
> > +            fn from(value: $enum_type) -> Self {
> > +                value as u32
> > +            }
> > +        }
> > +    };
> > +}
> 
> We might need some equivalent in other modules as well in the future -
> do you think we should move it to the root of the driver, and explain
> its purpose with a comment?

Let's do that once we need it in other modules.

> I am also thinking that we might want to turn this into a derive macro
> in the future, similar to `FromPrimitive` - so maybe a TODO item is
> warranted for this. I haven't looked closely at the `FromPrimitive`
> patchset yet but it would be neat if we could leverage it somehow.

This sounds exactly like what IntoPrimitive does. So, yes, I can add a comment
to replace the macro with IntoPrimitive in the future.