On 8/19/25 2:09 AM, Danilo Krummrich wrote:
On Tue Aug 19, 2025 at 5:11 AM CEST, John Hubbard wrote:
+/// PCI device class codes. Each entry contains the full 24-bit PCI
+/// class code (base class in bits 23-16, subclass in bits 15-8,
+/// programming interface in bits 7-0).
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::{device::Core, pci::{self, Class}, prelude::*};
+/// fn probe_device(pdev: &pci::Device<Core>) -> Result<()> {
+/// // Get the PCI class for this device
+/// let pci_class = pdev.pci_class();
+/// dev_info!(
+/// pdev.as_ref(),
+/// "Detected PCI class: (0x{:06x})\n",
+/// pci_class.as_u32()
+/// );
Maybe a bit cleaner to implement Display for pci::Class?
OK, will do.
+/// Ok(())
+/// }
+/// ```
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(transparent)]
+pub struct Class(u32);
[ Class impl and lots of pci class ids... ]
I think we should move all this to a new Rust module (rust/kernel/pci/class.rs)
to keep this file reasonably small.
You can add
use self::class::Class;
use self::class::ClassMask;
in this file to make it appear as e.g. kernel::pci::Class.
OK. In patch #3 you suggested combining with Vendor into an id.rs,
which seems like a good layout and naming system, I'll do that.
Sorry I didn't mention this in the previous version.
/// An adapter for the registration of PCI drivers.
pub struct Adapter<T: Driver>(T);
@@ -157,6 +355,23 @@ pub const fn from_class(class: u32, class_mask: u32) -> Self {
override_only: 0,
})
}
+
+ /// Create a new `pci::DeviceId` from a class number, mask, and specific vendor.
+ ///
+ /// This is more targeted than [`DeviceId::from_class`]: in addition to matching by Vendor, it
+ /// also matches the PCI Class (up to the entire 24 bits, depending on the mask).
+ pub const fn from_class_and_vendor(class: Class, class_mask: u32, vendor: u32) -> Self {
I think it would be good if class_mask would be a new type ClassMask that only
has the constants that are applicable for this field, i.e. MASK_FULL and
MASK_CLASS_SUBCLASS.
Yes, good idea, that will lock it down. We only ever want those two
mask choices here.
+ Self(bindings::pci_device_id {
+ vendor,
+ device: DeviceId::PCI_ANY_ID,
+ subvendor: DeviceId::PCI_ANY_ID,
+ subdevice: DeviceId::PCI_ANY_ID,
+ class: class.as_u32(),
+ class_mask,
+ driver_data: 0,
+ override_only: 0,
+ })
+ }
}
// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `pci_device_id` and does not add
@@ -410,6 +625,18 @@ pub fn resource_len(&self, bar: u32) -> Result<bindings::resource_size_t> {
// - by its type invariant `self.as_raw` is always a valid pointer to a `struct pci_dev`.
Ok(unsafe { bindings::pci_resource_len(self.as_raw(), bar.try_into()?) })
}
+
+ /// Returns the full 24-bit PCI class code as stored in hardware.
+ /// This includes base class, subclass, and programming interface.
+ pub fn pci_class_code_raw(&self) -> u32 {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
+ unsafe { (*self.as_raw()).class }
+ }
Do we need this method? I think drivers can just call pdev.pci_class().as_u32()
instead (which we could also name as_raw()).
Sounds good.
+ /// Returns the PCI class as a `Class` struct.
+ pub fn pci_class(&self) -> Class {
+ Class(self.pci_class_code_raw())
+ }
This is good! At a first glance the name looks a bit odd or redundant, but
people would clearly expect something different when this is called as
pdev.class() (i.e. a struct class representation).
Thanks, that's exactly the reasoning I used, too.
thanks,
--
John Hubbard
