On Tue, Dec 10, 2024 at 11:38 PM Danilo Krummrich <dakr@xxxxxxxxxx> wrote: > > On Tue, Dec 10, 2024 at 11:55:33AM +0100, Alice Ryhl wrote: > > On Mon, Dec 9, 2024 at 11:44 AM Danilo Krummrich <dakr@xxxxxxxxxx> wrote: > > > > > > On Fri, Dec 06, 2024 at 03:01:18PM +0100, Alice Ryhl wrote: > > > > On Thu, Dec 5, 2024 at 3:16 PM Danilo Krummrich <dakr@xxxxxxxxxx> wrote: > > > > > > > > > > Implement the basic PCI abstractions required to write a basic PCI > > > > > driver. This includes the following data structures: > > > > > > > > > > The `pci::Driver` trait represents the interface to the driver and > > > > > provides `pci::Driver::probe` for the driver to implement. > > > > > > > > > > The `pci::Device` abstraction represents a `struct pci_dev` and provides > > > > > abstractions for common functions, such as `pci::Device::set_master`. > > > > > > > > > > In order to provide the PCI specific parts to a generic > > > > > `driver::Registration` the `driver::RegistrationOps` trait is implemented > > > > > by `pci::Adapter`. > > > > > > > > > > `pci::DeviceId` implements PCI device IDs based on the generic > > > > > `device_id::RawDevceId` abstraction. > > > > > > > > > > Co-developed-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxx> > > > > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxx> > > > > > Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx> > > > > > > > > > +/// The PCI device representation. > > > > > +/// > > > > > +/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI > > > > > +/// device, hence, also increments the base device' reference count. > > > > > +#[derive(Clone)] > > > > > +pub struct Device(ARef<device::Device>); > > > > > > > > It seems more natural for this to be a wrapper around > > > > `Opaque<bindings::pci_dev>`. Then you can have both &Device and > > > > ARef<Device> depending on whether you want to hold a refcount or not. > > > > > > Yeah, but then every bus device has to re-implement the refcount dance we > > > already have in `device::Device` for the underlying base `struct device`. > > > > > > I forgot to mention this in my previous reply to Boqun, but we even documented > > > it this way in `device::Device` [1]. > > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/device.rs#n28 > > > > We could perhaps write a derive macro for AlwaysRefCounted that > > delegates to the inner type? That way, we can have the best of both > > worlds. > > Sounds interesting, how exactly would this work? > > (I'll already send out a v5, but let's keep discussing this.) Well, the derive macro could assume that the refcount is manipulated in the same way as the inner type does it. I admit that the idea is not fully formed, but if we can avoid wrapping ARef, that would be ideal. It sounds like the only reason you don't do that is that it's more unsafe, which the macro could reduce. Alice
