Re: [PATCH 5/5] rust: device: Add a stub abstraction for devices

[Gary Guo]

On Thu, 9 Mar 2023 16:06:06 -0300
Wedson Almeida Filho <wedsonaf@xxxxxxxxx> wrote:

> On Thu, 9 Mar 2023 at 14:11, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Thu, Mar 09, 2023 at 01:46:39PM -0300, Wedson Almeida Filho wrote:  
> > > On Thu, 9 Mar 2023 at 08:24, Greg Kroah-Hartman
> > > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:  
> > > > > > > +        // owns a reference. This is satisfied by the call to `get_device` above.
> > > > > > > +        Self { ptr }
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    /// Creates a new device instance from an existing [`RawDevice`] instance.
> > > > > > > +    pub fn from_dev(dev: &dyn RawDevice) -> Self {  
> > > > > >
> > > > > > I am a rust newbie, but I don't understand this "RawDevice" here at all.  
> > > > >
> > > > > Different buses will have their own Rust "Device" type, for example,
> > > > > pci::Device, amba::Device, platform::Device that wrap their C
> > > > > counterparts pci_dev, amba_device, platform_device.
> > > > >
> > > > > "RawDevice" is a trait for functionality that is common to all
> > > > > devices. It exposes the "struct device" of each bus/subsystem so that
> > > > > functions that work on any "struct device", for example, `clk_get`,
> > > > > `pr_info`. will automatically work on all subsystems.  
> > > >
> > > > Why is this being called "Raw" then?  Why not just "Device" to follow
> > > > along with the naming scheme that the kernel already uses?  
> > >
> > > Because it gives us access to underlying raw `struct device` pointer,
> > > in Rust raw pointers are those unsafe `*mut T` or `*const T`. I'm not
> > > married to the name though, we should probably look for a better one
> > > if this one is confusing.
> > >
> > > Just "Device" is already taken. It's a ref-counted `struct device` (it
> > > calls get_device/put_device in the right places automatically,
> > > guarantees no dandling pointers); it is meant to be used by code that
> > > needs to hold on to devices when they don't care about the bus. (It in
> > > fact implements `RawDevice`.)  
> >
> > I don't understand, why do you need both of these?  Why can't one just
> > do?  Why would you need one without the other?  I would think that
> > "Device" and "RawDevice" here would be the same thing, that is a way to
> > refer to a "larger" underlying struct device memory chunk in a way that
> > can be passed around without knowing, or caring, what the "real" device
> > type is.  
> 
> `Device` is a struct, it is the Rust abstraction for C's `struct device`.
> 
> Let's use the platform bus as our running example: we have
> `platform::Device` as the Rust abstraction for C's `struct
> platform_device`.
> 
> Let's use `clk_get`as our running example of a function that takes a
> `struct device` as argument.
> 
> If we have a platform device, we can't just call `clk_get` because the
> types don't match. In C, we access the `dev` field of `struct
> platform_device` before we call `clk_get` (i.e., we call
> clk_get(&pdev->dev, ...)), but in Rust we don't want to make the
> fields of `platform::Device` public, especially because they're fields
> of a C struct. So as part of `platform::Device` we'd have to implement
> something like:
> 
> impl platform::Device {
>     fn get_device(&self) -> &Device {
>     ...
>     }
> }
> 
> Then calling `clk_get` would be something like:
> 
> pdev.get_device().clk_get(...)

Just thinking out loud here, would it work if `platform::Device`
implements `Deref<Target = Device>`?

Best,
Gary