Re: [PATCH v3 15/16] rust: platform: add basic platform device / driver abstractions

[Danilo Krummrich]

On Wed, Dec 04, 2024 at 04:25:32PM -0300, Daniel Almeida wrote:
> Hi Danilo,
> 
> > On 22 Oct 2024, at 18:31, Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
> > +
> > +/// The platform device representation.
> > +///
> > +/// A platform device is based on an always reference counted `device:Device` instance. Cloning a
> > +/// platform device, hence, also increments the base device' reference count.
> > +///
> > +/// # Invariants
> > +///
> > +/// `Device` holds a valid reference of `ARef<device::Device>` whose underlying `struct device` is a
> > +/// member of a `struct platform_device`.
> > +#[derive(Clone)]
> > +pub struct Device(ARef<device::Device>);
> > +
> > +impl Device {
> > +    /// Convert a raw kernel device into a `Device`
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// `dev` must be an `Aref<device::Device>` whose underlying `bindings::device` is a member of a
> > +    /// `bindings::platform_device`.
> > +    unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
> > +        Self(dev)
> > +    }
> > +
> > +    fn as_dev(&self) -> &device::Device {
> 
> This has to be pub, since a platform::Device is at least as useful as a device::Device.
> 
> IOW: if an API takes &device::Device, there is no reason why someone with a &platform::Device
> shouldn’t be able to call it.
> 
> In particular, having this as private makes it impossible for a platform driver to use Abdiel’s DMA allocator at [0].

No worries, I already made it public in my branch [1], I'll send out a v4 soon.

[1] https://github.com/Rust-for-Linux/linux/blob/staging/dev/rust/kernel/platform.rs#L213

- Danilo

> 
> > +        &self.0
> > +    }
> > +
> > +    fn as_raw(&self) -> *mut bindings::platform_device {
> > +        // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
> > +        // embedded in `struct platform_device`.
> > +        unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }.cast_mut()
> > +    }
> > +}
> > +
> > +impl AsRef<device::Device> for Device {
> > +    fn as_ref(&self) -> &device::Device {
> > +        &self.0
> > +    }
> > +}
> > -- 
> > 2.46.2
> > 
> 
> — Daniel
> 
> [0]: https://lkml.org/lkml/2024/12/3/1281
>