linux-kernel - Re: [PATCH v2 1/2] gpu: nova-core: merge the Revision type into the Spec type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ce6366ed-4a9a-4a8d-9207-f1da487dca62@nvidia.com>
Date: Tue, 28 Oct 2025 10:25:33 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Alexandre Courbot <acourbot@...dia.com>,
 Joel Fernandes <joelagnelf@...dia.com>, Timur Tabi <ttabi@...dia.com>,
 Alistair Popple <apopple@...dia.com>, Edwin Peer <epeer@...dia.com>,
 Zhi Wang <zhiw@...dia.com>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Bjorn Helgaas <bhelgaas@...gle.com>,
 Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
 nouveau@...ts.freedesktop.org, rust-for-linux@...r.kernel.org,
 LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] gpu: nova-core: merge the Revision type into the
 Spec type

On 10/28/25 5:25 AM, Danilo Krummrich wrote:
> On Tue Oct 28, 2025 at 3:39 AM CET, John Hubbard wrote:
>> -pub(crate) struct Revision {
>> -    major: u8,
>> -    minor: u8,
>> -}
>> -
>> -impl Revision {
>> -    fn from_boot0(boot0: regs::NV_PMC_BOOT_0) -> Self {
>> -        Self {
>> -            major: boot0.major_revision(),
>> -            minor: boot0.minor_revision(),
>> -        }
>> -    }
>> -}
>> -
>> -impl fmt::Display for Revision {
>> -    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>> -        write!(f, "{:x}.{:x}", self.major, self.minor)
>> -    }
>> -}
>> -
>> -/// Structure holding the metadata of the GPU.
>> +/// Structure holding a basic description of the GPU: Architecture, Chipset and Revision.
>>   pub(crate) struct Spec {
>>       chipset: Chipset,
>> -    /// The revision of the chipset.
>> -    revision: Revision,
>> +    major_revision: u8,
>> +    minor_revision: u8,
>>   }
> 
> Why not keep the Revision type and its Display impl as well?

I just felt like it's not quite pulling its weight. But clearly
that feeling is not widely shared, so I'll put it back in. :)

> 
>>   
>>   impl Spec {
>> @@ -162,11 +142,25 @@ fn new(bar: &Bar0) -> Result<Spec> {
>>   
>>           Ok(Self {
>>               chipset: boot0.chipset()?,
>> -            revision: Revision::from_boot0(boot0),
>> +            major_revision: boot0.major_revision(),
>> +            minor_revision: boot0.minor_revision(),
>>           })
>>       }
>>   }
>>   
>> +impl fmt::Display for Spec {
>> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>> +        write!(
>> +            f,
>> +            "Chipset: {}, Architecture: {:?}, Revision: {:x}.{:x}",
>> +            self.chipset,
>> +            self.chipset.arch(),
>> +            self.major_revision,
>> +            self.minor_revision
>> +        )
> 
> This could just be:
> 
> 	write!(
> 	    f,
> 	    "Chipset: {}, Architecture: {:?}, Revision: {}",
> 	    self.chipset,
> 	    self.chipset.arch(),
> 	    self.revision,
> 	)
> 

Yes. That is nicer.

thanks,
John Hubbard