On Tue, Jun 25, 2024 at 12:53:48PM +0200, Andreas Hindborg wrote:
> Hi Danilo,
>
> Thanks for working on this. I just finished rebasing the Rust NVMe
> driver on these patches, and I have just one observation for now.
>
> Danilo Krummrich <dakr@xxxxxxxxxx> writes:
>
> [...]
>
> > +pub trait Driver {
> > + /// Data stored on device by driver.
> > + ///
> > + /// Corresponds to the data set or retrieved via the kernel's
> > + /// `pci_{set,get}_drvdata()` functions.
> > + ///
> > + /// Require that `Data` implements `ForeignOwnable`. We guarantee to
> > + /// never move the underlying wrapped data structure.
> > + ///
> > + /// TODO: Use associated_type_defaults once stabilized:
> > + ///
> > + /// `type Data: ForeignOwnable = ();`
> > + type Data: ForeignOwnable;
> > +
> > + /// The type holding information about each device id supported by the driver.
> > + ///
> > + /// TODO: Use associated_type_defaults once stabilized:
> > + ///
> > + /// type IdInfo: 'static = ();
> > + type IdInfo: 'static;
> > +
> > + /// The table of device ids supported by the driver.
> > + const ID_TABLE: IdTable<'static, DeviceId, Self::IdInfo>;
> > +
> > + /// PCI driver probe.
> > + ///
> > + /// Called when a new platform device is added or discovered.
> > + /// Implementers should attempt to initialize the device here.
> > + fn probe(dev: &mut Device, id: Option<&Self::IdInfo>) -> Result<Self::Data>;
>
> Since you changed the `Device` representation to be basically an `ARef`,
> the `&mut` makes no sense. I think we should either pass by value or
> immutable reference.
Agreed, I think we should just pass it by value.
I also noticed that I need to fix `set_master` and `enable_device_mem` to
require a mutable reference.
>
>
> Best regards,
> Andreas
>
>
> > +
> > + /// PCI driver remove.
> > + ///
> > + /// Called when a platform device is removed.
> > + /// Implementers should prepare the device for complete removal here.
> > + fn remove(data: &Self::Data);
> > +}
> > +
> > +/// 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>);
> > +
>
