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.0On 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?
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.
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
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.
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
© 2016 - 2025 Red Hat, Inc.