Derive Clone and Copy for Revision and Spec. Both are small
value types (4 bytes total) and Copy makes them easier to use
in later patches that pass them across function boundaries.

Add a Spec::chipset() accessor for use by later patches that
need the chipset outside of gpu.rs.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/gpu.rs | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 3b4ccc3d18b9..3cd7709883be 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -184,6 +184,7 @@ fn from(value: Architecture) -> Self {
     }
 }
 
+#[derive(Clone, Copy)]
 pub(crate) struct Revision {
     major: u8,
     minor: u8,
@@ -205,6 +206,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 }
 
 /// Structure holding a basic description of the GPU: `Chipset` and `Revision`.
+#[derive(Clone, Copy)]
 pub(crate) struct Spec {
     chipset: Chipset,
     revision: Revision,
@@ -241,6 +243,11 @@ fn new(dev: &device::Device, bar: &Bar0) -> Result<Spec> {
             dev_err!(dev, "Unsupported chipset: {}\n", boot42);
         })
     }
+
+    /// Returns this GPU's chipset.
+    pub(crate) fn chipset(&self) -> Chipset {
+        self.chipset
+    }
 }
 
 impl TryFrom<regs::NV_PMC_BOOT_42> for Spec {
@@ -291,10 +298,9 @@ pub(crate) fn new<'a>(
     ) -> impl PinInit<Self, Error> + 'a {
         try_pin_init!(Self {
             spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
-                dev_info!(pdev,"NVIDIA ({})\n", spec);
+                dev_info!(pdev, "NVIDIA ({})\n", spec);
             })?,
 
