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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 28 May 2024 14:19:24 +0200
Danilo Krummrich <dakr@xxxxxxxxxx> wrote:

> On Tue, May 28, 2024 at 08:01:26PM +0900, FUJITA Tomonori wrote:
>> On Mon, 27 May 2024 21:22:47 +0200
>> Danilo Krummrich <dakr@xxxxxxxxxx> 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?
>> >> 
>> > 
>> > Indeed, will fix this in v2 and use NonNull instead. I'll also offload most of
>> > the boilerplate in the 'request' functions to some common 'request_internal' one.
>> 
>> You might need to add 'Invariants' comment on Firmware struct.
> 
> Which ones do you think should be documented?

Something like the comment for struct Page looks fine to me. But the
Rust reviewers might have a different opinion.

/// The pointer is valid, and has ownership over the page.



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux