Re: [PATCH v5 2/4] rust: pci: provide access to PCI Vendor values

[John Hubbard <jhubbard@nvidia.com>]

On 8/25/25 3:47 PM, Elle Rhumsaa wrote:
> On Wed, Aug 20, 2025 at 09:42:05PM -0700, 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::as_raw()
>>     Vendor: From<u32> 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 | 355 +++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 355 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
>> index 0faec49bf8a2..d4675b7d4a86 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 1291553b4e15..dd91e25a6890 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;
>> @@ -115,6 +115,74 @@ 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<()> {
>> +///     // Get the raw PCI vendor ID and convert to Vendor
>> +///     let vendor_id = pdev.vendor_id();
>> +///     let vendor = Vendor::new(vendor_id.into());
>> +///     dev_info!(
>> +///         pdev.as_ref(),
>> +///         "Device: Vendor={}, Device=0x{:x}\n",
>> +///         vendor,
>> +///         pdev.device_id()
>> +///     );
>> +///     Ok(())
>> +/// }
>> +/// ```
>> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
>> +#[repr(transparent)]
>> +pub struct Vendor(u32);
>> +
>> +macro_rules! define_all_pci_vendors {
>> +    (
>> +        $($variant:ident = $binding:expr,)+
>> +    ) => {
>> +
>> +        impl Vendor {
>> +            $(
>> +                #[allow(missing_docs)]
>> +                pub const $variant: Self = Self($binding as u32);
>> +            )+
>> +        }
>> +
>> +        /// Convert a raw 16-bit vendor ID to a `Vendor`.
>> +        impl From<u32> for Vendor {
>> +            fn from(value: u32) -> Self {
>> +                match value {
>> +                    $(x if x == Self::$variant.0 => Self::$variant,)+
>> +                    _ => Self::UNKNOWN,
>> +                }
>> +            }
>> +        }
>> +    };
>> +}
>> +
>> +/// Once constructed, a `Vendor` contains a valid PCI Vendor ID.
>> +impl Vendor {
>> +    /// Create a new Vendor from a raw 16-bit vendor ID.
>> +    pub fn new(vendor_id: u32) -> Self {
>> +        Self::from(vendor_id)
>> +    }
> 
> Reversing this implementation would allow for a signature:
> 
> ```rust
>     pub const fn new(vendor_id: u32) -> Self {
>         ...
>     }
> ```
> 
> Which would allow use in `const` contexts. Until we get a stable
> `const-trait` impl, this is kind of the best workaround.
> 
> Then, the `From<u32>` implementation can call `Self::new`.
> 
> Not really applicable here, but you can also provide a `Default`
> implementation for `Self::new` with no parameters.
> 

I'm going to post a new version, v7, later today, that will
change this area around quite a bit.


thanks,
-- 
John Hubbard