-            // We must wait for GFW_BOOT completion before doing any significant setup on the GPU.
             _: {
                 gfw::wait_gfw_boot_completion(bar)
                     .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?;
-- 
2.53.0

On Wed Mar 25, 2026 at 3:52 AM GMT, John Hubbard wrote:
> Derive Clone and Copy for Revision and Spec. Both are small
> value types (4 bytes total) and Copy makes them easier to use
> in later patches that pass them across function boundaries.
>
> Add a Spec::chipset() accessor for use by later patches that
> need the chipset outside of gpu.rs.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  drivers/gpu/nova-core/gpu.rs | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 3b4ccc3d18b9..3cd7709883be 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -184,6 +184,7 @@ fn from(value: Architecture) -> Self {
>      }
>  }
>  
> +#[derive(Clone, Copy)]
>  pub(crate) struct Revision {
>      major: u8,
>      minor: u8,
> @@ -205,6 +206,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>  }
>  
>  /// Structure holding a basic description of the GPU: `Chipset` and `Revision`.
> +#[derive(Clone, Copy)]
>  pub(crate) struct Spec {
>      chipset: Chipset,
>      revision: Revision,
> @@ -241,6 +243,11 @@ fn new(dev: &device::Device, bar: &Bar0) -> Result<Spec> {
>              dev_err!(dev, "Unsupported chipset: {}\n", boot42);
>          })
>      }
> +
> +    /// Returns this GPU's chipset.
> +    pub(crate) fn chipset(&self) -> Chipset {

Given that you mark the type as `Copy`, this could be `self`.

Best,
Gary

> +        self.chipset
> +    }
>  }
>  
>  impl TryFrom<regs::NV_PMC_BOOT_42> for Spec {
> @@ -291,10 +298,9 @@ pub(crate) fn new<'a>(
>      ) -> impl PinInit<Self, Error> + 'a {
>          try_pin_init!(Self {
>              spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
> -                dev_info!(pdev,"NVIDIA ({})\n", spec);
> +                dev_info!(pdev, "NVIDIA ({})\n", spec);
>              })?,
>  
> -            // We must wait for GFW_BOOT completion before doing any significant setup on the GPU.
>              _: {
>                  gfw::wait_gfw_boot_completion(bar)
>                      .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?;

On 3/25/26 8:42 AM, Gary Guo wrote:
> On Wed Mar 25, 2026 at 3:52 AM GMT, John Hubbard wrote:
>> Derive Clone and Copy for Revision and Spec. Both are small
>> value types (4 bytes total) and Copy makes them easier to use
>> in later patches that pass them across function boundaries.
>>
>> Add a Spec::chipset() accessor for use by later patches that
>> need the chipset outside of gpu.rs.
>>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/gpu.rs | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index 3b4ccc3d18b9..3cd7709883be 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -184,6 +184,7 @@ fn from(value: Architecture) -> Self {
>>      }
>>  }
>>  
>> +#[derive(Clone, Copy)]
>>  pub(crate) struct Revision {
>>      major: u8,
>>      minor: u8,
>> @@ -205,6 +206,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>>  }
>>  
>>  /// Structure holding a basic description of the GPU: `Chipset` and `Revision`.
>> +#[derive(Clone, Copy)]
>>  pub(crate) struct Spec {
>>      chipset: Chipset,
>>      revision: Revision,
>> @@ -241,6 +243,11 @@ fn new(dev: &device::Device, bar: &Bar0) -> Result<Spec> {
>>              dev_err!(dev, "Unsupported chipset: {}\n", boot42);
>>          })
>>      }
>> +
>> +    /// Returns this GPU's chipset.
>> +    pub(crate) fn chipset(&self) -> Chipset {
> 
> Given that you mark the type as `Copy`, this could be `self`.
> 

Fixed, thanks.

thanks,
-- 
John Hubbard

> 
>> +        self.chipset
>> +    }
>>  }
>>  
>>  impl TryFrom<regs::NV_PMC_BOOT_42> for Spec {
>> @@ -291,10 +298,9 @@ pub(crate) fn new<'a>(
>>      ) -> impl PinInit<Self, Error> + 'a {
>>          try_pin_init!(Self {
>>              spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
>> -                dev_info!(pdev,"NVIDIA ({})\n", spec);
>> +                dev_info!(pdev, "NVIDIA ({})\n", spec);
>>              })?,
>>  
>> -            // We must wait for GFW_BOOT completion before doing any significant setup on the GPU.
>>              _: {
>>                  gfw::wait_gfw_boot_completion(bar)
>>                      .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?;
>

On Tue, 24 Mar 2026 20:52:15 -0700, John Hubbard <jhubbard@nvidia.com> wrote:
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 3b4ccc3d18b9..3cd7709883be 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -291,10 +298,9 @@ pub(crate) fn new<'a>(
>      ) -> impl PinInit<Self, Error> + 'a {
>          try_pin_init!(Self {
>              spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
> -                dev_info!(pdev,"NVIDIA ({})\n", spec);
> +                dev_info!(pdev, "NVIDIA ({})\n", spec);

This doesn't belong to this patch - let's apply this when we actually
touch this part of the code. The next patch removes this line, so that's
the right place to fix formatting.

>              })?,
>  
> -            // We must wait for GFW_BOOT completion before doing any significant setup on the GPU.

Same here, why remove this in this patch?

-- 
Alexandre Courbot <acourbot@nvidia.com>

On 3/25/26 3:47 AM, Alexandre Courbot wrote:
> On Tue, 24 Mar 2026 20:52:15 -0700, John Hubbard <jhubbard@nvidia.com> wrote:
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index 3b4ccc3d18b9..3cd7709883be 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -291,10 +298,9 @@ pub(crate) fn new<'a>(
>>      ) -> impl PinInit<Self, Error> + 'a {
>>          try_pin_init!(Self {
>>              spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
>> -                dev_info!(pdev,"NVIDIA ({})\n", spec);
>> +                dev_info!(pdev, "NVIDIA ({})\n", spec);
> 
> This doesn't belong to this patch - let's apply this when we actually
> touch this part of the code. The next patch removes this line, so that's
> the right place to fix formatting.
> 
>>              })?,
>>  
>> -            // We must wait for GFW_BOOT completion before doing any significant setup on the GPU.
> 
> Same here, why remove this in this patch?
> 

Fixed both, thanks.

thanks,
-- 
John Hubbard