Rust for Linux List
 help / color / mirror / Atom feed
From: Bruce Robertson <brucer42@gmail.com>
To: Alice Ryhl <aliceryhl@google.com>
Cc: rust-for-linux@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	sre@kernel.org, ojeda@kernel.org, igor.korotin@linux.dev,
	gary@garyguo.net, tamird@kernel.org, boqun@kernel.org
Subject: Re: [RFC PATCH 2/3] rust: power_supply: add power supply class abstraction
Date: Sat, 18 Jul 2026 03:00:58 +0000	[thread overview]
Message-ID: <20260718030058.1677-1-brucer42@gmail.com> (raw)
In-Reply-To: <ak9kAP1SZF8AchQT@google.com>

On Thu, Jul 09, 2026 at 09:04:00 +0000, Alice Ryhl wrote:
> > +/// 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>?

Yes, it should. That also lines up with Danilo's comment on the i2c
side of this series that I/O-capable operations need at least the
Bound device context. Will fix in v2.

Taking &Device<Bound> also helps with a lifetime problem the cover
letter flagged: the trampoline currently recovers the driver's private
data via device drvdata, which the i2c core only sets after probe()
returns, while power_supply_register() exposes sysfs before that. For
v2 I plan to pass the driver data through power_supply_config::drv_data
at registration time instead, so the callback never depends on drvdata
ordering, and to add a Sync bound on Driver since get_property() can be
called concurrently.

> 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.

Agreed. I'll rework it as Registration::new(dev: &Device<Bound>, ...)
in v2, following the existing Registration patterns in the tree, and
look at tying the descriptor lifetime to devres while I'm there rather
than relying on Drop ordering alone.

Thanks for the review.

Apologies for slow response. Was wilderness camping.

Bruce

  reply	other threads:[~2026-07-18  3:01 UTC|newest]

Thread overview: 9+ 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-11 10:34   ` Igor Korotin
2026-07-11 12:06     ` Danilo Krummrich
2026-07-18  3:03     ` 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
2026-07-18  3:00     ` Bruce Robertson [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=20260718030058.1677-1-brucer42@gmail.com \
    --to=brucer42@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=boqun@kernel.org \
    --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