On Sun, 29 Jun, 2025 00:14:18 -0700 "Wren Turkal" <wt@xxxxxxxxxxxxxxxx> wrote: > On 6/28/25 10:57 PM, Rahul Rameshbabu wrote: >> Device instances in the pci crate represent a valid struct pci_dev, not a struct >> device. >> >> Signed-off-by: Rahul Rameshbabu <sergeantsagara@xxxxxxxxxxxxxx> >> --- >> >> Notes: >> Notes: >> >> I noticed this while working on my HID abstraction work and figured it would be >> a small fixup I could send afterwards. >> >> Link: https://lore.kernel.org/rust-for-linux/20250629045031.92358-2-sergeantsagara@xxxxxxxxxxxxxx/ >> >> rust/kernel/pci.rs | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs >> index 6b94fd7a3ce9..af25a3fe92e5 100644 >> --- a/rust/kernel/pci.rs >> +++ b/rust/kernel/pci.rs >> @@ -254,7 +254,7 @@ pub trait Driver: Send { >> /// >> /// # Invariants >> /// >> -/// A [`Device`] instance represents a valid `struct device` created by the C portion of the kernel. >> +/// A [`Device`] instance represents a valid `struct pci_dev` created by the C portion of the kernel. > > Should this not just be a "a valid pci device" and let the type in the > function definition speak for the type instead of duplicating the type > name in the doc comment? > My theory is that this comment is explicitly done for folks reading this code from the C side. Rust tuple structs are not exactly a common semantic in other programming languages. That said, I am open to changing the comments if Danillo or others think that would be better as well. Either way, I appreciate you taking the time to respond. >> #[repr(transparent)] >> pub struct Device<Ctx: device::DeviceContext = device::Normal>( >> Opaque<bindings::pci_dev>, -- Thanks, Rahul Rameshbabu