Re: [PATCH] rust: pci: fix documentation related to Device instances

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux