[RFC PATCH 7/8] rust: add firmware abstractions

[FUJITA Tomonori]

Hi,

On Wed, 22 May 2024 09:37:30 +0200
Philipp Stanner <pstanner at redhat.com> wrote:

>> > +/// Abstraction around a C firmware struct.
>> > +///
>> > +/// This is a simple abstraction around the C firmware API. Just
>> > like with the C API, firmware can
>> > +/// be requested. Once requested the abstraction provides direct
>> > access to the firmware buffer as
>> > +/// `&[u8]`. Alternatively, the firmware can be copied to a new
>> > buffer using `Firmware::copy`. The
>> > +/// firmware is released once [`Firmware`] is dropped.
>> > +///
>> > +/// # Examples
>> > +///
>> > +/// ```
>> > +/// let fw = Firmware::request("path/to/firmware.bin",
>> > dev.as_ref())?;
>> > +/// driver_load_firmware(fw.data());
>> > +/// ```
>> > +pub struct Firmware(Opaque<*const bindings::firmware>);
>> 
>> Wrapping a raw pointer is not the intended use of Qpaque type?
>> 
> 
> What is the intended use?
> As I see it, all uses wrapp some binding::* – but a rawpointer to a
> binding shouldn't be wrapped by it?

If I understand correctly, it's for embedding C's struct in Rust's
struct (as all the usage in the tree do). It gives the tricks for
initialization and a pointer to the embedded object.

The C's firmware API gives a pointer to an initialized object so no
reason to use Opaque.

With such C API, Rust's struct simply uses raw pointers in old rust
branch. For example,

https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/chrdev.rs#L28

struct Cdev(*mut bindings::cdev);


Another choice that I know is NonNull:

https://lore.kernel.org/lkml/20240415-alice-mm-v5-4-6f55e4d8ef51@google.com/

pub struct Page {
    page: NonNull<bindings::page>,
}