The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Bruce Robertson <brucer42@gmail.com>
Cc: rust-for-linux@vger.kernel.org, linux-pm@vger.kernel.org,
	 linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Sebastian Reichel <sre@kernel.org>,
	Miguel Ojeda <ojeda@kernel.org>,
	Igor Korotin <igor.korotin@linux.dev>,
	 Gary Guo <gary@garyguo.net>,
	Tamir Duberstein <tamird@kernel.org>,
	Boqun Feng <boqun@kernel.org>
Subject: Re: [RFC PATCH 2/3] rust: power_supply: add power supply class abstraction
Date: Thu, 9 Jul 2026 09:04:00 +0000	[thread overview]
Message-ID: <ak9kAP1SZF8AchQT@google.com> (raw)
In-Reply-To: <20260708214738.25008-3-brucer42@gmail.com>

On Wed, Jul 08, 2026 at 09:47:37PM +0000, Bruce Robertson wrote:
> There is no Rust abstraction for the power supply class in mainline or
> rust-next. Add a minimal one:
> 
>   - a Driver trait carrying the supply's name, type and property list,
>     plus a get_property() method;
>   - a generic extern "C" trampoline the power supply core calls back
>     into, recovering the driver's private data via the parent device and
>     dispatching to get_property();
>   - a Registration that owns the power_supply_desc and unregisters the
>     supply on drop, freeing the descriptor in the correct order.
> 
> Property, type and status constants are re-exported so drivers need not
> reach into the generated bindings directly.
> 
> Signed-off-by: Bruce Robertson <brucer42@gmail.com>

> +impl Drop for Registration {
> +    fn drop(&mut self) {
> +        // SAFETY: `psy` came from a successful `power_supply_register` and has not been
> +        // unregistered. `power_supply_unregister` blocks until no callback is running;
> +        // `_desc` (dropped immediately after) is then no longer referenced by the core.
> +        unsafe { bindings::power_supply_unregister(self.psy) };
> +    }
> +}
> +
> +/// Register a power supply backed by driver `T` against `dev`.
> +pub fn register<T: Driver + 'static>(dev: &Device) -> Result<Registration> {

Should this be &Device<Bound>?

> +    let mut desc = KBox::new(bindings::power_supply_desc::default(), GFP_KERNEL)?;
> +    desc.name = T::NAME.as_char_ptr();
> +    desc.type_ = T::TYPE;
> +    desc.properties = T::PROPERTIES.as_ptr();
> +    desc.num_properties = T::PROPERTIES.len();
> +    desc.get_property = Some(get_property_trampoline::<T>);
> +
> +    // Stable heap address; moving the KBox into `Registration` keeps this valid
> +    // (moving a KBox moves the pointer, not the heap allocation).
> +    let desc_ptr: *const bindings::power_supply_desc = &*desc;
> +
> +    let cfg = bindings::power_supply_config {
> +        drv_data: dev.as_raw().cast::<kernel::ffi::c_void>(),
> +        ..Default::default()
> +    };
> +
> +    // SAFETY: `dev` valid; `desc_ptr` points into `desc`, kept alive in the returned
> +    // `Registration` and freed only after `power_supply_unregister`; `cfg` valid for the call.
> +    let psy = unsafe { bindings::power_supply_register(dev.as_raw(), desc_ptr, &cfg) };
> +    let psy = from_err_ptr(psy)?;
> +
> +    Ok(Registration { psy, _desc: desc })
> +}

This looks like it should be a ::new() method on Registration instead?

I would compare with existing Registration abstractions and mirror how
they do it.

Alice

  reply	other threads:[~2026-07-09  9:04 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08 21:47 [RFC PATCH 0/3] rust: power_supply class abstraction and SMB347 charger driver Bruce Robertson
2026-07-08 21:47 ` [RFC PATCH 1/3] rust: i2c: add SMBus byte transfer helpers Bruce Robertson
2026-07-08 21:47 ` [RFC PATCH 2/3] rust: power_supply: add power supply class abstraction Bruce Robertson
2026-07-09  9:04   ` Alice Ryhl [this message]
2026-07-08 21:47 ` [RFC PATCH 3/3] power: supply: add Rust SMB347 charger driver Bruce Robertson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak9kAP1SZF8AchQT@google.com \
    --to=aliceryhl@google.com \
    --cc=boqun@kernel.org \
    --cc=brucer42@gmail.com \
    --cc=gary@garyguo.net \
    --cc=igor.korotin@linux.dev \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sre@kernel.org \
    --cc=tamird@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox