[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8a99664b-8acb-4ace-9c2a-bdada5cac5cb@samsung.com>
Date: Sat, 2 Aug 2025 23:19:40 +0200
From: Michal Wilczynski <m.wilczynski@...sung.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Uwe Kleine-König <ukleinek@...nel.org>, Miguel Ojeda
<ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng
<boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>, Andreas
Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, Trevor
Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Drew Fustini
<drew@...7.com>, Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>, Rob
Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
Dooley <conor+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Alexandre Ghiti <alex@...ti.fr>, Marek Szyprowski
<m.szyprowski@...sung.com>, Benno Lossin <lossin@...nel.org>, Michael
Turquette <mturquette@...libre.com>, Drew Fustini <fustini@...nel.org>,
linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org,
rust-for-linux@...r.kernel.org, linux-riscv@...ts.infradead.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v12 2/3] rust: pwm: Add Kconfig and basic data
structures
On 7/25/25 17:08, Daniel Almeida wrote:
> Hi Michal,
>
>
> Overall looks good, a few minor comments:
Hi,
Congratulations for getting your IoMem series merged. Thank you for your
work, as it will allow me to proceed and re-add the driver part for this
series.
>
> […]
>
>> +
>> +/// Wrapper for board-dependent PWM arguments [`struct pwm_args`](srctree/include/linux/pwm.h).
>> +#[repr(transparent)]
>> +pub struct Args(Opaque<bindings::pwm_args>);
>> +
>> +impl Args {
>> + /// Creates an `Args` wrapper from a C struct pointer.
>> + ///
>> + /// # Safety
>> + ///
>> + /// The caller must ensure that `c_args_ptr` is a valid, non-null pointer
>> + /// to `bindings::pwm_args` and that the pointed-to data is valid
>> + /// for the duration of this function call (as data is copied).
>> + unsafe fn from_c_ptr(c_args_ptr: *const bindings::pwm_args) -> Self {
>> + // SAFETY: Caller guarantees `c_args_ptr` is valid. We dereference it to copy.
>> + Args(Opaque::new(unsafe { *c_args_ptr }))
>> + }
>
> from_raw()
>
>
>> +
>> + /// Returns the period of the PWM signal in nanoseconds.
>> + pub fn period(&self) -> u64 {
>> + // SAFETY: `self.0.get()` returns a pointer to the `bindings::pwm_args`
>> + // managed by the `Opaque` wrapper. This pointer is guaranteed to be
>> + // valid and aligned for the lifetime of `self` because `Opaque` owns a copy.
>> + unsafe { (*self.0.get()).period }
>> + }
>> +
>> + /// Returns the polarity of the PWM signal.
>> + pub fn polarity(&self) -> Result<Polarity, Error> {
>> + // SAFETY: `self.0.get()` returns a pointer to the `bindings::pwm_args`
>> + // managed by the `Opaque` wrapper. This pointer is guaranteed to be
>> + // valid and aligned for the lifetime of `self`.
>> + let raw_polarity = unsafe { (*self.0.get()).polarity };
>> + Polarity::try_from(raw_polarity)
>> + }
>> +}
>> +
>> +/// Wrapper for PWM state [`struct pwm_state`](srctree/include/linux/pwm.h).
>> +#[repr(transparent)]
>> +pub struct State(bindings::pwm_state);
>
> No Opaque<T>?
Since this is a copy of the state it's fine. The Args above should
follow similar pattern, the divergence stemmed when iterating with this
series. So I would rather fix the Args to also not be Opaque as it
doesn't need to be, since it's also a copy of the original (since it's
so small and read only).
>
>> +
>> +impl State {
>> + /// Creates a `State` wrapper by taking ownership of a C `pwm_state` value.
>> + pub(crate) fn from_c(c_state: bindings::pwm_state) -> Self {
>> + State(c_state)
>> + }
>> +
>> + /// Returns `true` if the PWM signal is enabled.
>> + pub fn enabled(&self) -> bool {
>> + self.0.enabled
>> + }
>> +}
>>
>> --
>> 2.34.1
>>
>>
>
> If the lack of Opaque<T> is not a problem for whatever reason:
>
> Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
>
> — Daniel
>
>
Best regards,
--
Michal Wilczynski <m.wilczynski@...sung.com>
Powered by blists - more mailing lists