Re: [PATCH v4 1/3] rust: pci: provide access to PCI Class and Class-related items

[John Hubbard <jhubbard@nvidia.com>]

On 8/20/25 4:51 AM, Danilo Krummrich wrote:
> On Wed Aug 20, 2025 at 5:08 AM CEST, John Hubbard wrote:
>> Allow callers to write Class::STORAGE_SCSI instead of
>> bindings::PCI_CLASS_STORAGE_SCSI, for example.
>>
>> New APIs:
>>      Class::STORAGE_SCSI, Class::NETWORK_ETHERNET, etc.
>>      Class::as_raw()
>>      Class: TryFrom<u32> for Class
>>      ClassMask: Full, ClassSubclass
>>      DeviceId::from_class_and_vendor()
>>      Device::pci_class()
>>
>> Cc: Danilo Krummrich <dakr@kernel.org>
>> Cc: Alexandre Courbot <acourbot@nvidia.com>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> +    /// Returns the PCI class as a `Class` struct.
>> +    /// Returns an error if the class code is not recognized.
>> +    pub fn pci_class(&self) -> Result<Class> {
>> +        // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>> +        Class::new(unsafe { (*self.as_raw()).class })
>> +    }
> 
> I think all this turned out very nice!
> 
> One thing to reconsider would be whether we really want this to be fallible.
> 
> It's probably better to define a pci::Class::UNKNOWN and implement
> 
> 	impl From<u32> for Class {
> 	    fn from(value: u32) -> Self {
> 	        match value {
> 	            $(x if x == Self::$variant.0 => Self::$variant,)+
> 	            _ => Self::UNKNOWN,
> 	        }
> 	    }
> 	}
> 
> instead.

Yes, I went back and forth on whether Class and Vendor should be
fallible, and finally settled on the wrong choice. haha :)


thanks,
-- 
John Hubbard