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

[John Hubbard]

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@xxxxxxxxxx>
> > Cc: Alexandre Courbot <acourbot@xxxxxxxxxx>
> > Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> > +    /// 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