Re: [PATCH v14 3/7] rust: pwm: Add complete abstraction layer

[Michal Wilczynski <m.wilczynski@samsung.com>]

On 8/20/25 23:11, Elle Rhumsaa wrote:

>> +/// Wrapper for a PWM device [`struct pwm_device`](srctree/include/linux/pwm.h).
>> +#[repr(transparent)]
>> +pub struct Device(Opaque<bindings::pwm_device>);
>> +
>> +impl Device {
>> +    /// Creates a reference to a [`Device`] from a valid C pointer.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
>> +    /// returned [`Device`] reference.
>> +    pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::pwm_device) -> &'a Self {
>> +        // SAFETY: The safety requirements guarantee the validity of the dereference, while the
>> +        // `Device` type being transparent makes the cast ok.
>> +        unsafe { &*ptr.cast::<Self>() }
>> +    }
> 
> This looks like the only constructor for `Device`, and it has restricted
> visibility.
> 
> Would it be helpful to provide a doc-test here to show intended usage?
> 
> After looking at the C code, it looks like the raw `pwm_device` pointer
> is extracted from a `device` struct, or stored in an array of
> `pwm_device` pointers.
> 
> Is the intent that the `pwm_device` will always be allocated from C
> code? For example, is a `pub fn new() -> &'a Self`-style constructor
> not possible/desired?
> 

You're correct the intent is for pwm_device to be allocated and managed
by the C core. The Rust code only ever receives a raw pointer to an
existing device during callbacks.

For this reason, a public new() constructor isn't appropriate, and the
from_raw function is pub(crate) for use only by the FFI adapter. A
doc-test is also impractical since we can't create a valid pwm_device in
a test environment.

> Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
> 

Best regards,
-- 
Michal Wilczynski <m.wilczynski@samsung.com>