* [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support
@ 2025-08-26 23:12 John Hubbard
2025-08-26 23:12 ` [PATCH v7 1/6] rust: pci: provide access to PCI Class and Class-related items John Hubbard
` (6 more replies)
0 siblings, 7 replies; 16+ messages in thread
From: John Hubbard @ 2025-08-26 23:12 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, John Hubbard, Elle Rhumsaa
Changes since v6:
* Applied changes from Danilo's and Alex's and Elle's reviews (thanks!):
* Rebased onto driver-core-next, which is here:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
* Changed pci::Vendor to be a u16, instead of a u32.
* Inlined all of the tiniest functions.
* Changed from Class/Vendor new(), to from_raw().
* Made from_raw() only accessible to super, which in this case is
the pci module.
* Restored infallible operations. That causes Alex's request for the
following reasonable behavior to work once again:
from_raw(0x10de).as_raw() == 0x10de
* Added a new patch, to inline the remaining PCI operations. This
provides consistent inline choices throughout pci.rs.
Changes since v5:
* Applied changes from Danilo's review (thanks!):
* Split the nova-core patch into two patches, for nova and pci.
* Added rust/kernel/pci/ to MAINTAINERS.
Changes since v4:
* Applied changes from Danilo's and Alex's review (thanks!):
* Reorganized the patches so that the Nova changes consume the
results of Class and Vendor upgrades, all in one shot.
* Made Class and Vendor types get constructed infallibly.
* This was all somewhat disruptive, and also required one more patch
in order to properly separate the various steps. But I think it is
all correct now. And CLIPPY=1 builds cleanly too.
* Elle Rhumsaa provided a Reviewed-by for v4 (thanks!), but due to the
churn in v5 here, I thought it best to not add that tag to v5 yet.
Instead, I have directly Cc'd Elle on the patches for now.
Changes since v3:
* Applied changes from Danilo's review (thanks!):
* Moved Class and Vendor to a new pci/id.rs file.
* Added ClassMask, to constrain callers to use only the two valid
masks.
* Removed pci_class_code_raw()
* Changed Class and Vendor .as_u32() to .as_raw(), because after
Danilo's comment I looked around rust/kernel and learned that
.as_raw() is the overwhelmingly used convention.
* Changed vendor_id() to return a Vendor instance directly.
* Also, validated Vendor during construction, just as is done
with Class. Both of these items are expected to match known
values, even for new devices, so that's a reasonable move.
Changes since v2:
* Applied changes from Danilo's and Alex's review (thanks!):
* Moved everything possible out of the new define_all_pci_classes!()
and define_all_pci_vendors!() macros.
* Used "impl TryFrom<u32> for Class/Vendor", instead of .from_u32().
* Made the new DeviceId methods infallible.
* Upgraded DeviceId::from_id() to accept a Vendor struct.
* Changed the names to be a little clearer:
* class_code_raw() --> pci_class_code_raw()
* class_enum() --> pci_class()
* Added doctests for the items that are not yet used in real drivers.
v2 is here:
https://lore.kernel.org/20250818013305.1089446-1-jhubbard@nvidia.com
Changes since v1:
1) Use the pci_device_table for filtering, instead of open-coding
filters in the .probe() callback.
2) Add PCI Class (class, subclass, implementation) and PCI Vendor to
Rust for Linux.
3) Rebased onto the latest nova-next branch, which is here:
https://gitlab.freedesktop.org/drm/nova.git
v1 is here:
https://lore.kernel.org/20250813232859.224316-1-jhubbard@nvidia.com
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Elle Rhumsaa <elle@weathered-steel.dev>
John Hubbard (6):
rust: pci: provide access to PCI Class and Class-related items
rust: pci: provide access to PCI Vendor values
rust: pci: add DeviceId::from_class_and_vendor() method
gpu: nova-core: avoid probing non-display/compute PCI functions
rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_*
rust: pci: inline several tiny functions
MAINTAINERS | 1 +
drivers/gpu/nova-core/driver.rs | 33 +-
rust/kernel/pci.rs | 55 ++-
rust/kernel/pci/id.rs | 577 ++++++++++++++++++++++++++
samples/rust/rust_dma.rs | 6 +-
samples/rust/rust_driver_auxiliary.rs | 12 +-
samples/rust/rust_driver_pci.rs | 9 +-
7 files changed, 665 insertions(+), 28 deletions(-)
create mode 100644 rust/kernel/pci/id.rs
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
--
2.51.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 1/6] rust: pci: provide access to PCI Class and Class-related items
2025-08-26 23:12 [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support John Hubbard
@ 2025-08-26 23:12 ` John Hubbard
2025-08-26 23:12 ` [PATCH v7 2/6] rust: pci: provide access to PCI Vendor values John Hubbard
` (5 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: John Hubbard @ 2025-08-26 23:12 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, John Hubbard, Elle Rhumsaa
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::from_raw() -- Only callable from pci module.
Class::as_raw()
ClassMask: Full, ClassSubclass
Device::pci_class()
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
MAINTAINERS | 1 +
rust/kernel/pci.rs | 11 ++
rust/kernel/pci/id.rs | 232 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 244 insertions(+)
create mode 100644 rust/kernel/pci/id.rs
diff --git a/MAINTAINERS b/MAINTAINERS
index fe168477caa4..28e200fa9423 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19532,6 +19532,7 @@ C: irc://irc.oftc.net/linux-pci
T: git git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
F: rust/helpers/pci.c
F: rust/kernel/pci.rs
+F: rust/kernel/pci/
F: samples/rust/rust_driver_pci.rs
PCIE BANDWIDTH CONTROLLER
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 887ee611b553..212c4a6834fb 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -23,6 +23,10 @@
};
use kernel::prelude::*;
+mod id;
+
+pub use self::id::{Class, ClassMask};
+
/// An adapter for the registration of PCI drivers.
pub struct Adapter<T: Driver>(T);
@@ -410,6 +414,13 @@ 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 PCI class as a `Class` struct.
+ #[inline]
+ pub fn pci_class(&self) -> Class {
+ // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
+ Class::from_raw(unsafe { (*self.as_raw()).class })
+ }
}
impl Device<device::Bound> {
diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
new file mode 100644
index 000000000000..55d9cdcc6658
--- /dev/null
+++ b/rust/kernel/pci/id.rs
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! PCI device identifiers and related types.
+//!
+//! This module contains PCI class codes and supporting types.
+
+use crate::{bindings, error::code::EINVAL, error::Error, prelude::*};
+use core::fmt;
+
+/// 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: {}\n",
+/// pci_class
+/// );
+/// Ok(())
+/// }
+/// ```
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(transparent)]
+pub struct Class(u32);
+
+/// PCI class mask constants for matching class codes.
+#[repr(u32)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ClassMask {
+ /// Match the full 24-bit class code.
+ Full = 0xffffff,
+ /// Match the upper 16 bits of the class code (base class and subclass only)
+ ClassSubclass = 0xffff00,
+}
+
+macro_rules! define_all_pci_classes {
+ (
+ $($variant:ident = $binding:expr,)+
+ ) => {
+
+ impl Class {
+ $(
+ #[allow(missing_docs)]
+ pub const $variant: Self = Self(Self::to_24bit_class($binding));
+ )+
+ }
+ };
+}
+
+/// Once constructed, a `Class` contains a valid PCI Class code.
+impl Class {
+ /// Create a Class from a raw 24-bit class code.
+ /// Only accessible from the parent pci module.
+ #[inline]
+ pub(super) fn from_raw(class_code: u32) -> Self {
+ Self(class_code)
+ }
+
+ /// Get the raw 24-bit class code value.
+ #[inline]
+ pub const fn as_raw(self) -> u32 {
+ self.0
+ }
+
+ // Converts a PCI class constant to 24-bit format.
+ //
+ // Many device drivers use only the upper 16 bits (base class and subclass), but some
+ // use the full 24 bits. In order to support both cases, store the class code as a 24-bit
+ // value, where 16-bit values are shifted up 8 bits.
+ const fn to_24bit_class(val: u32) -> u32 {
+ if val > 0xFFFF {
+ val
+ } else {
+ val << 8
+ }
+ }
+}
+
+impl fmt::Display for Class {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ write!(f, "0x{:06x}", self.0)
+ }
+}
+
+impl ClassMask {
+ /// Get the raw mask value.
+ #[inline]
+ pub const fn as_raw(self) -> u32 {
+ self as u32
+ }
+}
+
+impl TryFrom<u32> for ClassMask {
+ type Error = Error;
+
+ fn try_from(value: u32) -> Result<Self, Self::Error> {
+ match value {
+ 0xffffff => Ok(ClassMask::Full),
+ 0xffff00 => Ok(ClassMask::ClassSubclass),
+ _ => Err(EINVAL),
+ }
+ }
+}
+
+define_all_pci_classes! {
+ NOT_DEFINED = bindings::PCI_CLASS_NOT_DEFINED, // 0x000000
+ NOT_DEFINED_VGA = bindings::PCI_CLASS_NOT_DEFINED_VGA, // 0x000100
+
+ STORAGE_SCSI = bindings::PCI_CLASS_STORAGE_SCSI, // 0x010000
+ STORAGE_IDE = bindings::PCI_CLASS_STORAGE_IDE, // 0x010100
+ STORAGE_FLOPPY = bindings::PCI_CLASS_STORAGE_FLOPPY, // 0x010200
+ STORAGE_IPI = bindings::PCI_CLASS_STORAGE_IPI, // 0x010300
+ STORAGE_RAID = bindings::PCI_CLASS_STORAGE_RAID, // 0x010400
+ STORAGE_SATA = bindings::PCI_CLASS_STORAGE_SATA, // 0x010600
+ STORAGE_SATA_AHCI = bindings::PCI_CLASS_STORAGE_SATA_AHCI, // 0x010601
+ STORAGE_SAS = bindings::PCI_CLASS_STORAGE_SAS, // 0x010700
+ STORAGE_EXPRESS = bindings::PCI_CLASS_STORAGE_EXPRESS, // 0x010802
+ STORAGE_OTHER = bindings::PCI_CLASS_STORAGE_OTHER, // 0x018000
+
+ NETWORK_ETHERNET = bindings::PCI_CLASS_NETWORK_ETHERNET, // 0x020000
+ NETWORK_TOKEN_RING = bindings::PCI_CLASS_NETWORK_TOKEN_RING, // 0x020100
+ NETWORK_FDDI = bindings::PCI_CLASS_NETWORK_FDDI, // 0x020200
+ NETWORK_ATM = bindings::PCI_CLASS_NETWORK_ATM, // 0x020300
+ NETWORK_OTHER = bindings::PCI_CLASS_NETWORK_OTHER, // 0x028000
+
+ DISPLAY_VGA = bindings::PCI_CLASS_DISPLAY_VGA, // 0x030000
+ DISPLAY_XGA = bindings::PCI_CLASS_DISPLAY_XGA, // 0x030100
+ DISPLAY_3D = bindings::PCI_CLASS_DISPLAY_3D, // 0x030200
+ DISPLAY_OTHER = bindings::PCI_CLASS_DISPLAY_OTHER, // 0x038000
+
+ MULTIMEDIA_VIDEO = bindings::PCI_CLASS_MULTIMEDIA_VIDEO, // 0x040000
+ MULTIMEDIA_AUDIO = bindings::PCI_CLASS_MULTIMEDIA_AUDIO, // 0x040100
+ MULTIMEDIA_PHONE = bindings::PCI_CLASS_MULTIMEDIA_PHONE, // 0x040200
+ MULTIMEDIA_HD_AUDIO = bindings::PCI_CLASS_MULTIMEDIA_HD_AUDIO, // 0x040300
+ MULTIMEDIA_OTHER = bindings::PCI_CLASS_MULTIMEDIA_OTHER, // 0x048000
+
+ MEMORY_RAM = bindings::PCI_CLASS_MEMORY_RAM, // 0x050000
+ MEMORY_FLASH = bindings::PCI_CLASS_MEMORY_FLASH, // 0x050100
+ MEMORY_CXL = bindings::PCI_CLASS_MEMORY_CXL, // 0x050200
+ MEMORY_OTHER = bindings::PCI_CLASS_MEMORY_OTHER, // 0x058000
+
+ BRIDGE_HOST = bindings::PCI_CLASS_BRIDGE_HOST, // 0x060000
+ BRIDGE_ISA = bindings::PCI_CLASS_BRIDGE_ISA, // 0x060100
+ BRIDGE_EISA = bindings::PCI_CLASS_BRIDGE_EISA, // 0x060200
+ BRIDGE_MC = bindings::PCI_CLASS_BRIDGE_MC, // 0x060300
+ BRIDGE_PCI_NORMAL = bindings::PCI_CLASS_BRIDGE_PCI_NORMAL, // 0x060400
+ BRIDGE_PCI_SUBTRACTIVE = bindings::PCI_CLASS_BRIDGE_PCI_SUBTRACTIVE, // 0x060401
+ BRIDGE_PCMCIA = bindings::PCI_CLASS_BRIDGE_PCMCIA, // 0x060500
+ BRIDGE_NUBUS = bindings::PCI_CLASS_BRIDGE_NUBUS, // 0x060600
+ BRIDGE_CARDBUS = bindings::PCI_CLASS_BRIDGE_CARDBUS, // 0x060700
+ BRIDGE_RACEWAY = bindings::PCI_CLASS_BRIDGE_RACEWAY, // 0x060800
+ BRIDGE_OTHER = bindings::PCI_CLASS_BRIDGE_OTHER, // 0x068000
+
+ COMMUNICATION_SERIAL = bindings::PCI_CLASS_COMMUNICATION_SERIAL, // 0x070000
+ COMMUNICATION_PARALLEL = bindings::PCI_CLASS_COMMUNICATION_PARALLEL, // 0x070100
+ COMMUNICATION_MULTISERIAL = bindings::PCI_CLASS_COMMUNICATION_MULTISERIAL, // 0x070200
+ COMMUNICATION_MODEM = bindings::PCI_CLASS_COMMUNICATION_MODEM, // 0x070300
+ COMMUNICATION_OTHER = bindings::PCI_CLASS_COMMUNICATION_OTHER, // 0x078000
+
+ SYSTEM_PIC = bindings::PCI_CLASS_SYSTEM_PIC, // 0x080000
+ SYSTEM_PIC_IOAPIC = bindings::PCI_CLASS_SYSTEM_PIC_IOAPIC, // 0x080010
+ SYSTEM_PIC_IOXAPIC = bindings::PCI_CLASS_SYSTEM_PIC_IOXAPIC, // 0x080020
+ SYSTEM_DMA = bindings::PCI_CLASS_SYSTEM_DMA, // 0x080100
+ SYSTEM_TIMER = bindings::PCI_CLASS_SYSTEM_TIMER, // 0x080200
+ SYSTEM_RTC = bindings::PCI_CLASS_SYSTEM_RTC, // 0x080300
+ SYSTEM_PCI_HOTPLUG = bindings::PCI_CLASS_SYSTEM_PCI_HOTPLUG, // 0x080400
+ SYSTEM_SDHCI = bindings::PCI_CLASS_SYSTEM_SDHCI, // 0x080500
+ SYSTEM_RCEC = bindings::PCI_CLASS_SYSTEM_RCEC, // 0x080700
+ SYSTEM_OTHER = bindings::PCI_CLASS_SYSTEM_OTHER, // 0x088000
+
+ INPUT_KEYBOARD = bindings::PCI_CLASS_INPUT_KEYBOARD, // 0x090000
+ INPUT_PEN = bindings::PCI_CLASS_INPUT_PEN, // 0x090100
+ INPUT_MOUSE = bindings::PCI_CLASS_INPUT_MOUSE, // 0x090200
+ INPUT_SCANNER = bindings::PCI_CLASS_INPUT_SCANNER, // 0x090300
+ INPUT_GAMEPORT = bindings::PCI_CLASS_INPUT_GAMEPORT, // 0x090400
+ INPUT_OTHER = bindings::PCI_CLASS_INPUT_OTHER, // 0x098000
+
+ DOCKING_GENERIC = bindings::PCI_CLASS_DOCKING_GENERIC, // 0x0a0000
+ DOCKING_OTHER = bindings::PCI_CLASS_DOCKING_OTHER, // 0x0a8000
+
+ PROCESSOR_386 = bindings::PCI_CLASS_PROCESSOR_386, // 0x0b0000
+ PROCESSOR_486 = bindings::PCI_CLASS_PROCESSOR_486, // 0x0b0100
+ PROCESSOR_PENTIUM = bindings::PCI_CLASS_PROCESSOR_PENTIUM, // 0x0b0200
+ PROCESSOR_ALPHA = bindings::PCI_CLASS_PROCESSOR_ALPHA, // 0x0b1000
+ PROCESSOR_POWERPC = bindings::PCI_CLASS_PROCESSOR_POWERPC, // 0x0b2000
+ PROCESSOR_MIPS = bindings::PCI_CLASS_PROCESSOR_MIPS, // 0x0b3000
+ PROCESSOR_CO = bindings::PCI_CLASS_PROCESSOR_CO, // 0x0b4000
+
+ SERIAL_FIREWIRE = bindings::PCI_CLASS_SERIAL_FIREWIRE, // 0x0c0000
+ SERIAL_FIREWIRE_OHCI = bindings::PCI_CLASS_SERIAL_FIREWIRE_OHCI, // 0x0c0010
+ SERIAL_ACCESS = bindings::PCI_CLASS_SERIAL_ACCESS, // 0x0c0100
+ SERIAL_SSA = bindings::PCI_CLASS_SERIAL_SSA, // 0x0c0200
+ SERIAL_USB_UHCI = bindings::PCI_CLASS_SERIAL_USB_UHCI, // 0x0c0300
+ SERIAL_USB_OHCI = bindings::PCI_CLASS_SERIAL_USB_OHCI, // 0x0c0310
+ SERIAL_USB_EHCI = bindings::PCI_CLASS_SERIAL_USB_EHCI, // 0x0c0320
+ SERIAL_USB_XHCI = bindings::PCI_CLASS_SERIAL_USB_XHCI, // 0x0c0330
+ SERIAL_USB_CDNS = bindings::PCI_CLASS_SERIAL_USB_CDNS, // 0x0c0380
+ SERIAL_USB_DEVICE = bindings::PCI_CLASS_SERIAL_USB_DEVICE, // 0x0c03fe
+ SERIAL_FIBER = bindings::PCI_CLASS_SERIAL_FIBER, // 0x0c0400
+ SERIAL_SMBUS = bindings::PCI_CLASS_SERIAL_SMBUS, // 0x0c0500
+ SERIAL_IPMI_SMIC = bindings::PCI_CLASS_SERIAL_IPMI_SMIC, // 0x0c0700
+ SERIAL_IPMI_KCS = bindings::PCI_CLASS_SERIAL_IPMI_KCS, // 0x0c0701
+ SERIAL_IPMI_BT = bindings::PCI_CLASS_SERIAL_IPMI_BT, // 0x0c0702
+
+ WIRELESS_RF_CONTROLLER = bindings::PCI_CLASS_WIRELESS_RF_CONTROLLER, // 0x0d1000
+ WIRELESS_WHCI = bindings::PCI_CLASS_WIRELESS_WHCI, // 0x0d1010
+
+ INTELLIGENT_I2O = bindings::PCI_CLASS_INTELLIGENT_I2O, // 0x0e0000
+
+ SATELLITE_TV = bindings::PCI_CLASS_SATELLITE_TV, // 0x0f0000
+ SATELLITE_AUDIO = bindings::PCI_CLASS_SATELLITE_AUDIO, // 0x0f0100
+ SATELLITE_VOICE = bindings::PCI_CLASS_SATELLITE_VOICE, // 0x0f0300
+ SATELLITE_DATA = bindings::PCI_CLASS_SATELLITE_DATA, // 0x0f0400
+
+ CRYPT_NETWORK = bindings::PCI_CLASS_CRYPT_NETWORK, // 0x100000
+ CRYPT_ENTERTAINMENT = bindings::PCI_CLASS_CRYPT_ENTERTAINMENT, // 0x100100
+ CRYPT_OTHER = bindings::PCI_CLASS_CRYPT_OTHER, // 0x108000
+
+ SP_DPIO = bindings::PCI_CLASS_SP_DPIO, // 0x110000
+ SP_OTHER = bindings::PCI_CLASS_SP_OTHER, // 0x118000
+
+ ACCELERATOR_PROCESSING = bindings::PCI_CLASS_ACCELERATOR_PROCESSING, // 0x120000
+
+ OTHERS = bindings::PCI_CLASS_OTHERS, // 0xff0000
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 2/6] rust: pci: provide access to PCI Vendor values
2025-08-26 23:12 [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support John Hubbard
2025-08-26 23:12 ` [PATCH v7 1/6] rust: pci: provide access to PCI Class and Class-related items John Hubbard
@ 2025-08-26 23:12 ` John Hubbard
2025-08-28 13:25 ` Alexandre Courbot
2025-08-26 23:12 ` [PATCH v7 3/6] rust: pci: add DeviceId::from_class_and_vendor() method John Hubbard
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: John Hubbard @ 2025-08-26 23:12 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, John Hubbard, Elle Rhumsaa
This allows callers to write Vendor::SOME_COMPANY instead of
bindings::PCI_VENDOR_ID_SOME_COMPANY.
New APIs:
Vendor::SOME_COMPANY
Vendor::from_raw() -- Only accessible from the pci (parent) module.
Vendor::as_raw()
Vendor: fmt::Display for Vendor
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
rust/kernel/pci.rs | 2 +-
rust/kernel/pci/id.rs | 349 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 349 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 212c4a6834fb..f15cfd0e76d9 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -25,7 +25,7 @@
mod id;
-pub use self::id::{Class, ClassMask};
+pub use self::id::{Class, ClassMask, Vendor};
/// An adapter for the registration of PCI drivers.
pub struct Adapter<T: Driver>(T);
diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
index 55d9cdcc6658..4b0ad8d4edc6 100644
--- a/rust/kernel/pci/id.rs
+++ b/rust/kernel/pci/id.rs
@@ -2,7 +2,7 @@
//! PCI device identifiers and related types.
//!
-//! This module contains PCI class codes and supporting types.
+//! This module contains PCI class codes, Vendor IDs, and supporting types.
use crate::{bindings, error::code::EINVAL, error::Error, prelude::*};
use core::fmt;
@@ -109,6 +109,69 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
}
}
+/// PCI vendor IDs.
+///
+/// Each entry contains the 16-bit PCI vendor ID as assigned by the PCI SIG.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*};
+/// fn log_device_info(pdev: &pci::Device<Core>) -> Result<()> {
+/// // Compare raw vendor ID with known vendor constant
+/// let vendor_id = pdev.vendor_id();
+/// if vendor_id == Vendor::NVIDIA.as_raw() {
+/// dev_info!(
+/// pdev.as_ref(),
+/// "Found NVIDIA device: 0x{:x}\n",
+/// pdev.device_id()
+/// );
+/// }
+/// Ok(())
+/// }
+/// ```
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(transparent)]
+pub struct Vendor(u16);
+
+macro_rules! define_all_pci_vendors {
+ (
+ $($variant:ident = $binding:expr,)+
+ ) => {
+
+ impl Vendor {
+ $(
+ #[allow(missing_docs)]
+ pub const $variant: Self = Self($binding as u16);
+ )+
+ }
+ };
+}
+
+/// Once constructed, a `Vendor` contains a valid PCI Vendor ID.
+impl Vendor {
+ /// Create a Vendor from a raw 16-bit vendor ID.
+ /// Only accessible from the parent pci module.
+ #[expect(dead_code)]
+ #[inline]
+ pub(super) fn from_raw(vendor_id: u16) -> Self {
+ Self(vendor_id)
+ }
+
+ /// Get the raw 16-bit vendor ID value.
+ #[inline]
+ pub const fn as_raw(self) -> u16 {
+ self.0
+ }
+}
+
+impl fmt::Display for Vendor {
+ #[inline]
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ write!(f, "0x{:04x}", self.0)
+ }
+}
+
define_all_pci_classes! {
NOT_DEFINED = bindings::PCI_CLASS_NOT_DEFINED, // 0x000000
NOT_DEFINED_VGA = bindings::PCI_CLASS_NOT_DEFINED_VGA, // 0x000100
@@ -230,3 +293,287 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
OTHERS = bindings::PCI_CLASS_OTHERS, // 0xff0000
}
+
+define_all_pci_vendors! {
+ PCI_SIG = bindings::PCI_VENDOR_ID_PCI_SIG, // 0x0001
+ LOONGSON = bindings::PCI_VENDOR_ID_LOONGSON, // 0x0014
+ SOLIDIGM = bindings::PCI_VENDOR_ID_SOLIDIGM, // 0x025e
+ TTTECH = bindings::PCI_VENDOR_ID_TTTECH, // 0x0357
+ DYNALINK = bindings::PCI_VENDOR_ID_DYNALINK, // 0x0675
+ UBIQUITI = bindings::PCI_VENDOR_ID_UBIQUITI, // 0x0777
+ BERKOM = bindings::PCI_VENDOR_ID_BERKOM, // 0x0871
+ ITTIM = bindings::PCI_VENDOR_ID_ITTIM, // 0x0b48
+ COMPAQ = bindings::PCI_VENDOR_ID_COMPAQ, // 0x0e11
+ LSI_LOGIC = bindings::PCI_VENDOR_ID_LSI_LOGIC, // 0x1000
+ ATI = bindings::PCI_VENDOR_ID_ATI, // 0x1002
+ VLSI = bindings::PCI_VENDOR_ID_VLSI, // 0x1004
+ ADL = bindings::PCI_VENDOR_ID_ADL, // 0x1005
+ NS = bindings::PCI_VENDOR_ID_NS, // 0x100b
+ TSENG = bindings::PCI_VENDOR_ID_TSENG, // 0x100c
+ WEITEK = bindings::PCI_VENDOR_ID_WEITEK, // 0x100e
+ DEC = bindings::PCI_VENDOR_ID_DEC, // 0x1011
+ CIRRUS = bindings::PCI_VENDOR_ID_CIRRUS, // 0x1013
+ IBM = bindings::PCI_VENDOR_ID_IBM, // 0x1014
+ UNISYS = bindings::PCI_VENDOR_ID_UNISYS, // 0x1018
+ COMPEX2 = bindings::PCI_VENDOR_ID_COMPEX2, // 0x101a
+ WD = bindings::PCI_VENDOR_ID_WD, // 0x101c
+ AMI = bindings::PCI_VENDOR_ID_AMI, // 0x101e
+ AMD = bindings::PCI_VENDOR_ID_AMD, // 0x1022
+ TRIDENT = bindings::PCI_VENDOR_ID_TRIDENT, // 0x1023
+ AI = bindings::PCI_VENDOR_ID_AI, // 0x1025
+ DELL = bindings::PCI_VENDOR_ID_DELL, // 0x1028
+ MATROX = bindings::PCI_VENDOR_ID_MATROX, // 0x102B
+ MOBILITY_ELECTRONICS = bindings::PCI_VENDOR_ID_MOBILITY_ELECTRONICS, // 0x14f2
+ CT = bindings::PCI_VENDOR_ID_CT, // 0x102c
+ MIRO = bindings::PCI_VENDOR_ID_MIRO, // 0x1031
+ NEC = bindings::PCI_VENDOR_ID_NEC, // 0x1033
+ FD = bindings::PCI_VENDOR_ID_FD, // 0x1036
+ SI = bindings::PCI_VENDOR_ID_SI, // 0x1039
+ HP = bindings::PCI_VENDOR_ID_HP, // 0x103c
+ HP_3PAR = bindings::PCI_VENDOR_ID_HP_3PAR, // 0x1590
+ PCTECH = bindings::PCI_VENDOR_ID_PCTECH, // 0x1042
+ ASUSTEK = bindings::PCI_VENDOR_ID_ASUSTEK, // 0x1043
+ DPT = bindings::PCI_VENDOR_ID_DPT, // 0x1044
+ OPTI = bindings::PCI_VENDOR_ID_OPTI, // 0x1045
+ ELSA = bindings::PCI_VENDOR_ID_ELSA, // 0x1048
+ STMICRO = bindings::PCI_VENDOR_ID_STMICRO, // 0x104A
+ BUSLOGIC = bindings::PCI_VENDOR_ID_BUSLOGIC, // 0x104B
+ TI = bindings::PCI_VENDOR_ID_TI, // 0x104c
+ SONY = bindings::PCI_VENDOR_ID_SONY, // 0x104d
+ WINBOND2 = bindings::PCI_VENDOR_ID_WINBOND2, // 0x1050
+ ANIGMA = bindings::PCI_VENDOR_ID_ANIGMA, // 0x1051
+ EFAR = bindings::PCI_VENDOR_ID_EFAR, // 0x1055
+ MOTOROLA = bindings::PCI_VENDOR_ID_MOTOROLA, // 0x1057
+ PROMISE = bindings::PCI_VENDOR_ID_PROMISE, // 0x105a
+ FOXCONN = bindings::PCI_VENDOR_ID_FOXCONN, // 0x105b
+ UMC = bindings::PCI_VENDOR_ID_UMC, // 0x1060
+ PICOPOWER = bindings::PCI_VENDOR_ID_PICOPOWER, // 0x1066
+ MYLEX = bindings::PCI_VENDOR_ID_MYLEX, // 0x1069
+ APPLE = bindings::PCI_VENDOR_ID_APPLE, // 0x106b
+ YAMAHA = bindings::PCI_VENDOR_ID_YAMAHA, // 0x1073
+ QLOGIC = bindings::PCI_VENDOR_ID_QLOGIC, // 0x1077
+ CYRIX = bindings::PCI_VENDOR_ID_CYRIX, // 0x1078
+ CONTAQ = bindings::PCI_VENDOR_ID_CONTAQ, // 0x1080
+ OLICOM = bindings::PCI_VENDOR_ID_OLICOM, // 0x108d
+ SUN = bindings::PCI_VENDOR_ID_SUN, // 0x108e
+ NI = bindings::PCI_VENDOR_ID_NI, // 0x1093
+ CMD = bindings::PCI_VENDOR_ID_CMD, // 0x1095
+ BROOKTREE = bindings::PCI_VENDOR_ID_BROOKTREE, // 0x109e
+ SGI = bindings::PCI_VENDOR_ID_SGI, // 0x10a9
+ WINBOND = bindings::PCI_VENDOR_ID_WINBOND, // 0x10ad
+ PLX = bindings::PCI_VENDOR_ID_PLX, // 0x10b5
+ MADGE = bindings::PCI_VENDOR_ID_MADGE, // 0x10b6
+ THREECOM = bindings::PCI_VENDOR_ID_3COM, // 0x10b7
+ AL = bindings::PCI_VENDOR_ID_AL, // 0x10b9
+ NEOMAGIC = bindings::PCI_VENDOR_ID_NEOMAGIC, // 0x10c8
+ TCONRAD = bindings::PCI_VENDOR_ID_TCONRAD, // 0x10da
+ ROHM = bindings::PCI_VENDOR_ID_ROHM, // 0x10db
+ NVIDIA = bindings::PCI_VENDOR_ID_NVIDIA, // 0x10de
+ IMS = bindings::PCI_VENDOR_ID_IMS, // 0x10e0
+ AMCC = bindings::PCI_VENDOR_ID_AMCC, // 0x10e8
+ AMPERE = bindings::PCI_VENDOR_ID_AMPERE, // 0x1def
+ INTERG = bindings::PCI_VENDOR_ID_INTERG, // 0x10ea
+ REALTEK = bindings::PCI_VENDOR_ID_REALTEK, // 0x10ec
+ XILINX = bindings::PCI_VENDOR_ID_XILINX, // 0x10ee
+ INIT = bindings::PCI_VENDOR_ID_INIT, // 0x1101
+ CREATIVE = bindings::PCI_VENDOR_ID_CREATIVE, // 0x1102
+ TTI = bindings::PCI_VENDOR_ID_TTI, // 0x1103
+ SIGMA = bindings::PCI_VENDOR_ID_SIGMA, // 0x1105
+ VIA = bindings::PCI_VENDOR_ID_VIA, // 0x1106
+ SIEMENS = bindings::PCI_VENDOR_ID_SIEMENS, // 0x110A
+ VORTEX = bindings::PCI_VENDOR_ID_VORTEX, // 0x1119
+ EF = bindings::PCI_VENDOR_ID_EF, // 0x111a
+ IDT = bindings::PCI_VENDOR_ID_IDT, // 0x111d
+ FORE = bindings::PCI_VENDOR_ID_FORE, // 0x1127
+ PHILIPS = bindings::PCI_VENDOR_ID_PHILIPS, // 0x1131
+ EICON = bindings::PCI_VENDOR_ID_EICON, // 0x1133
+ CISCO = bindings::PCI_VENDOR_ID_CISCO, // 0x1137
+ ZIATECH = bindings::PCI_VENDOR_ID_ZIATECH, // 0x1138
+ SYSKONNECT = bindings::PCI_VENDOR_ID_SYSKONNECT, // 0x1148
+ DIGI = bindings::PCI_VENDOR_ID_DIGI, // 0x114f
+ XIRCOM = bindings::PCI_VENDOR_ID_XIRCOM, // 0x115d
+ SERVERWORKS = bindings::PCI_VENDOR_ID_SERVERWORKS, // 0x1166
+ ALTERA = bindings::PCI_VENDOR_ID_ALTERA, // 0x1172
+ SBE = bindings::PCI_VENDOR_ID_SBE, // 0x1176
+ TOSHIBA = bindings::PCI_VENDOR_ID_TOSHIBA, // 0x1179
+ TOSHIBA_2 = bindings::PCI_VENDOR_ID_TOSHIBA_2, // 0x102f
+ ATTO = bindings::PCI_VENDOR_ID_ATTO, // 0x117c
+ RICOH = bindings::PCI_VENDOR_ID_RICOH, // 0x1180
+ DLINK = bindings::PCI_VENDOR_ID_DLINK, // 0x1186
+ ARTOP = bindings::PCI_VENDOR_ID_ARTOP, // 0x1191
+ ZEITNET = bindings::PCI_VENDOR_ID_ZEITNET, // 0x1193
+ FUJITSU_ME = bindings::PCI_VENDOR_ID_FUJITSU_ME, // 0x119e
+ MARVELL = bindings::PCI_VENDOR_ID_MARVELL, // 0x11ab
+ MARVELL_EXT = bindings::PCI_VENDOR_ID_MARVELL_EXT, // 0x1b4b
+ V3 = bindings::PCI_VENDOR_ID_V3, // 0x11b0
+ ATT = bindings::PCI_VENDOR_ID_ATT, // 0x11c1
+ SPECIALIX = bindings::PCI_VENDOR_ID_SPECIALIX, // 0x11cb
+ ANALOG_DEVICES = bindings::PCI_VENDOR_ID_ANALOG_DEVICES, // 0x11d4
+ ZORAN = bindings::PCI_VENDOR_ID_ZORAN, // 0x11de
+ COMPEX = bindings::PCI_VENDOR_ID_COMPEX, // 0x11f6
+ MICROSEMI = bindings::PCI_VENDOR_ID_MICROSEMI, // 0x11f8
+ RP = bindings::PCI_VENDOR_ID_RP, // 0x11fe
+ CYCLADES = bindings::PCI_VENDOR_ID_CYCLADES, // 0x120e
+ ESSENTIAL = bindings::PCI_VENDOR_ID_ESSENTIAL, // 0x120f
+ O2 = bindings::PCI_VENDOR_ID_O2, // 0x1217
+ THREEDX = bindings::PCI_VENDOR_ID_3DFX, // 0x121a
+ AVM = bindings::PCI_VENDOR_ID_AVM, // 0x1244
+ STALLION = bindings::PCI_VENDOR_ID_STALLION, // 0x124d
+ AT = bindings::PCI_VENDOR_ID_AT, // 0x1259
+ ASIX = bindings::PCI_VENDOR_ID_ASIX, // 0x125b
+ ESS = bindings::PCI_VENDOR_ID_ESS, // 0x125d
+ SATSAGEM = bindings::PCI_VENDOR_ID_SATSAGEM, // 0x1267
+ ENSONIQ = bindings::PCI_VENDOR_ID_ENSONIQ, // 0x1274
+ TRANSMETA = bindings::PCI_VENDOR_ID_TRANSMETA, // 0x1279
+ ROCKWELL = bindings::PCI_VENDOR_ID_ROCKWELL, // 0x127A
+ ITE = bindings::PCI_VENDOR_ID_ITE, // 0x1283
+ ALTEON = bindings::PCI_VENDOR_ID_ALTEON, // 0x12ae
+ NVIDIA_SGS = bindings::PCI_VENDOR_ID_NVIDIA_SGS, // 0x12d2
+ PERICOM = bindings::PCI_VENDOR_ID_PERICOM, // 0x12D8
+ AUREAL = bindings::PCI_VENDOR_ID_AUREAL, // 0x12eb
+ ELECTRONICDESIGNGMBH = bindings::PCI_VENDOR_ID_ELECTRONICDESIGNGMBH, // 0x12f8
+ ESDGMBH = bindings::PCI_VENDOR_ID_ESDGMBH, // 0x12fe
+ CB = bindings::PCI_VENDOR_ID_CB, // 0x1307
+ SIIG = bindings::PCI_VENDOR_ID_SIIG, // 0x131f
+ RADISYS = bindings::PCI_VENDOR_ID_RADISYS, // 0x1331
+ MICRO_MEMORY = bindings::PCI_VENDOR_ID_MICRO_MEMORY, // 0x1332
+ DOMEX = bindings::PCI_VENDOR_ID_DOMEX, // 0x134a
+ INTASHIELD = bindings::PCI_VENDOR_ID_INTASHIELD, // 0x135a
+ QUATECH = bindings::PCI_VENDOR_ID_QUATECH, // 0x135C
+ SEALEVEL = bindings::PCI_VENDOR_ID_SEALEVEL, // 0x135e
+ HYPERCOPE = bindings::PCI_VENDOR_ID_HYPERCOPE, // 0x1365
+ DIGIGRAM = bindings::PCI_VENDOR_ID_DIGIGRAM, // 0x1369
+ KAWASAKI = bindings::PCI_VENDOR_ID_KAWASAKI, // 0x136b
+ CNET = bindings::PCI_VENDOR_ID_CNET, // 0x1371
+ LMC = bindings::PCI_VENDOR_ID_LMC, // 0x1376
+ NETGEAR = bindings::PCI_VENDOR_ID_NETGEAR, // 0x1385
+ APPLICOM = bindings::PCI_VENDOR_ID_APPLICOM, // 0x1389
+ MOXA = bindings::PCI_VENDOR_ID_MOXA, // 0x1393
+ CCD = bindings::PCI_VENDOR_ID_CCD, // 0x1397
+ EXAR = bindings::PCI_VENDOR_ID_EXAR, // 0x13a8
+ MICROGATE = bindings::PCI_VENDOR_ID_MICROGATE, // 0x13c0
+ THREEWARE = bindings::PCI_VENDOR_ID_3WARE, // 0x13C1
+ IOMEGA = bindings::PCI_VENDOR_ID_IOMEGA, // 0x13ca
+ ABOCOM = bindings::PCI_VENDOR_ID_ABOCOM, // 0x13D1
+ SUNDANCE = bindings::PCI_VENDOR_ID_SUNDANCE, // 0x13f0
+ CMEDIA = bindings::PCI_VENDOR_ID_CMEDIA, // 0x13f6
+ ADVANTECH = bindings::PCI_VENDOR_ID_ADVANTECH, // 0x13fe
+ MEILHAUS = bindings::PCI_VENDOR_ID_MEILHAUS, // 0x1402
+ LAVA = bindings::PCI_VENDOR_ID_LAVA, // 0x1407
+ TIMEDIA = bindings::PCI_VENDOR_ID_TIMEDIA, // 0x1409
+ ICE = bindings::PCI_VENDOR_ID_ICE, // 0x1412
+ MICROSOFT = bindings::PCI_VENDOR_ID_MICROSOFT, // 0x1414
+ OXSEMI = bindings::PCI_VENDOR_ID_OXSEMI, // 0x1415
+ CHELSIO = bindings::PCI_VENDOR_ID_CHELSIO, // 0x1425
+ EDIMAX = bindings::PCI_VENDOR_ID_EDIMAX, // 0x1432
+ ADLINK = bindings::PCI_VENDOR_ID_ADLINK, // 0x144a
+ SAMSUNG = bindings::PCI_VENDOR_ID_SAMSUNG, // 0x144d
+ GIGABYTE = bindings::PCI_VENDOR_ID_GIGABYTE, // 0x1458
+ AMBIT = bindings::PCI_VENDOR_ID_AMBIT, // 0x1468
+ MYRICOM = bindings::PCI_VENDOR_ID_MYRICOM, // 0x14c1
+ MEDIATEK = bindings::PCI_VENDOR_ID_MEDIATEK, // 0x14c3
+ TITAN = bindings::PCI_VENDOR_ID_TITAN, // 0x14D2
+ PANACOM = bindings::PCI_VENDOR_ID_PANACOM, // 0x14d4
+ SIPACKETS = bindings::PCI_VENDOR_ID_SIPACKETS, // 0x14d9
+ AFAVLAB = bindings::PCI_VENDOR_ID_AFAVLAB, // 0x14db
+ AMPLICON = bindings::PCI_VENDOR_ID_AMPLICON, // 0x14dc
+ BCM_GVC = bindings::PCI_VENDOR_ID_BCM_GVC, // 0x14a4
+ BROADCOM = bindings::PCI_VENDOR_ID_BROADCOM, // 0x14e4
+ TOPIC = bindings::PCI_VENDOR_ID_TOPIC, // 0x151f
+ MAINPINE = bindings::PCI_VENDOR_ID_MAINPINE, // 0x1522
+ ENE = bindings::PCI_VENDOR_ID_ENE, // 0x1524
+ SYBA = bindings::PCI_VENDOR_ID_SYBA, // 0x1592
+ MORETON = bindings::PCI_VENDOR_ID_MORETON, // 0x15aa
+ VMWARE = bindings::PCI_VENDOR_ID_VMWARE, // 0x15ad
+ ZOLTRIX = bindings::PCI_VENDOR_ID_ZOLTRIX, // 0x15b0
+ MELLANOX = bindings::PCI_VENDOR_ID_MELLANOX, // 0x15b3
+ DFI = bindings::PCI_VENDOR_ID_DFI, // 0x15bd
+ QUICKNET = bindings::PCI_VENDOR_ID_QUICKNET, // 0x15e2
+ ADDIDATA = bindings::PCI_VENDOR_ID_ADDIDATA, // 0x15B8
+ PDC = bindings::PCI_VENDOR_ID_PDC, // 0x15e9
+ FARSITE = bindings::PCI_VENDOR_ID_FARSITE, // 0x1619
+ ARIMA = bindings::PCI_VENDOR_ID_ARIMA, // 0x161f
+ BROCADE = bindings::PCI_VENDOR_ID_BROCADE, // 0x1657
+ SIBYTE = bindings::PCI_VENDOR_ID_SIBYTE, // 0x166d
+ ATHEROS = bindings::PCI_VENDOR_ID_ATHEROS, // 0x168c
+ NETCELL = bindings::PCI_VENDOR_ID_NETCELL, // 0x169c
+ CENATEK = bindings::PCI_VENDOR_ID_CENATEK, // 0x16CA
+ SYNOPSYS = bindings::PCI_VENDOR_ID_SYNOPSYS, // 0x16c3
+ USR = bindings::PCI_VENDOR_ID_USR, // 0x16ec
+ VITESSE = bindings::PCI_VENDOR_ID_VITESSE, // 0x1725
+ LINKSYS = bindings::PCI_VENDOR_ID_LINKSYS, // 0x1737
+ ALTIMA = bindings::PCI_VENDOR_ID_ALTIMA, // 0x173b
+ CAVIUM = bindings::PCI_VENDOR_ID_CAVIUM, // 0x177d
+ TECHWELL = bindings::PCI_VENDOR_ID_TECHWELL, // 0x1797
+ BELKIN = bindings::PCI_VENDOR_ID_BELKIN, // 0x1799
+ RDC = bindings::PCI_VENDOR_ID_RDC, // 0x17f3
+ GLI = bindings::PCI_VENDOR_ID_GLI, // 0x17a0
+ LENOVO = bindings::PCI_VENDOR_ID_LENOVO, // 0x17aa
+ QCOM = bindings::PCI_VENDOR_ID_QCOM, // 0x17cb
+ CDNS = bindings::PCI_VENDOR_ID_CDNS, // 0x17cd
+ ARECA = bindings::PCI_VENDOR_ID_ARECA, // 0x17d3
+ S2IO = bindings::PCI_VENDOR_ID_S2IO, // 0x17d5
+ SITECOM = bindings::PCI_VENDOR_ID_SITECOM, // 0x182d
+ TOPSPIN = bindings::PCI_VENDOR_ID_TOPSPIN, // 0x1867
+ COMMTECH = bindings::PCI_VENDOR_ID_COMMTECH, // 0x18f7
+ SILAN = bindings::PCI_VENDOR_ID_SILAN, // 0x1904
+ RENESAS = bindings::PCI_VENDOR_ID_RENESAS, // 0x1912
+ SOLARFLARE = bindings::PCI_VENDOR_ID_SOLARFLARE, // 0x1924
+ TDI = bindings::PCI_VENDOR_ID_TDI, // 0x192E
+ NXP = bindings::PCI_VENDOR_ID_NXP, // 0x1957
+ PASEMI = bindings::PCI_VENDOR_ID_PASEMI, // 0x1959
+ ATTANSIC = bindings::PCI_VENDOR_ID_ATTANSIC, // 0x1969
+ JMICRON = bindings::PCI_VENDOR_ID_JMICRON, // 0x197B
+ KORENIX = bindings::PCI_VENDOR_ID_KORENIX, // 0x1982
+ HUAWEI = bindings::PCI_VENDOR_ID_HUAWEI, // 0x19e5
+ NETRONOME = bindings::PCI_VENDOR_ID_NETRONOME, // 0x19ee
+ QMI = bindings::PCI_VENDOR_ID_QMI, // 0x1a32
+ AZWAVE = bindings::PCI_VENDOR_ID_AZWAVE, // 0x1a3b
+ REDHAT_QUMRANET = bindings::PCI_VENDOR_ID_REDHAT_QUMRANET, // 0x1af4
+ ASMEDIA = bindings::PCI_VENDOR_ID_ASMEDIA, // 0x1b21
+ REDHAT = bindings::PCI_VENDOR_ID_REDHAT, // 0x1b36
+ WCHIC = bindings::PCI_VENDOR_ID_WCHIC, // 0x1c00
+ SILICOM_DENMARK = bindings::PCI_VENDOR_ID_SILICOM_DENMARK, // 0x1c2c
+ AMAZON_ANNAPURNA_LABS = bindings::PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, // 0x1c36
+ CIRCUITCO = bindings::PCI_VENDOR_ID_CIRCUITCO, // 0x1cc8
+ AMAZON = bindings::PCI_VENDOR_ID_AMAZON, // 0x1d0f
+ ZHAOXIN = bindings::PCI_VENDOR_ID_ZHAOXIN, // 0x1d17
+ ROCKCHIP = bindings::PCI_VENDOR_ID_ROCKCHIP, // 0x1d87
+ HYGON = bindings::PCI_VENDOR_ID_HYGON, // 0x1d94
+ META = bindings::PCI_VENDOR_ID_META, // 0x1d9b
+ FUNGIBLE = bindings::PCI_VENDOR_ID_FUNGIBLE, // 0x1dad
+ HXT = bindings::PCI_VENDOR_ID_HXT, // 0x1dbf
+ TEKRAM = bindings::PCI_VENDOR_ID_TEKRAM, // 0x1de1
+ RPI = bindings::PCI_VENDOR_ID_RPI, // 0x1de4
+ ALIBABA = bindings::PCI_VENDOR_ID_ALIBABA, // 0x1ded
+ CXL = bindings::PCI_VENDOR_ID_CXL, // 0x1e98
+ TEHUTI = bindings::PCI_VENDOR_ID_TEHUTI, // 0x1fc9
+ SUNIX = bindings::PCI_VENDOR_ID_SUNIX, // 0x1fd4
+ HINT = bindings::PCI_VENDOR_ID_HINT, // 0x3388
+ THREEDLABS = bindings::PCI_VENDOR_ID_3DLABS, // 0x3d3d
+ NETXEN = bindings::PCI_VENDOR_ID_NETXEN, // 0x4040
+ AKS = bindings::PCI_VENDOR_ID_AKS, // 0x416c
+ WCHCN = bindings::PCI_VENDOR_ID_WCHCN, // 0x4348
+ ACCESSIO = bindings::PCI_VENDOR_ID_ACCESSIO, // 0x494f
+ S3 = bindings::PCI_VENDOR_ID_S3, // 0x5333
+ DUNORD = bindings::PCI_VENDOR_ID_DUNORD, // 0x5544
+ DCI = bindings::PCI_VENDOR_ID_DCI, // 0x6666
+ GLENFLY = bindings::PCI_VENDOR_ID_GLENFLY, // 0x6766
+ INTEL = bindings::PCI_VENDOR_ID_INTEL, // 0x8086
+ WANGXUN = bindings::PCI_VENDOR_ID_WANGXUN, // 0x8088
+ SCALEMP = bindings::PCI_VENDOR_ID_SCALEMP, // 0x8686
+ COMPUTONE = bindings::PCI_VENDOR_ID_COMPUTONE, // 0x8e0e
+ KTI = bindings::PCI_VENDOR_ID_KTI, // 0x8e2e
+ ADAPTEC = bindings::PCI_VENDOR_ID_ADAPTEC, // 0x9004
+ ADAPTEC2 = bindings::PCI_VENDOR_ID_ADAPTEC2, // 0x9005
+ HOLTEK = bindings::PCI_VENDOR_ID_HOLTEK, // 0x9412
+ NETMOS = bindings::PCI_VENDOR_ID_NETMOS, // 0x9710
+ THREECOM_2 = bindings::PCI_VENDOR_ID_3COM_2, // 0xa727
+ SOLIDRUN = bindings::PCI_VENDOR_ID_SOLIDRUN, // 0xd063
+ DIGIUM = bindings::PCI_VENDOR_ID_DIGIUM, // 0xd161
+ TIGERJET = bindings::PCI_VENDOR_ID_TIGERJET, // 0xe159
+ XILINX_RME = bindings::PCI_VENDOR_ID_XILINX_RME, // 0xea60
+ XEN = bindings::PCI_VENDOR_ID_XEN, // 0x5853
+ OCZ = bindings::PCI_VENDOR_ID_OCZ, // 0x1b85
+ NCUBE = bindings::PCI_VENDOR_ID_NCUBE, // 0x10ff
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 3/6] rust: pci: add DeviceId::from_class_and_vendor() method
2025-08-26 23:12 [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support John Hubbard
2025-08-26 23:12 ` [PATCH v7 1/6] rust: pci: provide access to PCI Class and Class-related items John Hubbard
2025-08-26 23:12 ` [PATCH v7 2/6] rust: pci: provide access to PCI Vendor values John Hubbard
@ 2025-08-26 23:12 ` John Hubbard
2025-08-26 23:12 ` [PATCH v7 4/6] gpu: nova-core: avoid probing non-display/compute PCI functions John Hubbard
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: John Hubbard @ 2025-08-26 23:12 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, John Hubbard, Elle Rhumsaa
Add a new method to create PCI DeviceIds that match both a specific
vendor and PCI class. This is more targeted than the existing
from_class() method as it filters on both vendor and class criteria.
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
rust/kernel/pci.rs | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index f15cfd0e76d9..26974cae4a22 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -161,6 +161,28 @@ 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).
+ #[inline]
+ pub const fn from_class_and_vendor(
+ class: Class,
+ class_mask: ClassMask,
+ vendor: Vendor,
+ ) -> Self {
+ Self(bindings::pci_device_id {
+ vendor: vendor.as_raw() as u32,
+ device: DeviceId::PCI_ANY_ID,
+ subvendor: DeviceId::PCI_ANY_ID,
+ subdevice: DeviceId::PCI_ANY_ID,
+ class: class.as_raw(),
+ class_mask: class_mask.as_raw(),
+ driver_data: 0,
+ override_only: 0,
+ })
+ }
}
// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `pci_device_id` and does not add
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 4/6] gpu: nova-core: avoid probing non-display/compute PCI functions
2025-08-26 23:12 [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support John Hubbard
` (2 preceding siblings ...)
2025-08-26 23:12 ` [PATCH v7 3/6] rust: pci: add DeviceId::from_class_and_vendor() method John Hubbard
@ 2025-08-26 23:12 ` John Hubbard
2025-08-26 23:12 ` [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_* John Hubbard
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: John Hubbard @ 2025-08-26 23:12 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, John Hubbard, Elle Rhumsaa
NovaCore has so far been too imprecise about figuring out if .probe()
has found a supported PCI PF (Physical Function). By that I mean:
.probe() sets up BAR0 (which involves a lot of very careful devres and
Device<Bound> details behind the scenes). And then if it is dealing with
a non-supported device such as the .1 audio PF on many GPUs, it fails
out due to an unexpected BAR0 size. We have been fortunate that the BAR0
sizes are different.
Really, we should be filtering on PCI class ID instead. These days I
think we can confidently pick out Nova's supported PF's via PCI class
ID. And if not, then we'll revisit.
The approach here is to filter on "Display VGA" or "Display 3D", which
is how PCI class IDs express "this is a modern GPU's PF".
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/gpu/nova-core/driver.rs | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 274989ea1fb4..5d23a91f51dd 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -1,6 +1,14 @@
// SPDX-License-Identifier: GPL-2.0
-use kernel::{auxiliary, bindings, c_str, device::Core, pci, prelude::*, sizes::SZ_16M, sync::Arc};
+use kernel::{
+ auxiliary, c_str,
+ device::Core,
+ pci,
+ pci::{Class, ClassMask, Vendor},
+ prelude::*,
+ sizes::SZ_16M,
+ sync::Arc,
+};
use crate::gpu::Gpu;
@@ -18,10 +26,25 @@ pub(crate) struct NovaCore {
PCI_TABLE,
MODULE_PCI_TABLE,
<NovaCore as pci::Driver>::IdInfo,
- [(
- pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as u32),
- ()
- )]
+ [
+ // Modern NVIDIA GPUs will show up as either VGA or 3D controllers.
+ (
+ pci::DeviceId::from_class_and_vendor(
+ Class::DISPLAY_VGA,
+ ClassMask::ClassSubclass,
+ Vendor::NVIDIA
+ ),
+ ()
+ ),
+ (
+ pci::DeviceId::from_class_and_vendor(
+ Class::DISPLAY_3D,
+ ClassMask::ClassSubclass,
+ Vendor::NVIDIA
+ ),
+ ()
+ ),
+ ]
);
impl pci::Driver for NovaCore {
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_*
2025-08-26 23:12 [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support John Hubbard
` (3 preceding siblings ...)
2025-08-26 23:12 ` [PATCH v7 4/6] gpu: nova-core: avoid probing non-display/compute PCI functions John Hubbard
@ 2025-08-26 23:12 ` John Hubbard
2025-08-28 13:25 ` Alexandre Courbot
2025-08-26 23:12 ` [PATCH v7 6/6] rust: pci: inline several tiny functions John Hubbard
2025-08-28 13:27 ` [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support Alexandre Courbot
6 siblings, 1 reply; 16+ messages in thread
From: John Hubbard @ 2025-08-26 23:12 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, John Hubbard, Elle Rhumsaa
Change Device::vendor_id() to return a Vendor type, and change
DeviceId::from_id() to accept a Vendor type.
Use the new pci::Vendor in the various Rust for Linux callers who were
previously using bindings::PCI_VENDOR_ID_*.
Doing so also allows removing "use kernel::bindings" entirely from most
of the affected files here.
Also, mark vendor_id() as inline.
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
rust/kernel/pci.rs | 16 +++++++++-------
rust/kernel/pci/id.rs | 18 ++++++++----------
samples/rust/rust_dma.rs | 6 +-----
samples/rust/rust_driver_auxiliary.rs | 12 +++++-------
samples/rust/rust_driver_pci.rs | 9 +++++----
5 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 26974cae4a22..182b938459a4 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -132,10 +132,10 @@ impl DeviceId {
/// Equivalent to C's `PCI_DEVICE` macro.
///
- /// Create a new `pci::DeviceId` from a vendor and device ID number.
- pub const fn from_id(vendor: u32, device: u32) -> Self {
+ /// Create a new `pci::DeviceId` from a vendor and device ID.
+ pub const fn from_id(vendor: Vendor, device: u32) -> Self {
Self(bindings::pci_device_id {
- vendor,
+ vendor: vendor.as_raw() as u32,
device,
subvendor: DeviceId::PCI_ANY_ID,
subdevice: DeviceId::PCI_ANY_ID,
@@ -232,7 +232,7 @@ macro_rules! pci_device_table {
/// <MyDriver as pci::Driver>::IdInfo,
/// [
/// (
-/// pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, bindings::PCI_ANY_ID as u32),
+/// pci::DeviceId::from_id(pci::Vendor::REDHAT, bindings::PCI_ANY_ID as u32),
/// (),
/// )
/// ]
@@ -413,10 +413,12 @@ fn as_raw(&self) -> *mut bindings::pci_dev {
}
impl Device {
- /// Returns the PCI vendor ID.
- pub fn vendor_id(&self) -> u16 {
+ /// Returns the PCI vendor ID as a validated Vendor.
+ #[inline]
+ pub fn vendor_id(&self) -> Vendor {
// SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
- unsafe { (*self.as_raw()).vendor }
+ let vendor_id = unsafe { (*self.as_raw()).vendor };
+ Vendor::from_raw(vendor_id)
}
/// Returns the PCI device ID.
diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
index 4b0ad8d4edc6..fd7a789e3015 100644
--- a/rust/kernel/pci/id.rs
+++ b/rust/kernel/pci/id.rs
@@ -118,15 +118,14 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
/// ```
/// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*};
/// fn log_device_info(pdev: &pci::Device<Core>) -> Result<()> {
-/// // Compare raw vendor ID with known vendor constant
-/// let vendor_id = pdev.vendor_id();
-/// if vendor_id == Vendor::NVIDIA.as_raw() {
-/// dev_info!(
-/// pdev.as_ref(),
-/// "Found NVIDIA device: 0x{:x}\n",
-/// pdev.device_id()
-/// );
-/// }
+/// // Get the validated PCI vendor ID
+/// let vendor = pdev.vendor_id();
+/// dev_info!(
+/// pdev.as_ref(),
+/// "Device: Vendor={}, Device=0x{:x}\n",
+/// vendor,
+/// pdev.device_id()
+/// );
/// Ok(())
/// }
/// ```
@@ -152,7 +151,6 @@ impl Vendor {
impl Vendor {
/// Create a Vendor from a raw 16-bit vendor ID.
/// Only accessible from the parent pci module.
- #[expect(dead_code)]
#[inline]
pub(super) fn from_raw(vendor_id: u16) -> Self {
Self(vendor_id)
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index c5e7cce68654..f3385c4a7e5b 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -5,7 +5,6 @@
//! To make this driver probe, QEMU must be run with `-device pci-testdev`.
use kernel::{
- bindings,
device::Core,
dma::{CoherentAllocation, Device, DmaMask},
pci,
@@ -45,10 +44,7 @@ unsafe impl kernel::transmute::FromBytes for MyStruct {}
PCI_TABLE,
MODULE_PCI_TABLE,
<DmaSampleDriver as pci::Driver>::IdInfo,
- [(
- pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, 0x5),
- ()
- )]
+ [(pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5), ())]
);
impl pci::Driver for DmaSampleDriver {
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index f2a820683fc3..55ece336ee45 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -5,7 +5,7 @@
//! To make this driver probe, QEMU must be run with `-device pci-testdev`.
use kernel::{
- auxiliary, bindings, c_str, device::Core, driver, error::Error, pci, prelude::*, InPlaceModule,
+ auxiliary, c_str, device::Core, driver, error::Error, pci, prelude::*, InPlaceModule,
};
use pin_init::PinInit;
@@ -50,10 +50,7 @@ struct ParentDriver {
PCI_TABLE,
MODULE_PCI_TABLE,
<ParentDriver as pci::Driver>::IdInfo,
- [(
- pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, 0x5),
- ()
- )]
+ [(pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5), ())]
);
impl pci::Driver for ParentDriver {
@@ -81,11 +78,12 @@ fn connect(adev: &auxiliary::Device) -> Result<()> {
let parent = adev.parent().ok_or(EINVAL)?;
let pdev: &pci::Device = parent.try_into()?;
+ let vendor = pdev.vendor_id();
dev_info!(
adev.as_ref(),
- "Connect auxiliary {} with parent: VendorID={:#x}, DeviceID={:#x}\n",
+ "Connect auxiliary {} with parent: VendorID={}, DeviceID={:#x}\n",
adev.id(),
- pdev.vendor_id(),
+ vendor,
pdev.device_id()
);
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 606946ff4d7f..f3819ac4bad6 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -4,7 +4,7 @@
//!
//! To make this driver probe, QEMU must be run with `-device pci-testdev`.
-use kernel::{bindings, c_str, device::Core, devres::Devres, pci, prelude::*, types::ARef};
+use kernel::{c_str, device::Core, devres::Devres, pci, prelude::*, types::ARef};
struct Regs;
@@ -38,7 +38,7 @@ struct SampleDriver {
MODULE_PCI_TABLE,
<SampleDriver as pci::Driver>::IdInfo,
[(
- pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, 0x5),
+ pci::DeviceId::from_id(pci::Vendor::REDHAT, 0x5),
TestIndex::NO_EVENTFD
)]
);
@@ -66,10 +66,11 @@ impl pci::Driver for SampleDriver {
const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
+ let vendor = pdev.vendor_id();
dev_dbg!(
pdev.as_ref(),
- "Probe Rust PCI driver sample (PCI ID: 0x{:x}, 0x{:x}).\n",
- pdev.vendor_id(),
+ "Probe Rust PCI driver sample (PCI ID: {}, 0x{:x}).\n",
+ vendor,
pdev.device_id()
);
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 6/6] rust: pci: inline several tiny functions
2025-08-26 23:12 [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support John Hubbard
` (4 preceding siblings ...)
2025-08-26 23:12 ` [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_* John Hubbard
@ 2025-08-26 23:12 ` John Hubbard
2025-08-28 13:27 ` [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support Alexandre Courbot
6 siblings, 0 replies; 16+ messages in thread
From: John Hubbard @ 2025-08-26 23:12 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, John Hubbard, Elle Rhumsaa
Several previous commits added Vendor and Class functionality. As part
of that, the new functions were inlined where appropriate. But that left
this file with inconsistent use of inlining. Fix that by inlining the
remaining items that should be.
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Elle Rhumsaa <elle@weathered-steel.dev>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
rust/kernel/pci.rs | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 182b938459a4..ee2215614dc7 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -133,6 +133,7 @@ impl DeviceId {
/// Equivalent to C's `PCI_DEVICE` macro.
///
/// Create a new `pci::DeviceId` from a vendor and device ID.
+ #[inline]
pub const fn from_id(vendor: Vendor, device: u32) -> Self {
Self(bindings::pci_device_id {
vendor: vendor.as_raw() as u32,
@@ -149,6 +150,7 @@ pub const fn from_id(vendor: Vendor, device: u32) -> Self {
/// Equivalent to C's `PCI_DEVICE_CLASS` macro.
///
/// Create a new `pci::DeviceId` from a class number and mask.
+ #[inline]
pub const fn from_class(class: u32, class_mask: u32) -> Self {
Self(bindings::pci_device_id {
vendor: DeviceId::PCI_ANY_ID,
@@ -385,6 +387,7 @@ fn release(&self) {
}
impl Bar {
+ #[inline]
fn index_is_valid(index: u32) -> bool {
// A `struct pci_dev` owns an array of resources with at most `PCI_NUM_RESOURCES` entries.
index < bindings::PCI_NUM_RESOURCES
@@ -407,6 +410,7 @@ fn deref(&self) -> &Self::Target {
}
impl<Ctx: device::DeviceContext> Device<Ctx> {
+ #[inline]
fn as_raw(&self) -> *mut bindings::pci_dev {
self.0.get()
}
@@ -422,6 +426,7 @@ pub fn vendor_id(&self) -> Vendor {
}
/// Returns the PCI device ID.
+ #[inline]
pub fn device_id(&self) -> u16 {
// SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
unsafe { (*self.as_raw()).device }
@@ -476,6 +481,7 @@ pub fn enable_device_mem(&self) -> Result {
}
/// Enable bus-mastering for this device.
+ #[inline]
pub fn set_master(&self) {
// SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
unsafe { bindings::pci_set_master(self.as_raw()) };
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v7 2/6] rust: pci: provide access to PCI Vendor values
2025-08-26 23:12 ` [PATCH v7 2/6] rust: pci: provide access to PCI Vendor values John Hubbard
@ 2025-08-28 13:25 ` Alexandre Courbot
2025-08-28 15:07 ` Danilo Krummrich
2025-08-29 21:48 ` John Hubbard
0 siblings, 2 replies; 16+ messages in thread
From: Alexandre Courbot @ 2025-08-28 13:25 UTC (permalink / raw)
To: John Hubbard, Danilo Krummrich
Cc: Joel Fernandes, Timur Tabi, Alistair Popple, David Airlie,
Simona Vetter, Bjorn Helgaas, Krzysztof Wilczyński,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, nouveau, linux-pci, rust-for-linux, LKML,
Elle Rhumsaa
On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
> This allows callers to write Vendor::SOME_COMPANY instead of
> bindings::PCI_VENDOR_ID_SOME_COMPANY.
>
> New APIs:
> Vendor::SOME_COMPANY
> Vendor::from_raw() -- Only accessible from the pci (parent) module.
> Vendor::as_raw()
> Vendor: fmt::Display for Vendor
>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Alexandre Courbot <acourbot@nvidia.com>
> Cc: Elle Rhumsaa <elle@weathered-steel.dev>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> rust/kernel/pci.rs | 2 +-
> rust/kernel/pci/id.rs | 349 +++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 349 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 212c4a6834fb..f15cfd0e76d9 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -25,7 +25,7 @@
>
> mod id;
>
> -pub use self::id::{Class, ClassMask};
> +pub use self::id::{Class, ClassMask, Vendor};
>
> /// An adapter for the registration of PCI drivers.
> pub struct Adapter<T: Driver>(T);
> diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
> index 55d9cdcc6658..4b0ad8d4edc6 100644
> --- a/rust/kernel/pci/id.rs
> +++ b/rust/kernel/pci/id.rs
> @@ -2,7 +2,7 @@
>
> //! PCI device identifiers and related types.
> //!
> -//! This module contains PCI class codes and supporting types.
> +//! This module contains PCI class codes, Vendor IDs, and supporting types.
>
> use crate::{bindings, error::code::EINVAL, error::Error, prelude::*};
> use core::fmt;
> @@ -109,6 +109,69 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
> }
> }
>
> +/// PCI vendor IDs.
> +///
> +/// Each entry contains the 16-bit PCI vendor ID as assigned by the PCI SIG.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*};
> +/// fn log_device_info(pdev: &pci::Device<Core>) -> Result<()> {
> +/// // Compare raw vendor ID with known vendor constant
> +/// let vendor_id = pdev.vendor_id();
> +/// if vendor_id == Vendor::NVIDIA.as_raw() {
> +/// dev_info!(
> +/// pdev.as_ref(),
> +/// "Found NVIDIA device: 0x{:x}\n",
> +/// pdev.device_id()
> +/// );
> +/// }
> +/// Ok(())
> +/// }
> +/// ```
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +#[repr(transparent)]
> +pub struct Vendor(u16);
> +
> +macro_rules! define_all_pci_vendors {
> + (
> + $($variant:ident = $binding:expr,)+
> + ) => {
> +
> + impl Vendor {
Why the blank line here? (same for the `define_all_pci_classes` in the
previous patch).
> + $(
> + #[allow(missing_docs)]
> + pub const $variant: Self = Self($binding as u16);
> + )+
> + }
> + };
> +}
> +
> +/// Once constructed, a `Vendor` contains a valid PCI Vendor ID.
> +impl Vendor {
> + /// Create a Vendor from a raw 16-bit vendor ID.
> + /// Only accessible from the parent pci module.
> + #[expect(dead_code)]
> + #[inline]
> + pub(super) fn from_raw(vendor_id: u16) -> Self {
> + Self(vendor_id)
> + }
> +
> + /// Get the raw 16-bit vendor ID value.
> + #[inline]
> + pub const fn as_raw(self) -> u16 {
> + self.0
> + }
> +}
> +
> +impl fmt::Display for Vendor {
> + #[inline]
> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> + write!(f, "0x{:04x}", self.0)
> + }
> +}
Possibly an exercice for a future patch, but do we want to display the
vendor name if it is defined, rather than its hex code (which is more
the job of `Debug`)? We could leverage the macro above to do that. The
same should be doable for the PCI classes.
I suspect strings for all the names already exist on the C side, in
which case we would want to reuse them instead of defining new ones.
Note that I don't think this needs to be done for this series - it's
just a thought as I was looking at this `Display` implementation that
looks more like a `Debug` one.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_*
2025-08-26 23:12 ` [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_* John Hubbard
@ 2025-08-28 13:25 ` Alexandre Courbot
2025-08-28 13:59 ` Danilo Krummrich
0 siblings, 1 reply; 16+ messages in thread
From: Alexandre Courbot @ 2025-08-28 13:25 UTC (permalink / raw)
To: John Hubbard, Danilo Krummrich
Cc: Joel Fernandes, Timur Tabi, Alistair Popple, David Airlie,
Simona Vetter, Bjorn Helgaas, Krzysztof Wilczyński,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, nouveau, linux-pci, rust-for-linux, LKML,
Elle Rhumsaa
On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
<snip>
> diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
> index 4b0ad8d4edc6..fd7a789e3015 100644
> --- a/rust/kernel/pci/id.rs
> +++ b/rust/kernel/pci/id.rs
> @@ -118,15 +118,14 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
> /// ```
> /// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*};
> /// fn log_device_info(pdev: &pci::Device<Core>) -> Result<()> {
> -/// // Compare raw vendor ID with known vendor constant
> -/// let vendor_id = pdev.vendor_id();
> -/// if vendor_id == Vendor::NVIDIA.as_raw() {
> -/// dev_info!(
> -/// pdev.as_ref(),
> -/// "Found NVIDIA device: 0x{:x}\n",
> -/// pdev.device_id()
> -/// );
> -/// }
> +/// // Get the validated PCI vendor ID
> +/// let vendor = pdev.vendor_id();
> +/// dev_info!(
> +/// pdev.as_ref(),
> +/// "Device: Vendor={}, Device=0x{:x}\n",
> +/// vendor,
> +/// pdev.device_id()
> +/// );
Why not use this new example starting from patch 2, which introduced the
previous code that this patch removes?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support
2025-08-26 23:12 [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support John Hubbard
` (5 preceding siblings ...)
2025-08-26 23:12 ` [PATCH v7 6/6] rust: pci: inline several tiny functions John Hubbard
@ 2025-08-28 13:27 ` Alexandre Courbot
6 siblings, 0 replies; 16+ messages in thread
From: Alexandre Courbot @ 2025-08-28 13:27 UTC (permalink / raw)
To: John Hubbard, Danilo Krummrich
Cc: Joel Fernandes, Timur Tabi, Alistair Popple, David Airlie,
Simona Vetter, Bjorn Helgaas, Krzysztof Wilczyński,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, nouveau, linux-pci, rust-for-linux, LKML,
Elle Rhumsaa
On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
> Changes since v6:
>
> * Applied changes from Danilo's and Alex's and Elle's reviews (thanks!):
> * Rebased onto driver-core-next, which is here:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
> * Changed pci::Vendor to be a u16, instead of a u32.
> * Inlined all of the tiniest functions.
> * Changed from Class/Vendor new(), to from_raw().
> * Made from_raw() only accessible to super, which in this case is
> the pci module.
> * Restored infallible operations. That causes Alex's request for the
> following reasonable behavior to work once again:
>
> from_raw(0x10de).as_raw() == 0x10de
>
> * Added a new patch, to inline the remaining PCI operations. This
> provides consistent inline choices throughout pci.rs.
I am far from a PCI expert, but regardless of bus considerations the
code appears to make sense to me (particularly since it removes some
uses of the `bindings` module!). Thus, and FWIW,
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_*
2025-08-28 13:25 ` Alexandre Courbot
@ 2025-08-28 13:59 ` Danilo Krummrich
2025-08-29 21:38 ` John Hubbard
0 siblings, 1 reply; 16+ messages in thread
From: Danilo Krummrich @ 2025-08-28 13:59 UTC (permalink / raw)
To: Alexandre Courbot
Cc: John Hubbard, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, Elle Rhumsaa
On 8/28/25 3:25 PM, Alexandre Courbot wrote:
> On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
> <snip>
>> diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
>> index 4b0ad8d4edc6..fd7a789e3015 100644
>> --- a/rust/kernel/pci/id.rs
>> +++ b/rust/kernel/pci/id.rs
>> @@ -118,15 +118,14 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
>> /// ```
>> /// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*};
>> /// fn log_device_info(pdev: &pci::Device<Core>) -> Result<()> {
>> -/// // Compare raw vendor ID with known vendor constant
>> -/// let vendor_id = pdev.vendor_id();
>> -/// if vendor_id == Vendor::NVIDIA.as_raw() {
>> -/// dev_info!(
>> -/// pdev.as_ref(),
>> -/// "Found NVIDIA device: 0x{:x}\n",
>> -/// pdev.device_id()
>> -/// );
>> -/// }
>> +/// // Get the validated PCI vendor ID
>> +/// let vendor = pdev.vendor_id();
>> +/// dev_info!(
>> +/// pdev.as_ref(),
>> +/// "Device: Vendor={}, Device=0x{:x}\n",
>> +/// vendor,
>> +/// pdev.device_id()
>> +/// );
>
> Why not use this new example starting from patch 2, which introduced the
> previous code that this patch removes?
I think that's because in v2 vendor_id() still returns the raw value. I think it
makes a little more sense if this patch simply introduces the example as an
example for vendor_id() itself.
I think struct Vendor does not necessarily need an example by itself.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 2/6] rust: pci: provide access to PCI Vendor values
2025-08-28 13:25 ` Alexandre Courbot
@ 2025-08-28 15:07 ` Danilo Krummrich
2025-08-29 21:48 ` John Hubbard
1 sibling, 0 replies; 16+ messages in thread
From: Danilo Krummrich @ 2025-08-28 15:07 UTC (permalink / raw)
To: Alexandre Courbot
Cc: John Hubbard, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, Elle Rhumsaa
On Thu Aug 28, 2025 at 3:25 PM CEST, Alexandre Courbot wrote:
> On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
>> This allows callers to write Vendor::SOME_COMPANY instead of
>> bindings::PCI_VENDOR_ID_SOME_COMPANY.
>>
>> New APIs:
>> Vendor::SOME_COMPANY
>> Vendor::from_raw() -- Only accessible from the pci (parent) module.
>> Vendor::as_raw()
>> Vendor: fmt::Display for Vendor
>>
>> Cc: Danilo Krummrich <dakr@kernel.org>
>> Cc: Alexandre Courbot <acourbot@nvidia.com>
>> Cc: Elle Rhumsaa <elle@weathered-steel.dev>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> ---
>> rust/kernel/pci.rs | 2 +-
>> rust/kernel/pci/id.rs | 349 +++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 349 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
>> index 212c4a6834fb..f15cfd0e76d9 100644
>> --- a/rust/kernel/pci.rs
>> +++ b/rust/kernel/pci.rs
>> @@ -25,7 +25,7 @@
>>
>> mod id;
>>
>> -pub use self::id::{Class, ClassMask};
>> +pub use self::id::{Class, ClassMask, Vendor};
>>
>> /// An adapter for the registration of PCI drivers.
>> pub struct Adapter<T: Driver>(T);
>> diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
>> index 55d9cdcc6658..4b0ad8d4edc6 100644
>> --- a/rust/kernel/pci/id.rs
>> +++ b/rust/kernel/pci/id.rs
>> @@ -2,7 +2,7 @@
>>
>> //! PCI device identifiers and related types.
>> //!
>> -//! This module contains PCI class codes and supporting types.
>> +//! This module contains PCI class codes, Vendor IDs, and supporting types.
>>
>> use crate::{bindings, error::code::EINVAL, error::Error, prelude::*};
>> use core::fmt;
>> @@ -109,6 +109,69 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
>> }
>> }
>>
>> +/// PCI vendor IDs.
>> +///
>> +/// Each entry contains the 16-bit PCI vendor ID as assigned by the PCI SIG.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*};
>> +/// fn log_device_info(pdev: &pci::Device<Core>) -> Result<()> {
>> +/// // Compare raw vendor ID with known vendor constant
>> +/// let vendor_id = pdev.vendor_id();
>> +/// if vendor_id == Vendor::NVIDIA.as_raw() {
>> +/// dev_info!(
>> +/// pdev.as_ref(),
>> +/// "Found NVIDIA device: 0x{:x}\n",
>> +/// pdev.device_id()
>> +/// );
>> +/// }
>> +/// Ok(())
>> +/// }
>> +/// ```
>> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
>> +#[repr(transparent)]
>> +pub struct Vendor(u16);
>> +
>> +macro_rules! define_all_pci_vendors {
>> + (
>> + $($variant:ident = $binding:expr,)+
>> + ) => {
>> +
>> + impl Vendor {
>
> Why the blank line here? (same for the `define_all_pci_classes` in the
> previous patch).
>
>> + $(
>> + #[allow(missing_docs)]
>> + pub const $variant: Self = Self($binding as u16);
>> + )+
>> + }
>> + };
>> +}
>> +
>> +/// Once constructed, a `Vendor` contains a valid PCI Vendor ID.
>> +impl Vendor {
>> + /// Create a Vendor from a raw 16-bit vendor ID.
>> + /// Only accessible from the parent pci module.
>> + #[expect(dead_code)]
>> + #[inline]
>> + pub(super) fn from_raw(vendor_id: u16) -> Self {
>> + Self(vendor_id)
>> + }
>> +
>> + /// Get the raw 16-bit vendor ID value.
>> + #[inline]
>> + pub const fn as_raw(self) -> u16 {
>> + self.0
>> + }
>> +}
>> +
>> +impl fmt::Display for Vendor {
>> + #[inline]
>> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>> + write!(f, "0x{:04x}", self.0)
>> + }
>> +}
>
> Possibly an exercice for a future patch, but do we want to display the
> vendor name if it is defined, rather than its hex code (which is more
> the job of `Debug`)? We could leverage the macro above to do that. The
> same should be doable for the PCI classes.
>
> I suspect strings for all the names already exist on the C side, in
> which case we would want to reuse them instead of defining new ones.
>
> Note that I don't think this needs to be done for this series - it's
> just a thought as I was looking at this `Display` implementation that
> looks more like a `Debug` one.
Yeah, this can be addressed subsequently; it might make sense to align Display
and Debug though. Currently, Vendor simply derives Debug, resulting in the
decimal value to be printed.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_*
2025-08-28 13:59 ` Danilo Krummrich
@ 2025-08-29 21:38 ` John Hubbard
2025-08-29 21:46 ` Danilo Krummrich
0 siblings, 1 reply; 16+ messages in thread
From: John Hubbard @ 2025-08-29 21:38 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot
Cc: Joel Fernandes, Timur Tabi, Alistair Popple, David Airlie,
Simona Vetter, Bjorn Helgaas, Krzysztof Wilczyński,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, nouveau, linux-pci, rust-for-linux, LKML,
Elle Rhumsaa
On 8/28/25 6:59 AM, Danilo Krummrich wrote:
> On 8/28/25 3:25 PM, Alexandre Courbot wrote:
>> On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
>> <snip>
>>> diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
>>> index 4b0ad8d4edc6..fd7a789e3015 100644
>>> --- a/rust/kernel/pci/id.rs
>>> +++ b/rust/kernel/pci/id.rs
>>> @@ -118,15 +118,14 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
>>> /// ```
>>> /// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*};
>>> /// fn log_device_info(pdev: &pci::Device<Core>) -> Result<()> {
>>> -/// // Compare raw vendor ID with known vendor constant
>>> -/// let vendor_id = pdev.vendor_id();
>>> -/// if vendor_id == Vendor::NVIDIA.as_raw() {
>>> -/// dev_info!(
>>> -/// pdev.as_ref(),
>>> -/// "Found NVIDIA device: 0x{:x}\n",
>>> -/// pdev.device_id()
>>> -/// );
>>> -/// }
>>> +/// // Get the validated PCI vendor ID
>>> +/// let vendor = pdev.vendor_id();
>>> +/// dev_info!(
>>> +/// pdev.as_ref(),
>>> +/// "Device: Vendor={}, Device=0x{:x}\n",
>>> +/// vendor,
>>> +/// pdev.device_id()
>>> +/// );
>>
>> Why not use this new example starting from patch 2, which introduced the
>> previous code that this patch removes?
>
> I think that's because in v2 vendor_id() still returns the raw value. I think it
That is correct.
> makes a little more sense if this patch simply introduces the example as an
> example for vendor_id() itself.
>
> I think struct Vendor does not necessarily need an example by itself.
I'm not quite sure if you are asking for a change to this patch? The
example already exercises .vendor_id(), so...?
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_*
2025-08-29 21:38 ` John Hubbard
@ 2025-08-29 21:46 ` Danilo Krummrich
2025-08-29 21:49 ` John Hubbard
0 siblings, 1 reply; 16+ messages in thread
From: Danilo Krummrich @ 2025-08-29 21:46 UTC (permalink / raw)
To: John Hubbard
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, Elle Rhumsaa
On 8/29/25 11:38 PM, John Hubbard wrote:
> On 8/28/25 6:59 AM, Danilo Krummrich wrote:
>> On 8/28/25 3:25 PM, Alexandre Courbot wrote:
>>> On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
>>> <snip>
>>>> diff --git a/rust/kernel/pci/id.rs b/rust/kernel/pci/id.rs
>>>> index 4b0ad8d4edc6..fd7a789e3015 100644
>>>> --- a/rust/kernel/pci/id.rs
>>>> +++ b/rust/kernel/pci/id.rs
>>>> @@ -118,15 +118,14 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
>>>> /// ```
>>>> /// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*};
>>>> /// fn log_device_info(pdev: &pci::Device<Core>) -> Result<()> {
>>>> -/// // Compare raw vendor ID with known vendor constant
>>>> -/// let vendor_id = pdev.vendor_id();
>>>> -/// if vendor_id == Vendor::NVIDIA.as_raw() {
>>>> -/// dev_info!(
>>>> -/// pdev.as_ref(),
>>>> -/// "Found NVIDIA device: 0x{:x}\n",
>>>> -/// pdev.device_id()
>>>> -/// );
>>>> -/// }
>>>> +/// // Get the validated PCI vendor ID
>>>> +/// let vendor = pdev.vendor_id();
>>>> +/// dev_info!(
>>>> +/// pdev.as_ref(),
>>>> +/// "Device: Vendor={}, Device=0x{:x}\n",
>>>> +/// vendor,
>>>> +/// pdev.device_id()
>>>> +/// );
>>>
>>> Why not use this new example starting from patch 2, which introduced the
>>> previous code that this patch removes?
>>
>> I think that's because in v2 vendor_id() still returns the raw value. I think it
>
> That is correct.
>
>> makes a little more sense if this patch simply introduces the example as an
>> example for vendor_id() itself.
>>
>> I think struct Vendor does not necessarily need an example by itself.
>
> I'm not quite sure if you are asking for a change to this patch? The
> example already exercises .vendor_id(), so...?
Yes, I think the example above should be on the vendor_id() method rather than
on the Vendor struct and should be introduced by this patch.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 2/6] rust: pci: provide access to PCI Vendor values
2025-08-28 13:25 ` Alexandre Courbot
2025-08-28 15:07 ` Danilo Krummrich
@ 2025-08-29 21:48 ` John Hubbard
1 sibling, 0 replies; 16+ messages in thread
From: John Hubbard @ 2025-08-29 21:48 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich
Cc: Joel Fernandes, Timur Tabi, Alistair Popple, David Airlie,
Simona Vetter, Bjorn Helgaas, Krzysztof Wilczyński,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, nouveau, linux-pci, rust-for-linux, LKML,
Elle Rhumsaa
On 8/28/25 6:25 AM, Alexandre Courbot wrote:
> On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
...
>> +macro_rules! define_all_pci_vendors {
>> + (
>> + $($variant:ident = $binding:expr,)+
>> + ) => {
>> +
>> + impl Vendor {
>
> Why the blank line here? (same for the `define_all_pci_classes` in the
> previous patch).
Removed in both places, thanks!
...
>> +impl fmt::Display for Vendor {
>> + #[inline]
>> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>> + write!(f, "0x{:04x}", self.0)
>> + }
>> +}
>
> Possibly an exercice for a future patch, but do we want to display the
> vendor name if it is defined, rather than its hex code (which is more
> the job of `Debug`)? We could leverage the macro above to do that. The
> same should be doable for the PCI classes.
>
> I suspect strings for all the names already exist on the C side, in
> which case we would want to reuse them instead of defining new ones.
A reasonable suspicion, but actually, they do not yet exist. So I think
we could just add them on the Rust side.
>
> Note that I don't think this needs to be done for this series - it's
> just a thought as I was looking at this `Display` implementation that
> looks more like a `Debug` one.
Good points. Yes, I can do a follow-up patch.
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_*
2025-08-29 21:46 ` Danilo Krummrich
@ 2025-08-29 21:49 ` John Hubbard
0 siblings, 0 replies; 16+ messages in thread
From: John Hubbard @ 2025-08-29 21:49 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
David Airlie, Simona Vetter, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, nouveau, linux-pci, rust-for-linux,
LKML, Elle Rhumsaa
On 8/29/25 2:46 PM, Danilo Krummrich wrote:
> On 8/29/25 11:38 PM, John Hubbard wrote:
>> On 8/28/25 6:59 AM, Danilo Krummrich wrote:
>>> On 8/28/25 3:25 PM, Alexandre Courbot wrote:
>>>> On Wed Aug 27, 2025 at 8:12 AM JST, John Hubbard wrote:
>>>> <snip>
>>> makes a little more sense if this patch simply introduces the example as an
>>> example for vendor_id() itself.
>>>
>>> I think struct Vendor does not necessarily need an example by itself.
>>
>> I'm not quite sure if you are asking for a change to this patch? The
>> example already exercises .vendor_id(), so...?
>
> Yes, I think the example above should be on the vendor_id() method rather than
> on the Vendor struct and should be introduced by this patch.
ohhh, so located directly above it. I finally get it. haha. I'll do that, thanks. :)
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-08-29 21:50 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26 23:12 [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support John Hubbard
2025-08-26 23:12 ` [PATCH v7 1/6] rust: pci: provide access to PCI Class and Class-related items John Hubbard
2025-08-26 23:12 ` [PATCH v7 2/6] rust: pci: provide access to PCI Vendor values John Hubbard
2025-08-28 13:25 ` Alexandre Courbot
2025-08-28 15:07 ` Danilo Krummrich
2025-08-29 21:48 ` John Hubbard
2025-08-26 23:12 ` [PATCH v7 3/6] rust: pci: add DeviceId::from_class_and_vendor() method John Hubbard
2025-08-26 23:12 ` [PATCH v7 4/6] gpu: nova-core: avoid probing non-display/compute PCI functions John Hubbard
2025-08-26 23:12 ` [PATCH v7 5/6] rust: pci: use pci::Vendor instead of bindings::PCI_VENDOR_ID_* John Hubbard
2025-08-28 13:25 ` Alexandre Courbot
2025-08-28 13:59 ` Danilo Krummrich
2025-08-29 21:38 ` John Hubbard
2025-08-29 21:46 ` Danilo Krummrich
2025-08-29 21:49 ` John Hubbard
2025-08-26 23:12 ` [PATCH v7 6/6] rust: pci: inline several tiny functions John Hubbard
2025-08-28 13:27 ` [PATCH v7 0/6] rust, nova-core: PCI Class, Vendor support Alexandre Courbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).